AWS: Added experimental option to skip zone check #28417
Conversation
|
Can one of the admins verify that this patch is reasonable to test? If so, please reply "ok to test". This message may repeat a few times in short succession due to jenkinsci/ghprb-plugin#292. Sorry. Otherwise, if this message is too spammy, please complain to ixdy. |
k8s-github-robot
added
size/S
|
This looks fine generally. I wonder if it should be called "DisableZoneCheck", or "DisableAWSValidation", but I don't think it really matters much. I think we do need a comment saying (1) what it is for (non-AWS clouds) (2) that this is experimental (for the moment), and (3) to ask users to state that they're running against a non-AWS cloud when opening any issues (otherwise these bug reports will drive me insane!) I like that we have the Warning message, so we'll at least have some evidence in the logs! Does everything else work? How does API endpoint discovery work with Eucalyptus? I'm all in favor of getting this flag in there, and then discovering what else breaks. It'll probably make the code better for AWS itself as well to run against another backend. So thanks for the contribution. If you can add the comment, ping me & I'll LGTM! |
|
ok to test |
justinsb
added
the
priority/backlog
|
Thanks for the feedback. Would you like me to add the comment in a git commit or a comment in the code? I'm not wedded to the variable name and am fine changing it so something sensible. As far as the API endpoint discovery, I am working on that. If there needs to be a change/enhancement, it probably needs to go in https://github.com/aws/aws-sdk-go. Otherwise, I've tested this in AWS and it seems to work as I expect. |
|
Comment in the code would be great, thanks. I think as long as you just describe it as experimental, we can add any needed subsequent fixes and even change the name if needed. If the name "DisableStrictZoneCheck" makes the most sense to you, then that's the best name :-) I'm just thinking that you'll likely have more tweaks in future, and this name might prove very specific. But you can always change the name later if it's marked as a WIP/experimental! |
|
I finally got back to this. I've added comments per your request. Please let me know if you have any other suggestions. |
|
@justinsb - When you get an opportunity, could you please take a look at my latest commit and let me know if the comments I added address you request? Thanks! |
|
Hi @justinsb. Is there anything else I need to do to have this merged? Please let me know. |
|
GCE e2e build/test passed for commit d69fe11. |
|
Silly me. I didn't pay attention to the fact my build was passing one of the tests. @justinsb, can you please give an LGTM if it looks good to you? |
|
LGTM - thanks! |
justinsb
added
the
lgtm
k8s-github-robot
removed
the
lgtm
justinsb
changed the title
justinsb
added
release-note
|
@k8s-bot test this [submit-queue is verifying that this PR is safe to merge] |
|
GCE e2e build/test passed for commit d69fe11. |
|
Automatic merge from submit-queue |
This pull request resolves #28380. In the vast majority of cases, it is appropriate to validate the AWS region against a known set of regions. However, there is the edge case where this is undesirable as Kubernetes may be deployed in an AWS-like environment where the region is not one of the known regions.
By adding the optional DisableStrictZoneCheck true to the [Global] section in the aws.conf file (e.g. /etc/aws/aws.conf) one can bypass the ragion validation.