🐛 Check MachineConfig spec for full equality with Rke2CPSpec #325
Conversation
Danil-Grigorev
changed the title
Danil-Grigorev
force-pushed
the
re-rollout-cp-on-commands-change
branch
2 times, most recently
from
fdd44aa
to
2a7f097
Compare
Signed-off-by: Danil Grigorev <danil.grigorev@suse.com>
Danil-Grigorev
force-pushed
the
re-rollout-cp-on-commands-change
branch
from
2a7f097
to
da42f22
Compare
| m, ok := o.(*clusterv1.Machine) | ||
| if !ok { | ||
| panic(fmt.Sprintf("Expected a Machine but got a %T", o)) | ||
| } |
There was a problem hiding this comment.
Is this explicit check required? Isn't it guaranteed that the object the controller watches is of type clusterv1.Machine?
There was a problem hiding this comment.
Mostly just copied from kubeadm provider implementation at this point, but this should be guarantied in most of the cases, and is guarantied when using generic watches. Though the desired UX for using generic watches expects to replace all For/Owns methods with Watches. This is inconvenient and does not worth the effort. To eliminate the need for an explicit type casting, a set of helper methods can be introduced upstream to connect non generic controller with generic EventMappers of Predicates.
| c, ok := o.(*clusterv1.Cluster) | ||
| if !ok { | ||
| panic(fmt.Sprintf("Expected a Cluster but got a %T", o)) | ||
| } |
There was a problem hiding this comment.
Same question as with Machine.
What this PR does / why we need it:
We need to ensure that any change of
RKE2ConfigTemplateis causing CP machines re-rollout.Which issue(s) this PR fixes (optional, in
fixes #<issue number>(, fixes #<issue_number>, ...)format, will close the issue(s) when PR gets merged):Fixes #307
Special notes for your reviewer:
Checklist: