Queue resyncs in ShouldEnqueueContentChange
#1302
Conversation
Signed-off-by: Connor Catlett <[email protected]>
k8s-ci-robot
added
release-note
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: ConnorJC3 The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
k8s-ci-robot
added
the
size/S
|
/lgtm |
k8s-ci-robot
added
the
lgtm
|
looks great! I think it makes more sense than #1259 |
|
@akalenyu: changing LGTM is restricted to collaborators In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
| @@ -693,7 +697,7 @@ func ShouldEnqueueContentChange(old *crdv1.VolumeSnapshotContent, new *crdv1.Vol | |||
| if sanitized.Annotations == nil { | |||
| sanitized.Annotations = map[string]string{} | |||
| } | |||
| for annotation, _ := range sidecarControlledContentAnnotations { | |||
| for annotation := range sidecarControlledContentAnnotations { | |||
There was a problem hiding this comment.
This change (and the similar one below) was caused by gofmt
What type of PR is this?
/kind bug
What this PR does / why we need it:
Resyncs are sent to an informer's update handler via a no-diff (i.e.
newandoldare identical) update. Currently,ShouldEnqueueContentChangewill incorrectly returnfalsein this case, as it detects no change.We can reliably determine if an update is caused by a resync by comparing the resource versions, as any normal update will cause these to differ. This is also how the
external-attacherhandles this case in its similarshouldEnqueueVAChange: https://github.com/kubernetes-csi/external-attacher/blob/c8f0c14a7d985c33d8fe80a55ebb46a1ee3df877/pkg/controller/controller.go#L288-L291Which issue(s) this PR fixes:
Partial fix of #1258
Special notes for your reviewer:
N/A
Does this PR introduce a user-facing change?: