-
Notifications
You must be signed in to change notification settings - Fork 288
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
Enable taints support in snow #2327
Conversation
Codecov Report
@@ Coverage Diff @@
## main #2327 +/- ##
==========================================
+ Coverage 56.23% 56.33% +0.10%
==========================================
Files 304 305 +1
Lines 24676 24740 +64
==========================================
+ Hits 13876 13937 +61
- Misses 9507 9509 +2
- Partials 1293 1294 +1
Continue to review full report at Codecov.
|
pkg/providers/snow/objects.go
Outdated
} | ||
|
||
func recreateKubeadmConfigTemplateNeeded(new, old *bootstrapv1.KubeadmConfigTemplate) bool { | ||
return !v1alpha1.TaintsSliceEqual(new.Spec.Template.Spec.JoinConfiguration.NodeRegistration.Taints, old.Spec.Template.Spec.JoinConfiguration.NodeRegistration.Taints) |
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.
can we just compare new.Spec
and old.Spec
? That would make this a bit more future proof
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.
not all fields in spec trigger machine recreate, meaning no new kubeadmconfigtemplate created
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.
chatted w folks offline -- we are treating kubeadmconfigtemplate immutable. Refer: #189 (comment)
# Conflicts: # pkg/clusterapi/apibuilder.go # pkg/clusterapi/apibuilder_test.go # pkg/clusterapi/identity_test.go
[APPROVALNOTIFIER] This PR is APPROVED Approval requirements bypassed by manually added approval. This pull-request has been approved by: 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 |
Issue #, if available:
#1574
Description of changes:
Add taints support for snow provider. Need to recreate
KubeadmConfigTemplate
andSnowMachineTemplate
whenever worker node group's taints are updated.Testing (if applicable):
unit tests
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.