-
Notifications
You must be signed in to change notification settings - Fork 61
🌱 Add annotation to skip registering unmanaged volumes #1431
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
🌱 Add annotation to skip registering unmanaged volumes #1431
Conversation
When present, this annotation will cause the reconcile to exit early and update the condition to indicate that the annotation has been seen. This is useful for an external operation that needs to either prevent registration, or perform an unregister once disks are registered.
f1e75eb to
a5ec26c
Compare
Minimum allowed line rate is |
|
|
||
| // NoUnmanagedVolumesRegisterAnnotationKey is the annotation to not create any CNSRegisterVolumes | ||
| // by skipping the unmanaged volume register reconcile. | ||
| NoUnmanagedVolumesRegisterAnnotationKey = "vmoperator.vmware.com/no-unmanaged-volumes-register" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
personally not a fan of the no prefix. what do you think of one of those?
disallow-register-unmanaged-volumesdisallow-unmanaged-volumes-registerprevent-unmanaged-volumes-registerprevent-register-unmanaged-volumes
| logger := pkglog.FromContextOrDefault(ctx) | ||
|
|
||
| if _, ok := vm.Annotations[pkgconst.NoUnmanagedVolumesRegisterAnnotationKey]; ok { | ||
| logger.Info("Skipping register unmanaged volumes because of annotation") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should we log the annotation here?
| cond := pkgcond.Get(vm, unmanagedvolsreg.Condition) | ||
| Expect(cond).ToNot(BeNil()) | ||
| Expect(cond.Status).To(Equal(metav1.ConditionFalse)) | ||
| Expect(cond.Reason).To(Equal("NoUnmanagedVolumesRegisterAnnotation")) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
shouldn't we have a assertion that we didn't register any volumes
What does this PR do, and why is it needed?
When present, this annotation will cause the reconcile to exit early and update the condition to indicate that the annotation has been seen. This is useful for an external operation that needs to either prevent registration, or perform an unregister once disks are registered.
Which issue(s) is/are addressed by this PR? (optional, in
fixes #<issue number>(, fixes #<issue_number>, ...)format, will close the issue(s) when PR gets merged):Fixes #
Are there any special notes for your reviewer:
Please add a release note if necessary: