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.

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

Add new regions and corresponding AMIs #209

Merged
merged 2 commits into from
Jul 26, 2023

Conversation

furkatgofurov7
Copy link

We have added new regions in rancher/ui#5037 to the rancher UI and the new regions are visible. However when validating them, it was noticed that there is mismatch between, supported AWS region list for Cloud credentials <-> AWS region list for provisioning EKS clusters. More info here: rancher/dashboard#8701 (comment)

This PR adds those new regions to the regions list with it's corresponding AMIs from https://cloud-images.ubuntu.com/locator/ec2/

@furkatgofurov7 furkatgofurov7 requested a review from rmweir June 16, 2023 12:49
@furkatgofurov7
Copy link
Author

furkatgofurov7 commented Jun 16, 2023

@rmweir can you PTAL, thanks!

@rmweir
Copy link

rmweir commented Jun 22, 2023

@furkatgofurov7 my bad but these look out of date now. Please update. Anyone can approve these PRs as long as they check the AMIs against the list, this includes UI.

@furkatgofurov7 furkatgofurov7 force-pushed the add-new-regions-details branch from 95d6386 to cb05333 Compare June 22, 2023 17:26
@furkatgofurov7
Copy link
Author

@furkatgofurov7 my bad but these look out of date now. Please update. Anyone can approve these PRs as long as they check the AMIs against the list, this includes UI.

@rmweir updated now

rmweir
rmweir previously approved these changes Jun 23, 2023
Copy link

@rmweir rmweir left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@rmweir
Copy link

rmweir commented Jun 23, 2023

Looks like a test is failing so LGTM once that is fixed.

@furkatgofurov7
Copy link
Author

furkatgofurov7 commented Jun 26, 2023

All tests are failing with the same message:

--- FAIL: TestValidateAwsRegionInvalid (0.00s)
    amazonec2_test.go:135: 
        	Error Trace:	/go/src/github.com/rancher/machine/drivers/amazonec2/amazonec2_test.go:135
        	Error:      	An error is expected but got nil.
        	Test:       	TestValidateAwsRegionInvalid
failed to read user data file "/tmp/awsuserdata2983833526/does-not-exist.yml": open /tmp/awsuserdata2983833526/does-not-exist.yml: no such file or directory
FAIL

and looks like not relevant to this change.

@jiaqiluo jiaqiluo requested a review from a team July 14, 2023 21:52
Copy link
Member

@jiaqiluo jiaqiluo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The CI fails because the function TestValidateAwsRegionInvalid expects the region "eu-central-2" is not in the regionDetails list.

To make the CI happy, you must change the above function to use another value for testing.

@furkatgofurov7
Copy link
Author

@rmweir @jiaqiluo can you PTAL and help merging this before it gets outdated again, thanks!

Copy link
Member

@jiaqiluo jiaqiluo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM as the CI passed.

@furkatgofurov7 furkatgofurov7 merged commit 4b574c7 into rancher:master Jul 26, 2023
@furkatgofurov7 furkatgofurov7 deleted the add-new-regions-details branch July 26, 2023 20:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants