Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

replace m4.large default with m5.large #850

Merged
merged 1 commit into from
Jan 13, 2022
Merged

replace m4.large default with m5.large #850

merged 1 commit into from
Jan 13, 2022

Conversation

gregsheremeta
Copy link
Contributor

There is no reason to ever use m4 over m5. m4's are
more expensive because it's legacy hardware. m5 is cheaper
and better.

/assign @csrwng

@sjenning
Copy link
Contributor

/lgtm

@sjenning
Copy link
Contributor

openshift installer did this last year openshift/installer#3853

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Jan 12, 2022
@enxebre enxebre mentioned this pull request Jan 12, 2022
Merged
@enxebre
Copy link
Member

enxebre commented Jan 12, 2022 •
edited
Loading

Thanks! nit: could also update the comment in the api definition and regenerate the CRDs/docs // InstanceType is an ec2 instance type for node instances (e.g. m4-large).

@gregsheremeta
Copy link
Contributor Author

/hold

could also update the comment in the api definition and regenerate the CRDs/docs

yeah just noticed that (m4.large vs m4-large -- not used to hyphens!)

@openshift-ci openshift-ci bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jan 12, 2022
@openshift-ci openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label Jan 12, 2022
@gregsheremeta
Copy link
Contributor Author

/unhold

please re-review

@openshift-ci openshift-ci bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jan 12, 2022
@netlify
Copy link

netlify bot commented Jan 12, 2022 •
edited
Loading

✔️ Deploy Preview for hypershift-docs ready!

🔨 Explore the source changes: 1f354fd

🔍 Inspect the deploy log: https://app.netlify.com/sites/hypershift-docs/deploys/61e00dbd4c9b9b00084f9c7e

😎 Browse the preview: https://deploy-preview-850--hypershift-docs.netlify.app/reference/api

@enxebre
Copy link
Member

enxebre commented Jan 12, 2022

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Jan 12, 2022
@alvaroaleman
Copy link
Contributor

/hold
merge conflicts :(

@openshift-ci openshift-ci bot added do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Jan 13, 2022
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Jan 13, 2022

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: alvaroaleman, gregsheremeta

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jan 13, 2022
@gregsheremeta
replace m4.large default with m5.large
1f354fd 
There is no reason to ever use m4 over m5. m4's are
more expensive because it's legacy hardware. m5 is cheaper
and better.

Updated docs and comments that referred to m5-large too.
@openshift-ci openshift-ci bot removed lgtm Indicates that a PR is ready to be merged. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Jan 13, 2022
@gregsheremeta
Copy link
Contributor Author

/unhold

@openshift-ci openshift-ci bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jan 13, 2022
@csrwng
Copy link
Contributor

csrwng commented Jan 13, 2022

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Jan 13, 2022
@openshift-bot
Copy link

/retest-required

Please review the full test history for this PR and help us cut down flakes.

1 similar comment
@openshift-bot
Copy link

/retest-required

Please review the full test history for this PR and help us cut down flakes.

@gregsheremeta
Copy link
Contributor Author 

[2022-01-13T14:19:15Z] NoClusters: No clusters in pool are ready to be claimed 
INFO[2022-01-13T15:49:15Z] Releasing cluster claims for test e2e-aws    
INFO[2022-01-13T15:49:15Z] Ran for 1h35m22s                             
ERRO[2022-01-13T15:49:15Z] Some steps failed:                           
ERRO[2022-01-13T15:49:15Z] 
  * could not run steps: step e2e-aws failed: failed to wait for the created cluster claim to become ready: timed out waiting for the condition

i guess CI is down or something

@openshift-bot
Copy link

/retest-required

Please review the full test history for this PR and help us cut down flakes.

3 similar comments
@openshift-bot
Copy link

/retest-required

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link

/retest-required

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link

/retest-required

Please review the full test history for this PR and help us cut down flakes.

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Jan 13, 2022

@gregsheremeta: all tests passed!

Full PR test history. Your PR dashboard.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

@openshift-merge-robot openshift-merge-robot merged commit 0c257b9 into openshift:main Jan 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@alvaroaleman alvaroaleman

@ironcladlou ironcladlou

@sjenning sjenning

Assignees

@sjenning sjenning

@enxebre enxebre

@csrwng csrwng

@alvaroaleman alvaroaleman

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@gregsheremeta @sjenning @enxebre @alvaroaleman @csrwng @openshift-bot @openshift-merge-robot