Ensure object returned by volume getCloudProvider incorporates cloud config #23769

saad-ali · 2016-04-02T00:04:12Z

This PR addresses #23517.

Problem
The existing GCE PD and AWS EBS volume plugin code were fetching cloud provider without specifying a cloud config: cloudprovider.GetCloudProvider("gce", nil)
This caused the cloud provider to use default auth mechanism, which is not acceptable for the provisioning controller running on GKE master.

Fix
This PR does the following:

Modifies the GCE PD and AWS EBS volume plugin code to use the cloud provider object pre-constructed by the binary with a cloud config.
Enable provisioning E2E test for GKE (to catch future issues).

Thanks to @cjcullen for debugging and finding the root cause! 👍

This should be cherry-picked into the v1.2 branch for the next release.

k8s-cherrypick-bot · 2016-04-02T00:06:52Z

Removing label cherrypick-candidate because no release milestone was set. This is an invalid state and thus this PR is not being considered for cherry-pick to any release branch. Please add an appropriate release milestone and then re-add the label.

k8s-github-robot · 2016-04-02T00:09:37Z

Labelling this PR as size/M

k8s-bot · 2016-04-02T00:39:17Z

GCE e2e build/test passed for commit e95dd021bca7c6b1a357b492badac3b364b17b67.

roberthbailey · 2016-04-02T03:49:46Z

/cc @justinsb since this also makes some minor changes to the AWS cloud provider.

justinsb · 2016-04-02T14:29:14Z

pkg/volume/aws_ebs/aws_util.go

+    }
+
+    cloudProvider := plugin.host.GetCloudProvider()
+	awsCloudProvider, ok := cloudProvider.(*aws.AWSCloud)


I'm seeing weird indentation here.. gofmt needed?

justinsb · 2016-04-02T14:32:27Z

Needs gofmt (CI picked it up too - I thought I was going crazy!). But changes LGTM on the AWS side.

I also like that the provider is now a real singleton - I wonder if that was causing issues in the past for the AWS volume mount cache (before I removed it in 1.2)

alex-mohr · 2016-04-04T16:24:22Z

@saad-ali can you please get the gofmt and build failures fixed, submit this, cherrypick it to 1.2 and get @david-mcmahon to signoff and start the 1.2.2 build please? @a-robinson is GKE oncall this week.

After that's done, let's start a postmortem to figure out what happened and how we can improve the process so similar issues are less likely to re-occur?

saad-ali · 2016-04-04T17:54:07Z

Yep, will do.

k8s-github-robot · 2016-04-04T18:06:36Z

PR changed after LGTM, removing LGTM.

k8s-bot · 2016-04-04T18:34:14Z

GCE e2e build/test passed for commit e7b14e7.

k8s-github-robot · 2016-04-04T19:55:02Z

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

k8s-bot · 2016-04-04T20:25:11Z

GCE e2e build/test passed for commit e7b14e7.

saad-ali · 2016-04-04T21:08:47Z

@k8s-bot unit test this issue: #21954

saad-ali · 2016-04-04T22:25:36Z

@k8s-bot unit test this issue: #21640

cjcullen · 2016-04-05T00:27:46Z

LGTM

saad-ali · 2016-04-05T00:30:50Z

Verified dynamic provision on GKE works with this PR:

2016/04/04 17:23:05 e2e.go:194: Running: Ginkgo tests
Setting up for KUBERNETES_PROVIDER="gke".
... in gke:prepare-e2e()
... in gke:detect-master()
... in gke:detect-project()
Your active configuration is: [default]

... Using project: cloud-kubernetes-dev
... in gke:detect-node-instance-group()
Apr  4 17:23:10.161: INFO: Fetching cloud provider for "gke"

[...]
Running Suite: Kubernetes e2e suite
===================================
Random Seed: 1459815789 - Will randomize all specs
Will run 1 of 268 specs

Apr  4 17:23:10.683: INFO: >>> testContext.KubeConfig: /usr/local/google/home/saadali/.kube/config

Apr  4 17:23:10.809: INFO: Waiting up to 10m0s for all pods (need at least 0) in namespace 'kube-system' to be running and ready
Apr  4 17:23:11.073: INFO: 10 / 10 pods in namespace 'kube-system' are running and ready (0 seconds elapsed)
Apr  4 17:23:11.073: INFO: expected 3 pod replicas in namespace 'kube-system', 3 are Running and Ready.
SSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSS
------------------------------
[k8s.io] Dynamic provisioning [k8s.io] DynamicProvisioner 
  should create and delete persistent volumes
  /go/src/k8s.io/kubernetes/_output/dockerized/go/src/k8s.io/kubernetes/test/e2e/volume_provisioning.go:122
[BeforeEach] [k8s.io] Dynamic provisioning
  /go/src/k8s.io/kubernetes/_output/dockerized/go/src/k8s.io/kubernetes/test/e2e/framework.go:101
STEP: Creating a kubernetes client
Apr  4 17:23:11.074: INFO: >>> testContext.KubeConfig: /usr/local/google/home/saadali/.kube/config

STEP: Building a namespace api object
Apr  4 17:23:11.120: INFO: Waiting up to 2m0s for service account default to be provisioned in ns e2e-tests-volume-provisioning-lr4pk
Apr  4 17:23:11.158: INFO: Service account default in ns e2e-tests-volume-provisioning-lr4pk with secrets found. (38.350711ms)
STEP: Waiting for a default service account to be provisioned in namespace
Apr  4 17:23:11.158: INFO: Waiting up to 2m0s for service account default to be provisioned in ns e2e-tests-volume-provisioning-lr4pk
Apr  4 17:23:11.196: INFO: Service account default in ns e2e-tests-volume-provisioning-lr4pk with secrets found. (38.250266ms)
[BeforeEach] [k8s.io] Dynamic provisioning
  /go/src/k8s.io/kubernetes/_output/dockerized/go/src/k8s.io/kubernetes/test/e2e/volume_provisioning.go:49
[It] should create and delete persistent volumes
  /go/src/k8s.io/kubernetes/_output/dockerized/go/src/k8s.io/kubernetes/test/e2e/volume_provisioning.go:122
STEP: creating a claim with a dynamic provisioning annotation
Apr  4 17:23:11.240: INFO: Waiting up to 5m0s for PersistentVolumeClaim pvc-t5h6s to have phase Bound
Apr  4 17:23:11.285: INFO: PersistentVolumeClaim pvc-t5h6s found but phase is Pending instead of Bound.
Apr  4 17:23:13.324: INFO: PersistentVolumeClaim pvc-t5h6s found but phase is Pending instead of Bound.
Apr  4 17:23:15.362: INFO: PersistentVolumeClaim pvc-t5h6s found and phase=Bound (4.12245163s)
STEP: checking the claim
STEP: checking the created volume is writable
Apr  4 17:23:15.486: INFO: Waiting up to 15m0s for pod pvc-volume-tester-27jl5 status to be success or failure
Apr  4 17:23:15.525: INFO: Nil State.Terminated for container 'volume-tester' in pod 'pvc-volume-tester-27jl5' in namespace 'e2e-tests-volume-provisioning-lr4pk' so far
Apr  4 17:23:15.525: INFO: Waiting for pod pvc-volume-tester-27jl5 in namespace 'e2e-tests-volume-provisioning-lr4pk' status to be 'success or failure'(found phase: "Pending", readiness: false) (38.769729ms elapsed)
[...]
Apr  4 17:25:16.467: INFO: Nil State.Terminated for container 'volume-tester' in pod 'pvc-volume-tester-o4avb' in namespace 'e2e-tests-volume-provisioning-lr4pk' so far
Apr  4 17:25:16.467: INFO: Waiting for pod pvc-volume-tester-o4avb in namespace 'e2e-tests-volume-provisioning-lr4pk' status to be 'success or failure'(found phase: "Pending", readiness: false) (1m48.597845805s elapsed)
STEP: Saw pod success
STEP: Sleeping to let kubelet destroy all pods
STEP: deleting the claim
Apr  4 17:26:18.605: INFO: Waiting up to 10m0s for PersistentVolume pv-gce-o4lw5 to get deleted
Apr  4 17:26:18.643: INFO: PersistentVolume pv-gce-o4lw5 found and phase=Released (38.376811ms)
Apr  4 17:26:19.682: INFO: PersistentVolume pv-gce-o4lw5 found and phase=Released (1.077469187s)
Apr  4 17:26:20.722: INFO: PersistentVolume pv-gce-o4lw5 found and phase=Released (2.116694121s)
Apr  4 17:26:21.761: INFO: PersistentVolume pv-gce-o4lw5 found and phase=Released (3.156407539s)
Apr  4 17:26:22.800: INFO: PersistentVolume pv-gce-o4lw5 was removed
[AfterEach] [k8s.io] Dynamic provisioning
  /go/src/k8s.io/kubernetes/_output/dockerized/go/src/k8s.io/kubernetes/test/e2e/framework.go:102
Apr  4 17:26:22.838: INFO: Waiting up to 1m0s for all nodes to be ready
STEP: Destroying namespace "e2e-tests-volume-provisioning-lr4pk" for this suite.

• [SLOW TEST:196.964 seconds]
[k8s.io] Dynamic provisioning
/go/src/k8s.io/kubernetes/_output/dockerized/go/src/k8s.io/kubernetes/test/e2e/framework.go:420
  [k8s.io] DynamicProvisioner
  /go/src/k8s.io/kubernetes/_output/dockerized/go/src/k8s.io/kubernetes/test/e2e/framework.go:420
    should create and delete persistent volumes
    /go/src/k8s.io/kubernetes/_output/dockerized/go/src/k8s.io/kubernetes/test/e2e/volume_provisioning.go:122
------------------------------
SSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSS
Ran 1 of 268 Specs in 197.356 seconds
SUCCESS! -- 1 Passed | 0 Failed | 0 Pending | 267 Skipped PASS

k8s-github-robot · 2016-04-05T01:21:18Z

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

k8s-bot · 2016-04-05T01:51:53Z

GCE e2e build/test passed for commit e7b14e7.

k8s-github-robot · 2016-04-05T03:12:58Z

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

k8s-bot · 2016-04-05T03:43:08Z

GCE e2e build/test passed for commit e7b14e7.

k8s-github-robot · 2016-04-05T03:52:13Z

Automatic merge from submit-queue

zmerlynn · 2016-04-06T20:05:47Z

@saad-ali: I tried to create an automated cherry-pick for this on release-1.2 and the following conflict arose:

Applying: Ensure volume GetCloudProvider code uses cloud config
Using index info to reconstruct a base tree...
M       pkg/volume/aws_ebs/aws_util.go
M       pkg/volume/gce_pd/gce_util.go
A       test/e2e/volume_provisioning.go
Falling back to patching base and 3-way merge...
CONFLICT (modify/delete): test/e2e/volume_provisioning.go deleted in d5fcbb8a2cea9a780242187aeb810969fdb64562 and modified in Ensure volume GetCloudProvider code uses cloud config. Version Ensure volume GetCloudProvider code uses cloud config of test/e2e/volume_provisioning.go left in tree.
Auto-merging pkg/volume/gce_pd/gce_util.go
Auto-merging pkg/volume/aws_ebs/aws_util.go
error: Failed to merge in the changes.
Patch failed at 0001 Ensure volume GetCloudProvider code uses cloud config
The copy of the patch that failed is found in: .git/rebase-apply/patch
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

Please create a cherry-pick ASAP if you want this in 1.2.2 or the release train will leave without you.

saad-ali · 2016-04-06T21:00:04Z

Please create a cherry-pick ASAP if you want this in 1.2.2 or the release train will leave without you.

Working on it.

saad-ali · 2016-04-06T21:35:21Z

#23943

#23769-upstream-release-1.2 Automated cherry pick of #21140 #23769 upstream release 1.2

k8s-cherrypick-bot · 2016-04-06T22:21:58Z

Commit found in the "release-1.2" branch appears to be this PR. Removing the "cherrypick-candidate" label. If this is an error find help to get your PR picked.

…ck-of-#21140-kubernetes#23769-upstream-release-1.2 Automated cherry pick of kubernetes#21140 kubernetes#23769 upstream release 1.2

saad-ali assigned cjcullen Apr 2, 2016

saad-ali added release-note Denotes a PR that will be considered when it comes time to generate release notes. cherrypick-candidate labels Apr 2, 2016

googlebot added the cla: yes label Apr 2, 2016

k8s-cherrypick-bot removed the cherrypick-candidate label Apr 2, 2016

k8s-github-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Apr 2, 2016

saad-ali added this to the v1.2 milestone Apr 2, 2016

saad-ali added the cherrypick-candidate label Apr 2, 2016

roberthbailey added the priority/critical-urgent Highest priority. Must be actively worked on as someone's top priority right now. label Apr 2, 2016

justinsb reviewed Apr 2, 2016
View reviewed changes

alex-mohr added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Apr 4, 2016

Ensure volume GetCloudProvider code uses cloud config

e7b14e7

saad-ali force-pushed the fixVolumeCloudProvider branch from e95dd02 to e7b14e7 Compare April 4, 2016 17:59

k8s-github-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Apr 4, 2016

roberthbailey added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Apr 4, 2016

saad-ali closed this Apr 5, 2016

saad-ali reopened this Apr 5, 2016

k8s-github-robot merged commit 02e0b29 into kubernetes:master Apr 5, 2016

saad-ali deleted the fixVolumeCloudProvider branch April 5, 2016 22:21

roberthbailey added the cherry-pick-approved Indicates a cherry-pick PR into a release branch has been approved by the release branch manager. label Apr 6, 2016

saad-ali mentioned this pull request Apr 6, 2016

Automated cherry pick of #21140 #23769 upstream release 1.2 #23943

Merged

zmerlynn added a commit that referenced this pull request Apr 6, 2016

Merge pull request #23943 from saad-ali/automated-cherry-pick-of-#21140-

b0c9299

#23769-upstream-release-1.2 Automated cherry pick of #21140 #23769 upstream release 1.2

k8s-cherrypick-bot removed the cherrypick-candidate label Apr 6, 2016

roberthbailey mentioned this pull request Apr 7, 2016

Insufficient Permission for Persistent Volume Provisioning Example #23517

Closed

saad-ali mentioned this pull request May 5, 2016

Move getCloudProvider retries to getCloudProvider() #25179

Merged

petercm mentioned this pull request May 13, 2016

GCE Ingress fails to sync instance group in multi-az cluster kubernetes-retired/contrib#983

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Ensure object returned by volume getCloudProvider incorporates cloud config #23769

Ensure object returned by volume getCloudProvider incorporates cloud config #23769

saad-ali commented Apr 2, 2016

k8s-cherrypick-bot commented Apr 2, 2016

k8s-github-robot commented Apr 2, 2016

k8s-bot commented Apr 2, 2016

roberthbailey commented Apr 2, 2016

justinsb Apr 2, 2016

justinsb commented Apr 2, 2016

alex-mohr commented Apr 4, 2016

saad-ali commented Apr 4, 2016

k8s-github-robot commented Apr 4, 2016

k8s-bot commented Apr 4, 2016

k8s-github-robot commented Apr 4, 2016

k8s-bot commented Apr 4, 2016

saad-ali commented Apr 4, 2016

saad-ali commented Apr 4, 2016

cjcullen commented Apr 5, 2016

saad-ali commented Apr 5, 2016

k8s-github-robot commented Apr 5, 2016

k8s-bot commented Apr 5, 2016

k8s-github-robot commented Apr 5, 2016

k8s-bot commented Apr 5, 2016

k8s-github-robot commented Apr 5, 2016

zmerlynn commented Apr 6, 2016

saad-ali commented Apr 6, 2016

saad-ali commented Apr 6, 2016

k8s-cherrypick-bot commented Apr 6, 2016

Ensure object returned by volume getCloudProvider incorporates cloud config #23769

Ensure object returned by volume getCloudProvider incorporates cloud config #23769

Conversation

saad-ali commented Apr 2, 2016

k8s-cherrypick-bot commented Apr 2, 2016

k8s-github-robot commented Apr 2, 2016

k8s-bot commented Apr 2, 2016

roberthbailey commented Apr 2, 2016

justinsb Apr 2, 2016

Choose a reason for hiding this comment

justinsb commented Apr 2, 2016

alex-mohr commented Apr 4, 2016

saad-ali commented Apr 4, 2016

k8s-github-robot commented Apr 4, 2016

k8s-bot commented Apr 4, 2016

k8s-github-robot commented Apr 4, 2016

k8s-bot commented Apr 4, 2016

saad-ali commented Apr 4, 2016

saad-ali commented Apr 4, 2016

cjcullen commented Apr 5, 2016

saad-ali commented Apr 5, 2016

k8s-github-robot commented Apr 5, 2016

k8s-bot commented Apr 5, 2016

k8s-github-robot commented Apr 5, 2016

k8s-bot commented Apr 5, 2016

k8s-github-robot commented Apr 5, 2016

zmerlynn commented Apr 6, 2016

saad-ali commented Apr 6, 2016

saad-ali commented Apr 6, 2016

k8s-cherrypick-bot commented Apr 6, 2016