diff --git a/controllers/openstackserver_controller.go b/controllers/openstackserver_controller.go index 26494bfa49..d1023a2cef 100644 --- a/controllers/openstackserver_controller.go +++ b/controllers/openstackserver_controller.go @@ -39,6 +39,7 @@ import ( "sigs.k8s.io/cluster-api/util/annotations" v1beta1conditions "sigs.k8s.io/cluster-api/util/deprecated/v1beta1/conditions" "sigs.k8s.io/cluster-api/util/patch" + "sigs.k8s.io/cluster-api/util/predicates" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/builder" "sigs.k8s.io/controller-runtime/pkg/client" @@ -50,7 +51,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/reconcile" orcv1alpha1 "github.com/k-orc/openstack-resource-controller/v2/api/v1alpha1" - "github.com/k-orc/openstack-resource-controller/v2/pkg/predicates" + orcpredicates "github.com/k-orc/openstack-resource-controller/v2/pkg/predicates" infrav1alpha1 "sigs.k8s.io/cluster-api-provider-openstack/api/v1alpha1" infrav1 "sigs.k8s.io/cluster-api-provider-openstack/api/v1beta1" @@ -78,6 +79,7 @@ type OpenStackServerReconciler struct { // +kubebuilder:rbac:groups=infrastructure.cluster.x-k8s.io,resources=openstackservers,verbs=get;list;watch;create;update;patch;delete // +kubebuilder:rbac:groups=infrastructure.cluster.x-k8s.io,resources=openstackservers/status,verbs=get;update;patch +// +kubebuilder:rbac:groups=cluster.x-k8s.io,resources=clusters;clusters/status,verbs=get;list;watch // +kubebuilder:rbac:groups=ipam.cluster.x-k8s.io,resources=ipaddressclaims;ipaddressclaims/status,verbs=get;watch;create;update;patch;delete // +kubebuilder:rbac:groups=ipam.cluster.x-k8s.io,resources=ipaddresses;ipaddresses/status,verbs=get;list;watch // +kubebuilder:rbac:groups=openstack.k-orc.cloud,resources=images,verbs=get;list;watch @@ -218,7 +220,12 @@ func (r *OpenStackServerReconciler) SetupWithManager(ctx context.Context, mgr ct } return requests }), - builder.WithPredicates(predicates.NewBecameAvailable(mgr.GetLogger(), &orcv1alpha1.Image{})), + builder.WithPredicates(orcpredicates.NewBecameAvailable(mgr.GetLogger(), &orcv1alpha1.Image{})), + ). + Watches( + &clusterv1.Cluster{}, + handler.EnqueueRequestsFromMapFunc(r.requeueOpenStackServersForCluster(ctx)), + builder.WithPredicates(predicates.ClusterPausedTransitionsOrInfrastructureProvisioned(mgr.GetScheme(), log)), ). Watches( &ipamv1.IPAddressClaim{}, @@ -752,3 +759,50 @@ func OpenStackServerReconcileComplete(log logr.Logger) predicate.Funcs { GenericFunc: func(event.GenericEvent) bool { return false }, } } + +// requeueOpenStackServersForCluster returns a handler.MapFunc that watches for +// Cluster changes and triggers reconciliation of all OpenStackServers in that cluster. +func (r *OpenStackServerReconciler) requeueOpenStackServersForCluster(ctx context.Context) handler.MapFunc { + log := ctrl.LoggerFrom(ctx) + return func(ctx context.Context, o client.Object) []ctrl.Request { + c, ok := o.(*clusterv1.Cluster) + if !ok { + panic(fmt.Sprintf("Expected a Cluster but got a %T", o)) + } + + log := log.WithValues("objectMapper", "clusterToOpenStackServer", "namespace", c.Namespace, "cluster", c.Name) + + // Don't handle deleted clusters + if !c.DeletionTimestamp.IsZero() { + log.V(4).Info("Cluster has a deletion timestamp, skipping mapping.") + return nil + } + + // List all OpenStackServers in the cluster + serverList := &infrav1alpha1.OpenStackServerList{} + if err := r.Client.List( + ctx, + serverList, + client.InNamespace(c.Namespace), + client.MatchingLabels{clusterv1.ClusterNameLabel: c.Name}, + ); err != nil { + log.Error(err, "Failed to list OpenStackServers for cluster") + return nil + } + + // Create reconcile requests for all servers + requests := make([]ctrl.Request, 0, len(serverList.Items)) + for i := range serverList.Items { + server := &serverList.Items[i] + requests = append(requests, ctrl.Request{ + NamespacedName: client.ObjectKey{ + Namespace: server.Namespace, + Name: server.Name, + }, + }) + log.V(5).Info("Queueing OpenStackServer for reconciliation", "server", server.Name) + } + + return requests + } +} diff --git a/controllers/openstackserver_controller_test.go b/controllers/openstackserver_controller_test.go index 253264fb26..d2ca4953c5 100644 --- a/controllers/openstackserver_controller_test.go +++ b/controllers/openstackserver_controller_test.go @@ -17,6 +17,7 @@ limitations under the License. package controllers import ( + "context" "fmt" "reflect" "testing" @@ -33,9 +34,13 @@ import ( "go.uber.org/mock/gomock" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" "k8s.io/utils/ptr" clusterv1beta1 "sigs.k8s.io/cluster-api/api/core/v1beta1" + clusterv1 "sigs.k8s.io/cluster-api/api/core/v1beta2" "sigs.k8s.io/cluster-api/util/conditions" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/fake" infrav1alpha1 "sigs.k8s.io/cluster-api-provider-openstack/api/v1alpha1" infrav1 "sigs.k8s.io/cluster-api-provider-openstack/api/v1beta1" @@ -194,6 +199,191 @@ var deleteRootVolume = func(r *recorders) { r.volume.DeleteVolume(rootVolumeUUID, volumes.DeleteOpts{}).Return(nil) } +func TestOpenStackServerReconciler_requeueOpenStackServersForCluster(t *testing.T) { + tests := []struct { + name string + cluster *clusterv1.Cluster + servers []*infrav1alpha1.OpenStackServer + clusterDeleting bool + wantRequests int + wantServerNames []string + }{ + { + name: "returns reconcile requests for all servers in cluster", + cluster: &clusterv1.Cluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-cluster", + Namespace: "test-ns", + }, + }, + servers: []*infrav1alpha1.OpenStackServer{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "server-1", + Namespace: "test-ns", + Labels: map[string]string{ + clusterv1beta1.ClusterNameLabel: "test-cluster", + }, + }, + Spec: infrav1alpha1.OpenStackServerSpec{ + Flavor: ptr.To("m1.small"), + Image: infrav1.ImageParam{ + Filter: &infrav1.ImageFilter{Name: ptr.To("test-image")}, + }, + }, + }, + { + ObjectMeta: metav1.ObjectMeta{ + Name: "server-2", + Namespace: "test-ns", + Labels: map[string]string{ + clusterv1beta1.ClusterNameLabel: "test-cluster", + }, + }, + Spec: infrav1alpha1.OpenStackServerSpec{ + Flavor: ptr.To("m1.small"), + Image: infrav1.ImageParam{ + Filter: &infrav1.ImageFilter{Name: ptr.To("test-image")}, + }, + }, + }, + }, + wantRequests: 2, + wantServerNames: []string{"server-1", "server-2"}, + }, + { + name: "returns empty for deleted cluster", + cluster: &clusterv1.Cluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-cluster", + Namespace: "test-ns", + }, + }, + servers: []*infrav1alpha1.OpenStackServer{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "server-1", + Namespace: "test-ns", + Labels: map[string]string{ + clusterv1beta1.ClusterNameLabel: "test-cluster", + }, + }, + Spec: infrav1alpha1.OpenStackServerSpec{ + Flavor: ptr.To("m1.small"), + Image: infrav1.ImageParam{ + Filter: &infrav1.ImageFilter{Name: ptr.To("test-image")}, + }, + }, + }, + }, + clusterDeleting: true, + wantRequests: 0, + }, + { + name: "returns empty when no servers exist", + cluster: &clusterv1.Cluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-cluster", + Namespace: "test-ns", + }, + }, + servers: []*infrav1alpha1.OpenStackServer{}, + wantRequests: 0, + }, + { + name: "only returns servers from same cluster", + cluster: &clusterv1.Cluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-cluster", + Namespace: "test-ns", + }, + }, + servers: []*infrav1alpha1.OpenStackServer{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "server-1", + Namespace: "test-ns", + Labels: map[string]string{ + clusterv1beta1.ClusterNameLabel: "test-cluster", + }, + }, + Spec: infrav1alpha1.OpenStackServerSpec{ + Flavor: ptr.To("m1.small"), + Image: infrav1.ImageParam{ + Filter: &infrav1.ImageFilter{Name: ptr.To("test-image")}, + }, + }, + }, + { + ObjectMeta: metav1.ObjectMeta{ + Name: "server-2", + Namespace: "test-ns", + Labels: map[string]string{ + clusterv1beta1.ClusterNameLabel: "other-cluster", + }, + }, + Spec: infrav1alpha1.OpenStackServerSpec{ + Flavor: ptr.To("m1.small"), + Image: infrav1.ImageParam{ + Filter: &infrav1.ImageFilter{Name: ptr.To("test-image")}, + }, + }, + }, + }, + wantRequests: 1, + wantServerNames: []string{"server-1"}, + }, + } + + for i := range tests { + tt := &tests[i] + t.Run(tt.name, func(t *testing.T) { + g := NewGomegaWithT(t) + ctx := context.TODO() + + // Set deletion timestamp and finalizers if needed + if tt.clusterDeleting { + now := metav1.Now() + tt.cluster.DeletionTimestamp = &now + tt.cluster.Finalizers = []string{"test-finalizer"} + } + + // Create a fake client with the test data + scheme := runtime.NewScheme() + g.Expect(clusterv1.AddToScheme(scheme)).To(Succeed()) + g.Expect(infrav1alpha1.AddToScheme(scheme)).To(Succeed()) + + objs := []client.Object{tt.cluster} + for _, server := range tt.servers { + objs = append(objs, server) + } + + fakeClient := fake.NewClientBuilder().WithScheme(scheme).WithObjects(objs...).Build() + + // Create reconciler and call mapper function + reconciler := &OpenStackServerReconciler{ + Client: fakeClient, + } + mapFunc := reconciler.requeueOpenStackServersForCluster(ctx) + requests := mapFunc(ctx, tt.cluster) + + // Verify results + if tt.wantRequests == 0 { + g.Expect(requests).To(Or(BeNil(), BeEmpty())) + } else { + g.Expect(requests).To(HaveLen(tt.wantRequests)) + if len(tt.wantServerNames) > 0 { + gotNames := make([]string, len(requests)) + for i, req := range requests { + gotNames[i] = req.Name + } + g.Expect(gotNames).To(ConsistOf(tt.wantServerNames)) + } + } + }) + } +} + func TestOpenStackServer_serverToInstanceSpec(t *testing.T) { tests := []struct { name string