Skip to content
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

🐛 Check MachineConfig spec for full equality with Rke2CPSpec #325

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
57 changes: 57 additions & 0 deletions bootstrap/internal/controllers/rke2config_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ import (
"k8s.io/utils/ptr"
ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/handler"
"sigs.k8s.io/controller-runtime/pkg/log"
kubeyaml "sigs.k8s.io/yaml"

Expand Down Expand Up @@ -290,9 +291,65 @@ func (r *RKE2ConfigReconciler) SetupWithManager(mgr ctrl.Manager) error {

return ctrl.NewControllerManagedBy(mgr).
For(&bootstrapv1.RKE2Config{}).
Watches(
&clusterv1.Machine{},
handler.EnqueueRequestsFromMapFunc(r.MachineToBootstrapMapFunc),
).
Watches(
&clusterv1.Cluster{},
handler.EnqueueRequestsFromMapFunc(r.ClusterToRKE2Configs),
).
Complete(r)
}

// MachineToBootstrapMapFunc is a handler.ToRequestsFunc to be used to enqueue
// request for reconciliation of RKE2Config.
func (r *RKE2ConfigReconciler) MachineToBootstrapMapFunc(_ context.Context, o client.Object) []ctrl.Request {
m, ok := o.(*clusterv1.Machine)
if !ok {
panic(fmt.Sprintf("Expected a Machine but got a %T", o))
}
Comment on lines +308 to +311
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this explicit check required? Isn't it guaranteed that the object the controller watches is of type clusterv1.Machine?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.


result := []ctrl.Request{}

if m.Spec.Bootstrap.ConfigRef != nil && m.Spec.Bootstrap.ConfigRef.GroupVersionKind() == bootstrapv1.GroupVersion.WithKind("RKE2Config") {
name := client.ObjectKey{Namespace: m.Namespace, Name: m.Spec.Bootstrap.ConfigRef.Name}
result = append(result, ctrl.Request{NamespacedName: name})
}

return result
}

// ClusterToRKE2Configs is a handler.ToRequestsFunc to be used to enqueue
// requests for reconciliation of RKE2Configs.
func (r *RKE2ConfigReconciler) ClusterToRKE2Configs(ctx context.Context, o client.Object) []ctrl.Request {
result := []ctrl.Request{}

c, ok := o.(*clusterv1.Cluster)
if !ok {
panic(fmt.Sprintf("Expected a Cluster but got a %T", o))
}
Comment on lines +328 to +331
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same question as with Machine.


selectors := []client.ListOption{
client.InNamespace(c.Namespace),
client.MatchingLabels{
clusterv1.ClusterNameLabel: c.Name,
},
}

machineList := &clusterv1.MachineList{}
if err := r.Client.List(ctx, machineList, selectors...); err != nil {
return nil
}

for _, m := range machineList.Items {
m := m
result = append(result, r.MachineToBootstrapMapFunc(ctx, &m)...)
}

return result
}

// handleClusterNotInitialized handles the first control plane node.
func (r *RKE2ConfigReconciler) handleClusterNotInitialized(ctx context.Context, scope *Scope) (res ctrl.Result, reterr error) { //nolint:funlen
if !scope.HasControlPlaneOwner {
Expand Down
2 changes: 1 addition & 1 deletion pkg/rke2/machine_filters.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ func matchesRKE2BootstrapConfig(machineConfigs map[string]*bootstrapv1.RKE2Confi
}

// Check if RCP AgentConfig and machineBootstrapConfig matches
return reflect.DeepEqual(machineConfig.Spec.AgentConfig, rcp.Spec.AgentConfig)
return reflect.DeepEqual(machineConfig.Spec, rcp.Spec.RKE2ConfigSpec)
}
}

Expand Down
48 changes: 48 additions & 0 deletions pkg/rke2/machine_filters_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,54 @@ var _ = Describe("matchAgentConfig", func() {
Expect(matches.Oldest().Name).To(Equal("machine-test"))
},
)

It("shouldn't match Agent Config and different preBootstrapCommands", func() {
machineConfigs := map[string]*bootstrapv1.RKE2Config{
"someMachine": {},
"machine-test": {
ObjectMeta: v1.ObjectMeta{
Name: "rke2-config-example",
Namespace: "example",
},
Spec: bootstrapv1.RKE2ConfigSpec{
AgentConfig: bootstrapv1.RKE2AgentConfig{
NodeLabels: []string{"hello=world"},
},
PreRKE2Commands: []string{"test"},
},
},
}
machineCollection := collections.FromMachines(&machine)
Expect(len(machineCollection)).To(Equal(1))
matches := machineCollection.AnyFilter(matchesRKE2BootstrapConfig(machineConfigs, &rcp))

Expect(len(matches)).To(Equal(0))
},
)

It("shouldn't match Agent Config and different postBootstrapCommands", func() {
machineConfigs := map[string]*bootstrapv1.RKE2Config{
"someMachine": {},
"machine-test": {
ObjectMeta: v1.ObjectMeta{
Name: "rke2-config-example",
Namespace: "example",
},
Spec: bootstrapv1.RKE2ConfigSpec{
AgentConfig: bootstrapv1.RKE2AgentConfig{
NodeLabels: []string{"hello=world"},
},
PostRKE2Commands: []string{"test"},
},
},
}
machineCollection := collections.FromMachines(&machine)
Expect(len(machineCollection)).To(Equal(1))
matches := machineCollection.AnyFilter(matchesRKE2BootstrapConfig(machineConfigs, &rcp))

Expect(len(matches)).To(Equal(0))
},
)
})

var _ = Describe("matching Kubernetes Version", func() {
Expand Down
Loading