capacity: fix handling of topology changes with immediate binding SCs #475
Conversation
k8s-ci-robot
added
release-note
k8s-ci-robot
added
the
size/XS
|
/assign @msau42 I noticed this bug when trying out the steps in the upcoming blog post kubernetes/website#23102 I'll work around the bug in the example, therefore we don't need to hurry with getting a bug fix release out. |
This works around kubernetes-csi/external-provisioner#475 by not creating any storage classes with immediate binding.
| @@ -299,7 +299,7 @@ func (c *Controller) onTopologyChanges(added []*topology.Segment, removed []*top | |||
| continue | |||
| } | |||
| if !c.immediateBinding && sc.VolumeBindingMode != nil && *sc.VolumeBindingMode == storagev1.VolumeBindingImmediate { | |||
| return | |||
| continue | |||
There was a problem hiding this comment.
Can we add a unit test for this?
There was a problem hiding this comment.
We can, but it's not perfect: we need a specific ordering of storage classes (first the one with immediate binding, then the one with late binding), and there's no guarantee that the informer will list them in any particular order. But in practice, the informer seems to use "first in, first out", so I was able to trigger the problem.
But worse, onTopologyChanges wasn't tested at all. Fixed...
A copy-and-pasted "return" instead of "continue" caused processing to stop after encountering a storage class with immediate binding, which had the effect that further CSIStorageCapacity objects for other storage classes never got created. This only happened when topology changed at runtime. When topology was already known when the controller started, all expected objects got created.
pohly
force-pushed
the
storage-capacity
branch
from
86724ac to
9352d21
Compare
k8s-ci-robot
added
size/L
|
/lgtm Can you cherry-pick this to release-2.0 branch? |
k8s-ci-robot
added
the
lgtm
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: msau42, 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 |
k8s-ci-robot
added
the
approved
See PR #476 |
Update dependency go modules for k8s v1.28.0
What type of PR is this?
/kind bug
What this PR does / why we need it:
A copy-and-pasted "return" instead of "continue" caused processing to
stop after encountering a storage class with immediate binding, which
had the effect that further CSIStorageCapacity objects for other
storage classes never got created.
This only happened when topology changed at runtime. When topology
was already known when the controller started, all expected objects
got created.
Does this PR introduce a user-facing change?: