AWS: trust region if found from AWS metadata #38880

justinsb · 2016-12-16T18:27:35Z

AWS: trust region if found from AWS metadata

Means we can run in newly announced regions without a code change.

We don't register the ECR provider in new regions, so we will still need
a code change for now.

Fix #35014

k8s-reviewable · 2016-12-16T18:27:40Z

This change is

derekwaynecarr · 2016-12-20T18:37:21Z

This is extremely useful. Thanks for putting this together.

justinsb · 2016-12-20T20:43:42Z

Thanks @derekwaynecarr - you can see the ECR issue as well ... with this PR we won't have to register new regions in our code, but we still won't be able to use ECR in a new region unless we are running in that region.

To tackle that I think we would have to either change it so that we could build docker configs on demand, or just issue a DescribeRegions call and automatically register all of those regions. Unsure which is better right now.

chrislovecnm · 2016-12-28T01:02:17Z

you can see the ECR issue as well ... with this PR we won't have to register new regions in our code, but we still won't be able to use ECR in a new region unless we are running in that region.
To tackle that I think we would have to either change it so that we could build docker configs on demand, or just issue a DescribeRegions call and automatically register all of those regions. Unsure which is better right now.

Does this remove functionality that we have now with ECR?

chrislovecnm

Quick question

chrislovecnm · 2016-12-28T01:04:08Z

pkg/credentialprovider/aws/aws_credentials.go

@@ -100,21 +80,20 @@ type ecrProvider struct {

 var _ credentialprovider.DockerConfigProvider = &ecrProvider{}

-// Init creates a lazy provider for each AWS region, in order to support
+// RegisterCredentialsProvider registers a credential provider for the specified region.


Do we have this documented anywhere? If so do we need to update the docs?

chrislovecnm · 2016-12-28T01:05:57Z

@erictune a couple of questions, and since there actually is an active @kubernetes/sig-aws-misc group that is active, should we be doing code reviews now?

justinsb · 2016-12-28T01:18:26Z

Does this remove functionality that we have now with ECR?

No, it just adds functionality: we automatically recognize the ECR region we are running in, even if it isn't in our hard-coded list.

chrislovecnm

Looks great ... @erictune can we get this in? This is super important :)

chrislovecnm · 2017-01-04T00:22:13Z

@erictune can I get a PR review :)

colhom · 2017-01-06T23:07:01Z

fixes kubernetes-retired/kube-aws#198

zmerlynn · 2017-01-10T05:43:58Z

@k8s-bot aws test this

zmerlynn · 2017-01-10T05:46:52Z

/lgtm

justinsb · 2017-01-17T05:59:02Z

@zmerlynn OK if I reapply LGTM after the rebase I had to do?

zmerlynn · 2017-01-18T00:17:14Z

/lgtm

@justinsb yup

k8s-ci-robot · 2017-01-18T00:30:18Z

Jenkins Bazel Build failed for commit 6f632d04cfe06fa4189fb0d60f710cdeb73ccfcd. Full PR test history. cc @justinsb

The magic incantation to run this job again is @k8s-bot bazel test this. Please help us cut down flakes by linking to an open flake issue when you hit one in your PR.

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.

k8s-ci-robot · 2017-01-18T05:56:16Z

Jenkins GKE smoke e2e failed for commit 3eec5409f56583bc80912c581c2471ea4053d98a. Full PR test history. cc @justinsb

The magic incantation to run this job again is @k8s-bot cvm gke e2e test this. Please help us cut down flakes by linking to an open flake issue when you hit one in your PR.

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.

k8s-ci-robot · 2017-01-18T06:03:46Z

Jenkins verification failed for commit 3eec5409f56583bc80912c581c2471ea4053d98a. Full PR test history. cc @justinsb

The magic incantation to run this job again is @k8s-bot verify test this. Please help us cut down flakes by linking to an open flake issue when you hit one in your PR.

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.

Means we can run in newly announced regions without a code change. We don't register the ECR provider in new regions, so we will still need a code change for now. This also means we do trust config / instance metadata, and don't reject incorrectly configured zones. Fix kubernetes#35014

k8s-ci-robot · 2017-01-18T15:01:45Z

Jenkins GCI GCE e2e failed for commit 04b787b. Full PR test history. cc @justinsb

The magic incantation to run this job again is @k8s-bot gci gce e2e test this. Please help us cut down flakes by linking to an open flake issue when you hit one in your PR.

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.

justinsb · 2017-01-19T03:07:25Z

@k8s-bot gci gce e2e test this

k8s-github-robot · 2017-01-24T00:17:25Z

[APPROVALNOTIFIER] Needs approval from an approver in each of these OWNERS Files:

~~pkg/cloudprovider/providers/aws/OWNERS~~ [smarterclayton]
~~pkg/credentialprovider/aws/OWNERS~~ [deads2k,smarterclayton]

You can indicate your approval by writing /approve in a comment
You can cancel your approval by writing /approve cancel in a comment

chrislovecnm · 2017-01-24T00:52:36Z

Can we get this cherry picked into 1.5.x??

chrislovecnm · 2017-01-24T00:53:49Z

@zmerlynn can we get another looks good to me?

zmerlynn · 2017-01-24T00:54:09Z

/lgtm

grodrigues3 · 2017-01-24T02:49:26Z

@k8s-bot test this

Tests are more than 96 hours old. Re-running tests.

deads2k · 2017-01-24T12:46:43Z

Wow, this approval thing is going to be really frustrating. Also, why would it suggest asking yourself for approval??

I trust @zmerlynn to know what this does.

/approve

zmerlynn · 2017-01-24T17:41:23Z

@deads2k: Looks like the comment has to start with /approve

deads2k · 2017-01-24T17:53:47Z

/approve

smarterclayton · 2017-01-24T20:37:41Z

/approve

k8s-github-robot · 2017-01-24T22:12:11Z

@k8s-bot test this [submit-queue is verifying that this PR is safe to merge]

k8s-ci-robot · 2017-01-24T22:31:33Z

Jenkins GCI GKE smoke e2e failed for commit 04b787b. Full PR test history.

cc @justinsb, your PR dashboard

The magic incantation to run this job again is @k8s-bot gci gke e2e test this. Please help us cut down flakes by linking to an open flake issue when you hit one in your PR.

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.

k8s-github-robot · 2017-01-24T23:05:00Z

Automatic merge from submit-queue

k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Dec 16, 2016

k8s-github-robot assigned erictune Dec 16, 2016

k8s-github-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. release-note Denotes a PR that will be considered when it comes time to generate release notes. labels Dec 16, 2016

justinsb force-pushed the peninsula_club branch 2 times, most recently from d7aea6a to 1133b41 Compare December 20, 2016 17:50

k8s-github-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Dec 20, 2016

justinsb force-pushed the peninsula_club branch 2 times, most recently from 6ab00b0 to 80e8897 Compare December 20, 2016 18:05

k8s-github-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Dec 20, 2016

justinsb mentioned this pull request Dec 20, 2016

AWS: recognize eu-west-2 region #38746

Merged

justinsb force-pushed the peninsula_club branch from 80e8897 to 0ed32a5 Compare December 20, 2016 20:46

chrislovecnm mentioned this pull request Dec 28, 2016

Support eu-west-2 (London) availability zones kubernetes/kops#1267

Closed

chrislovecnm reviewed Dec 28, 2016

View reviewed changes

chrislovecnm approved these changes Dec 28, 2016

View reviewed changes

ipedrazas mentioned this pull request Jan 6, 2017

eu-west-2 unsupported ? kubernetes-retired/kube-aws#198

Closed

justinsb assigned zmerlynn and unassigned erictune Jan 10, 2017

zmerlynn approved these changes Jan 10, 2017

View reviewed changes

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jan 10, 2017

justinsb force-pushed the peninsula_club branch from a04b706 to 6f632d0 Compare January 13, 2017 07:26

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jan 18, 2017

justinsb force-pushed the peninsula_club branch from 6f632d0 to ec695bc Compare January 18, 2017 05:05

k8s-github-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jan 18, 2017

justinsb force-pushed the peninsula_club branch from ec695bc to 3eec540 Compare January 18, 2017 05:37

justinsb force-pushed the peninsula_club branch from 3eec540 to 04b787b Compare January 18, 2017 14:18

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jan 24, 2017

k8s-github-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jan 24, 2017

k8s-github-robot merged commit a4f3319 into kubernetes:master Jan 24, 2017

bparees mentioned this pull request Jun 14, 2018

Document how to work around unknown AWS regions in S3 driver openshift/image-registry#104

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

AWS: trust region if found from AWS metadata #38880

AWS: trust region if found from AWS metadata #38880

justinsb commented Dec 16, 2016

k8s-reviewable commented Dec 16, 2016

derekwaynecarr commented Dec 20, 2016

justinsb commented Dec 20, 2016

chrislovecnm commented Dec 28, 2016

chrislovecnm left a comment

chrislovecnm Dec 28, 2016

chrislovecnm commented Dec 28, 2016

justinsb commented Dec 28, 2016

chrislovecnm left a comment

chrislovecnm commented Jan 4, 2017

colhom commented Jan 6, 2017

zmerlynn commented Jan 10, 2017

zmerlynn commented Jan 10, 2017

justinsb commented Jan 17, 2017

zmerlynn commented Jan 18, 2017

k8s-ci-robot commented Jan 18, 2017

k8s-ci-robot commented Jan 18, 2017

k8s-ci-robot commented Jan 18, 2017

k8s-ci-robot commented Jan 18, 2017

justinsb commented Jan 19, 2017

k8s-github-robot commented Jan 24, 2017 •

edited

Loading

chrislovecnm commented Jan 24, 2017

chrislovecnm commented Jan 24, 2017

zmerlynn commented Jan 24, 2017

grodrigues3 commented Jan 24, 2017

deads2k commented Jan 24, 2017

zmerlynn commented Jan 24, 2017 •

edited

Loading

deads2k commented Jan 24, 2017

smarterclayton commented Jan 24, 2017

k8s-github-robot commented Jan 24, 2017

k8s-ci-robot commented Jan 24, 2017

k8s-github-robot commented Jan 24, 2017

AWS: trust region if found from AWS metadata #38880

AWS: trust region if found from AWS metadata #38880

Conversation

justinsb commented Dec 16, 2016

k8s-reviewable commented Dec 16, 2016

derekwaynecarr commented Dec 20, 2016

justinsb commented Dec 20, 2016

chrislovecnm commented Dec 28, 2016

chrislovecnm left a comment

Choose a reason for hiding this comment

chrislovecnm Dec 28, 2016

Choose a reason for hiding this comment

chrislovecnm commented Dec 28, 2016

justinsb commented Dec 28, 2016

chrislovecnm left a comment

Choose a reason for hiding this comment

chrislovecnm commented Jan 4, 2017

colhom commented Jan 6, 2017

zmerlynn commented Jan 10, 2017

zmerlynn commented Jan 10, 2017

justinsb commented Jan 17, 2017

zmerlynn commented Jan 18, 2017

k8s-ci-robot commented Jan 18, 2017

k8s-ci-robot commented Jan 18, 2017

k8s-ci-robot commented Jan 18, 2017

k8s-ci-robot commented Jan 18, 2017

justinsb commented Jan 19, 2017

k8s-github-robot commented Jan 24, 2017 • edited Loading

chrislovecnm commented Jan 24, 2017

chrislovecnm commented Jan 24, 2017

zmerlynn commented Jan 24, 2017

grodrigues3 commented Jan 24, 2017

deads2k commented Jan 24, 2017

zmerlynn commented Jan 24, 2017 • edited Loading

deads2k commented Jan 24, 2017

smarterclayton commented Jan 24, 2017

k8s-github-robot commented Jan 24, 2017

k8s-ci-robot commented Jan 24, 2017

k8s-github-robot commented Jan 24, 2017

k8s-github-robot commented Jan 24, 2017 •

edited

Loading

zmerlynn commented Jan 24, 2017 •

edited

Loading