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

test: fix azure disk e2e test failure #82324

Merged
merged 2 commits into from Sep 11, 2019

Conversation

@andyzhangx
Copy link
Member

commented Sep 4, 2019

What type of PR is this?
/kind failing-test

What this PR does / why we need it:

This PR fixed the azure disk e2e test failure, default type of azure disk has been changed to managed, so in the test, we also need to change the disk type to managed, otherwise we hit following error:

Warning  FailedAttachVolume  13s (x6 over 31s)  attachdetach-controller  AttachVolume.Attach failed for volume "vol1" : Attach volume "jsafrane-e2e-41eb8696-5c88-4906-a8c3-debf71f7b856.vhd" to instance "/subscriptions/xxx/resourceGroups/jsafrane-dev-6t8r2-rg/providers/Microsoft.Compute/virtualMachines/jsafrane-dev-6t8r2-worker-centralus2-5m9r8" failed with compute.VirtualMachinesClient#CreateOrUpdate: Failure sending request: StatusCode=0 -- Original Error: autorest/azure: Service returned an error. Status=<nil> Code="OperationNotAllowed" Message="Addition of a blob based disk to VM with managed disks is not supported." Target="dataDisk"

Which issue(s) this PR fixes:

Fixes #82272

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

none

Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:

none

/assign @jsafrane

cc @ritazh

/priority important-soon
/sig cloud-provider
/area provider/azure
/kind test

@jsafrane

This comment has been minimized.

Copy link
Member

commented Sep 4, 2019

This PR helps a bit. I had to change also PV / in-line volume definition this way:

--- a/vendor/k8s.io/kubernetes/test/e2e/storage/drivers/in_tree.go
+++ b/vendor/k8s.io/kubernetes/test/e2e/storage/drivers/in_tree.go
@@ -1397,10 +1397,12 @@ func (a *azureDriver) GetVolumeSource(readOnly bool, fsType string, volume tests
 
        diskName := av.volumeName[(strings.LastIndex(av.volumeName, "/") + 1):]
 
+       kind := v1.AzureManagedDisk
        volSource := v1.VolumeSource{
                AzureDisk: &v1.AzureDiskVolumeSource{
                        DiskName:    diskName,
                        DataDiskURI: av.volumeName,
+                       Kind: &kind,
                        ReadOnly:    &readOnly,
                },
        }
@@ -1416,10 +1418,12 @@ func (a *azureDriver) GetPersistentVolumeSource(readOnly bool, fsType string, vo
 
        diskName := av.volumeName[(strings.LastIndex(av.volumeName, "/") + 1):]
 
+       kind := v1.AzureManagedDisk
        pvSource := v1.PersistentVolumeSource{
                AzureDisk: &v1.AzureDiskVolumeSource{
                        DiskName:    diskName,
                        DataDiskURI: av.volumeName,
+                       Kind: &kind,
                        ReadOnly:    &readOnly,
                },
        }

And I had to restrict any pod that runs with a such in-line volume to a zone where the volume was created (the test creates the volume in framework.TestContext.CloudConfig.Zone).

--- a/test/e2e/storage/drivers/in_tree.go
+++ b/test/e2e/storage/drivers/in_tree.go
 func (a *azureDriver) CreateVolume(config *testsuites.PerTestConfig, volType testpatterns.TestVolType) testsuites.TestVolume {
        ginkgo.By("creating a test azure disk volume")
+       if volType == testpatterns.InlineVolume {
+               // Volume will be created in framework.TestContext.CloudConfig.Zone zone,
+               // so pods should be also scheduled there.
+               config.ClientNodeSelector = map[string]string{
+                       v1.LabelZoneFailureDomain: framework.TestContext.CloudConfig.Zone,
+               }
+       }
        volumeName, err := framework.CreatePDWithRetry()
        framework.ExpectNoError(err)
        return &azureVolume{

With these changes, I am able to run a test with in-line and pre-provisioned Azure volume.

@andyzhangx

This comment has been minimized.

Copy link
Member Author

commented Sep 4, 2019

great, shall I merge your code change? @jsafrane

@jsafrane

This comment has been minimized.

Copy link
Member

commented Sep 4, 2019

great, shall I merge your code change? @jsafrane

Yes, if it looks good for you

@k8s-ci-robot k8s-ci-robot added size/M and removed size/S labels Sep 4, 2019
@jsafrane

This comment has been minimized.

Copy link
Member

commented Sep 4, 2019

Thanks a lot for the PR!
/lgtm
/approve

@k8s-ci-robot k8s-ci-robot added the lgtm label Sep 4, 2019
@jsafrane

This comment has been minimized.

Copy link
Member

commented Sep 4, 2019

/assign @pohly @andrewsykim
for approval

@fejta

This comment has been minimized.

Copy link
Contributor

commented Sep 4, 2019

/retest

@andyzhangx

This comment has been minimized.

Copy link
Member Author

commented Sep 10, 2019

ping @pohly @andrewsykim thanks

@pohly

This comment has been minimized.

Copy link
Contributor

commented Sep 10, 2019

/approve

@k8s-ci-robot

This comment has been minimized.

Copy link
Contributor

commented Sep 10, 2019

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: andyzhangx, jsafrane, pohly

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot merged commit 411caaf into kubernetes:master Sep 11, 2019
24 checks passed
24 checks passed
cla/linuxfoundation andyzhangx authorized
Details
pull-kubernetes-bazel-build Job succeeded.
Details
pull-kubernetes-bazel-test Job succeeded.
Details
pull-kubernetes-conformance-image-test Skipped.
pull-kubernetes-conformance-kind-ipv6 Job succeeded.
Details
pull-kubernetes-cross Skipped.
pull-kubernetes-dependencies Job succeeded.
Details
pull-kubernetes-e2e-gce Job succeeded.
Details
pull-kubernetes-e2e-gce-100-performance Job succeeded.
Details
pull-kubernetes-e2e-gce-csi-serial Job succeeded.
Details
pull-kubernetes-e2e-gce-device-plugin-gpu Job succeeded.
Details
pull-kubernetes-e2e-gce-iscsi Skipped.
pull-kubernetes-e2e-gce-iscsi-serial Skipped.
pull-kubernetes-e2e-gce-storage-slow Job succeeded.
Details
pull-kubernetes-godeps Skipped.
pull-kubernetes-integration Job succeeded.
Details
pull-kubernetes-kubemark-e2e-gce-big Job succeeded.
Details
pull-kubernetes-local-e2e Skipped.
pull-kubernetes-node-e2e Job succeeded.
Details
pull-kubernetes-node-e2e-containerd Job succeeded.
Details
pull-kubernetes-typecheck Job succeeded.
Details
pull-kubernetes-verify Job succeeded.
Details
pull-publishing-bot-validate Skipped.
tide In merge pool.
Details
@k8s-ci-robot k8s-ci-robot added this to the v1.17 milestone Sep 11, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.