-
Notifications
You must be signed in to change notification settings - Fork 476
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
Support AWS China install #209
Conversation
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: wanghaoran1988 The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
### Non-Goals | ||
|
||
* It's not a goal to detail how to request and setup a AWS account in AWS China. | ||
* It's not a goal to detail how to do UPI install. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think another goal would be supporting any endpoints for services that are not part of the SDK for those region.
#### Setup AWS China Account | ||
|
||
The AMIs in global regions are not useable in AWS China regions, we need setup an AWS China Account to host our RHCOS AMIs, so that installer can use them to setup the cluster. | ||
|
||
#### Push RHCOS AMIs to AWS China account, and share them to public. | ||
|
||
Currently, we have CI jobs push the AMIs to public regions, after the AWS China account setup is ready, we should make our CI job start push our AMIs to AWS China regions, and share them to all accounts in AWS China regions. | ||
|
||
#### OpenShift installer support AWS China regions | ||
|
||
The OpenShift installer should be able to use the AMIs that pushed to AWS China regions to provision clusters, and use the correct api endpoints and ARNs, Notable difference for AWS China: | ||
|
||
* AWS resources ARNs in China regions are prefixed with "arn:arn-cn" | ||
* Ec2 service endpoint is "ec2.amazonaws.com.cn" | ||
* Route53 currently is not GA, we can use api endpoint "route53.amazonaws.com.cn" or "api.route53.cn" in AWS China. | ||
|
||
#### Cloud credential operator support AWS China regions | ||
|
||
Cloud credential operator will create AWS client and use IAM service to validate the permission for provided AWS credential, to support AWS China, it should use IAM api endpoint "iam.amazonaws.com.cn" for AWS China regions. | ||
|
||
#### Ingress operator support AWS China regions | ||
|
||
Ingress operator will create ELBs and using route53 service to update related DNS records, to support AWS China, it should use | ||
"route53.amazonaws.com.cn" or "api.route53.cn" api endpoint. And for the resource groups tagging api, it should use "tagging.cn-northwest-1.amazonaws.com.cn" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
these don't seem to correctly belong to User Stories
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What do you mean "these"? Would you mind tell me where I should put these?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Comment on lines 58 to 82
These sound like implementation/proposal and not user stories.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done, moved to "implemetations" part
|
||
* AWS resources ARNs in China regions are prefixed with "arn:arn-cn" | ||
* Ec2 service endpoint is "ec2.amazonaws.com.cn" | ||
* Route53 currently is not GA, we can use api endpoint "route53.amazonaws.com.cn" or "api.route53.cn" in AWS China. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should mention that AWS SDK is missing the endpoint, so we should decide what endpoint is going to be hard coded.
The OpenShift installer should be able to use the AMIs that pushed to AWS China regions to provision clusters, and use the correct api endpoints and ARNs, Notable difference for AWS China: | ||
|
||
* AWS resources ARNs in China regions are prefixed with "arn:arn-cn" | ||
* Ec2 service endpoint is "ec2.amazonaws.com.cn" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
AWS SDK already has endpoint information for this region, so i think we should probably focus on
- make sure you are using SDK defaults
- define how the SDK should be configured so that (1) is achievable.
|
||
The AMIs in global regions are not useable in AWS China regions, we need setup an AWS China Account to host our RHCOS AMIs, so that installer can use them to setup the cluster. | ||
|
||
#### Push RHCOS AMIs to AWS China account, and share them to public. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
|
||
#### Cloud credential operator support AWS China regions | ||
|
||
Cloud credential operator will create AWS client and use IAM service to validate the permission for provided AWS credential, to support AWS China, it should use IAM api endpoint "iam.amazonaws.com.cn" for AWS China regions. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Again i don't think we should be hard coding things that are already in SDK..
Something along the lines of https://github.com/openshift/enhancements/pull/209/files#r379014913 would make more sense.
|
||
#### Ingress operator support AWS China regions | ||
|
||
Ingress operator will create ELBs and using route53 service to update related DNS records, to support AWS China, it should use |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For tagging same as https://github.com/openshift/enhancements/pull/209/files#r379015589
For route 53, i think https://github.com/openshift/enhancements/pull/209/files#r379014462
And this also needs to define, the route53 resources behavior when using resourcetaggingapi
the resourcetaggingapi returns no results for route53 zone, unless the region being used is some predetermined one.. like us-east-1 for all public regions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also ingress operator doesn't create ELB, the kube-controller-manager does.
* Ec2 service endpoint is "ec2.amazonaws.com.cn" | ||
* Route53 currently is not GA, we can use api endpoint "route53.amazonaws.com.cn" or "api.route53.cn" in AWS China. | ||
|
||
#### Cloud credential operator support AWS China regions |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
|
||
Cloud credential operator will create AWS client and use IAM service to validate the permission for provided AWS credential, to support AWS China, it should use IAM api endpoint "iam.amazonaws.com.cn" for AWS China regions. | ||
|
||
#### Ingress operator support AWS China regions |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cc @ironcladlou
|
||
### Test Plan | ||
|
||
Our testing CI should include one AWS China Region, and run the installer and e2e tests in AWS China account. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should include that DP team must setup the account before we can begin testing and have it available to devs in openshift. Calling that out explicitly should help us.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These requests have been made.
This intersects with #201 in that if we had that it'd be a bit more obvious how to upload an AMI to this region manually. |
c403d80
to
6b8c205
Compare
@abhinavdahiya Comments should be addressed, could you take another look again ? |
* It's not a goal to detail how to do UPI install. | ||
* It's not a goal to support all AWS service endpoints that are not part of the SDK for AWS China Regions. | ||
|
||
## Proposal |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need to worry about the Great Firewall? Quay uses S3 as it’s backing store and China blocks it from time to time. It’s been a while since I’ve had to pay attention to that, so maybe it’s not an issue anymore.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Quay.io is slow, but it's still workable at the moment.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay, that doubles my concern then. Slow download speeds are going to have a large impact on cluster install times and their ability to scale quickly. And there still seems to be the possibility that the firewall will block the traffic outright.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Quay has a georeplication project underway which might help, but we cannot depend on that as a prerequisite for this China bringup, especially since so far in China testing we haven't had any failures. Also, we're not do not expect a large volume of cluster installs or scaling activity in China.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wait quay really uses S3 and not a proper CDN that would have regional distribution? Hmm, AFAICS I'm not getting any redirects for the blobs indeed.
@abhinavdahiya @crawford hey - looking to make sure we've addressed all the comments on this to your satisfaction. If not, can you point us to where we need to update the PR? Who can merge this once we've got it all buttoned up? |
will try to take a look at this again. I think I can lgtm and merge this when it's ready. |
@abhinavdahiya Any update on this review? Is there anything blocking or that we can clarify/assist with? |
Issues go stale after 90d of inactivity. Mark the issue as fresh by commenting If this issue is safe to close now please do so with /lifecycle stale |
Stale issues rot after 30d of inactivity. Mark the issue as fresh by commenting If this issue is safe to close now please do so with /lifecycle rotten |
Rotten issues close after 30d of inactivity. Reopen the issue by commenting /close |
@openshift-bot: Closed this PR. In response to this:
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. |
/cc @jeremyeder @abhinavdahiya