Support for wait while PV reattach #608

prashanth26 · 2021-05-17T10:09:56Z

Basic working for support for reattaching works with VolumeAttachments

What this PR does / why we need it:

Which issue(s) this PR fixes:
Fixes #249

Special notes for your reviewer:

Release note:

Draining of pods with PVs (Persistent Volume) now waits for re-attachment of PV on a different node when `volumeAttachments` support is enabled on the cluster. Else it falls back to the default PV reattachment timeout value configured. The default value is `90s` and this can be overwritten via the `machine-pv-reattach-timeout` flag. Please enable permissions to allow listing of `volumeAttachments` resource while importing these changes.

AxiomSamarth · 2021-05-20T05:51:00Z

pkg/util/provider/drain/drain.go

-		pvList := pvMap[i]
-		vols, err := o.getVolIDsFromDriver(pvList)
+	for podHash, persistantVolumeList := range pvMap {
+		persistantVolumeListDeepCopy := persistantVolumeList


Did you mean to call the DeepCopy() method in this assignment?

The value for persistantVolumeList is changing after every run. Hence I wanted to create a copy of the variable to use it for the object I am building here. I hope that looks okay?

prashanth26 · 2021-06-02T05:26:58Z

The PR is ready for review. Minor things for TODOs need to be fixed.
/invite @amshuman-kr

amshuman-kr

Thanks for the changes @prashanth26! I have some comments below. The main one about the one goroutine per pod.

pkg/util/backoff/wait_test.go

pkg/util/k8sutils/helper.go

pkg/util/provider/drain/drain.go

pkg/util/provider/drain/volume_attachment.go

pkg/util/provider/machinecontroller/controller.go

prashanth26 · 2021-06-09T03:27:03Z

/ok-to-test

- Draining of pods with PV (Persistent Volume) now waits for re-attachment of PV on a different node. - When volumeAttachments support is enabled on the cluster, it tracks volume attachments to determine this. - Else it falls back to the default PV reattachment timeout value configured. Default value is 3mins. Co-authored-by: Amshuman K R <amshuman.rao.karaya@sap.com>

prashanth26 · 2021-06-09T07:59:37Z

/ok-to-test

prashanth26 requested a review from a team as a code owner May 17, 2021 10:09

gardener-robot added needs/review Needs review size/xl Size of pull request is huge (see gardener-robot robot/bots/size.py) needs/second-opinion Needs second review by someone else labels May 17, 2021

gardener-robot-ci-2 added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label May 17, 2021

prashanth26 marked this pull request as draft May 17, 2021 10:11

gardener-robot-ci-3 added needs/ok-to-test Needs approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels May 17, 2021

prashanth26 force-pushed the serial-reattach branch from d72564e to bee5c04 Compare May 17, 2021 10:14

gardener-robot-ci-1 added reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels May 17, 2021

prashanth26 force-pushed the serial-reattach branch from bee5c04 to 48073d2 Compare May 17, 2021 10:19

gardener-robot-ci-2 added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label May 17, 2021

gardener-robot-ci-3 removed the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label May 17, 2021

prashanth26 force-pushed the serial-reattach branch from 48073d2 to 61d8781 Compare May 17, 2021 10:55

gardener-robot-ci-1 added reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels May 17, 2021

prashanth26 force-pushed the serial-reattach branch from 61d8781 to c99b715 Compare May 18, 2021 05:02

gardener-robot-ci-1 added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label May 18, 2021

gardener-robot-ci-3 removed the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label May 18, 2021

gardener-robot added the lifecycle/rotten Nobody worked on this for 12 months (final aging stage) label May 19, 2021

AxiomSamarth reviewed May 20, 2021

View reviewed changes

gardener-robot added lifecycle/stale Nobody worked on this for 6 months (will further age) and removed lifecycle/rotten Nobody worked on this for 12 months (final aging stage) labels May 21, 2021

gardener-robot added lifecycle/rotten Nobody worked on this for 12 months (final aging stage) and removed lifecycle/stale Nobody worked on this for 6 months (will further age) labels Jun 2, 2021

prashanth26 marked this pull request as ready for review June 2, 2021 05:25

gardener-robot requested a review from amshuman-kr June 2, 2021 05:27

amshuman-kr suggested changes Jun 3, 2021

View reviewed changes

prashanth26 force-pushed the serial-reattach branch from 4eb698e to ca09a31 Compare June 8, 2021 10:24

gardener-robot-ci-1 added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Jun 8, 2021

gardener-robot-ci-2 removed the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Jun 8, 2021

prashanth26 force-pushed the serial-reattach branch from ca09a31 to b0d61c9 Compare June 8, 2021 10:26

gardener-robot-ci-1 added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Jun 8, 2021

gardener-robot-ci-3 removed the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Jun 8, 2021

gardener-robot added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Jun 9, 2021

gardener-robot-ci-1 removed the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Jun 9, 2021

prashanth26 force-pushed the serial-reattach branch from b0d61c9 to d488fd5 Compare June 9, 2021 04:57

gardener-robot-ci-3 added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Jun 9, 2021

gardener-robot-ci-1 removed the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Jun 9, 2021

gardener-robot added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Jun 9, 2021

gardener-robot-ci-3 removed the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Jun 9, 2021

prashanth26 added reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed needs/ok-to-test Needs approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels Jun 9, 2021

gardener-robot-ci-3 added needs/ok-to-test Needs approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels Jun 9, 2021

prashanth26 merged commit c48f10d into gardener:master Jun 9, 2021

prashanth26 mentioned this pull request Jun 14, 2021

Wait until volume is attached on another node #561

Closed

prashanth26 deleted the serial-reattach branch June 16, 2021 10:43

This was referenced Jul 14, 2021

Updated MCM RBACs to support VolumeAttachments gardener/gardener-extension-provider-azure#339

Merged

Increased PV reattachment timeout to 150s gardener/gardener-extension-provider-azure#344

Merged

prashanth26 mentioned this pull request Jul 28, 2021

Updated MCM RBACs to support VolumeAttachments gardener/gardener-extension-provider-gcp#304

Merged

AxiomSamarth mentioned this pull request Aug 11, 2021

Updated MCM RBACs to support VolumeAttachments gardener/gardener-extension-provider-aws#388

Merged

kon-angelo mentioned this pull request Oct 4, 2021

Update MCM RBAC permissions for volumeattachments gardener/gardener-extension-provider-openstack#341

Merged

Gerrit91 mentioned this pull request Oct 19, 2021

Update dependencies, build with go-1.16, use mcm v0.40 metal-stack/machine-controller-manager-provider-metal#14

Merged

1 task

himanshu-kun mentioned this pull request Jul 20, 2023

Force Drain and detachment for Volumes for Unhealthy Nodes which were NotReady for over 5min #781

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support for wait while PV reattach #608

Support for wait while PV reattach #608

prashanth26 commented May 17, 2021 •

edited

AxiomSamarth May 20, 2021

prashanth26 Jun 3, 2021

prashanth26 commented Jun 2, 2021

amshuman-kr left a comment

prashanth26 commented Jun 9, 2021

prashanth26 commented Jun 9, 2021

Support for wait while PV reattach #608

Support for wait while PV reattach #608

Conversation

prashanth26 commented May 17, 2021 • edited

AxiomSamarth May 20, 2021

Choose a reason for hiding this comment

prashanth26 Jun 3, 2021

Choose a reason for hiding this comment

prashanth26 commented Jun 2, 2021

amshuman-kr left a comment

Choose a reason for hiding this comment

prashanth26 commented Jun 9, 2021

prashanth26 commented Jun 9, 2021

prashanth26 commented May 17, 2021 •

edited