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

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

@k8s-ci-robot k8s-ci-robot added release-note-none Denotes a PR that doesn't merit a release note. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. kind/failing-test Categorizes issue or PR as related to a consistently or frequently failing test. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. sig/cloud-provider Categorizes an issue or PR as relevant to SIG Cloud Provider. area/provider/azure Issues or PRs related to azure provider cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. area/e2e-test-framework Issues or PRs related to refactoring the kubernetes e2e test framework area/test sig/testing Categorizes an issue or PR as relevant to SIG Testing. labels Sep 4, 2019
@jsafrane
Copy link
Member

jsafrane 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
Copy link
Member Author

great, shall I merge your code change? @jsafrane

@jsafrane
Copy link
Member

jsafrane 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 Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Sep 4, 2019
@jsafrane
Copy link
Member

jsafrane commented Sep 4, 2019

Thanks a lot for the PR!
/lgtm
/approve

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Sep 4, 2019
@jsafrane
Copy link
Member

jsafrane commented Sep 4, 2019

/assign @pohly @andrewsykim
for approval

@fejta
Copy link
Contributor

fejta commented Sep 4, 2019

/retest

@andyzhangx
Copy link
Member Author

ping @pohly @andrewsykim thanks

@pohly
Copy link
Contributor

pohly commented Sep 10, 2019

/approve

@k8s-ci-robot
Copy link
Contributor

[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 added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Sep 10, 2019
@k8s-ci-robot k8s-ci-robot merged commit 411caaf into kubernetes:master Sep 11, 2019
@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
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. area/e2e-test-framework Issues or PRs related to refactoring the kubernetes e2e test framework area/provider/azure Issues or PRs related to azure provider area/test cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/failing-test Categorizes issue or PR as related to a consistently or frequently failing test. lgtm "Looks good to me", indicates that a PR is ready to be merged. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. release-note-none Denotes a PR that doesn't merit a release note. sig/cloud-provider Categorizes an issue or PR as relevant to SIG Cloud Provider. sig/testing Categorizes an issue or PR as relevant to SIG Testing. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Azure storage tests are not working
6 participants