diff --git a/bundle/manifests/odf-multicluster-orchestrator.clusterserviceversion.yaml b/bundle/manifests/odf-multicluster-orchestrator.clusterserviceversion.yaml index dda49127..3aa12f64 100644 --- a/bundle/manifests/odf-multicluster-orchestrator.clusterserviceversion.yaml +++ b/bundle/manifests/odf-multicluster-orchestrator.clusterserviceversion.yaml @@ -36,7 +36,7 @@ metadata: ] capabilities: Basic Install console.openshift.io/plugins: '["odf-multicluster-console"]' - createdAt: "2024-07-04T11:28:14Z" + createdAt: "2024-07-09T07:46:31Z" olm.skipRange: "" operators.openshift.io/infrastructure-features: '["disconnected"]' operators.operatorframework.io/builder: operator-sdk-v1.34.1 @@ -229,7 +229,11 @@ spec: resources: - managedclusterviews verbs: - - '*' + - create + - get + - list + - update + - watch - apiGroups: - work.open-cluster-management.io resources: diff --git a/config/rbac/role.yaml b/config/rbac/role.yaml index 329c7e81..efee82c3 100644 --- a/config/rbac/role.yaml +++ b/config/rbac/role.yaml @@ -166,7 +166,11 @@ rules: resources: - managedclusterviews verbs: - - '*' + - create + - get + - list + - update + - watch - apiGroups: - work.open-cluster-management.io resources: diff --git a/controllers/managedcluster_controller.go b/controllers/managedcluster_controller.go index 239951d6..07be02a0 100644 --- a/controllers/managedcluster_controller.go +++ b/controllers/managedcluster_controller.go @@ -2,7 +2,9 @@ package controllers import ( "context" + "fmt" "log/slog" + "reflect" "github.com/red-hat-storage/odf-multicluster-orchestrator/controllers/utils" viewv1beta1 "github.com/stolostron/multicloud-operators-foundation/pkg/apis/view/v1beta1" @@ -12,6 +14,7 @@ import ( ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/builder" "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/event" "sigs.k8s.io/controller-runtime/pkg/handler" "sigs.k8s.io/controller-runtime/pkg/predicate" "sigs.k8s.io/controller-runtime/pkg/reconcile" @@ -37,7 +40,7 @@ func (r *ManagedClusterReconciler) Reconcile(ctx context.Context, req reconcile. return ctrl.Result{}, client.IgnoreNotFound(err) } - if err := r.ensureManagedClusterViews(ctx, managedCluster.Name); err != nil { + if err := r.processManagedClusterViews(ctx, managedCluster.Name); err != nil { logger.Error("Failed to ensure ManagedClusterView", "error", err) return ctrl.Result{}, err } @@ -55,7 +58,7 @@ func (r *ManagedClusterReconciler) SetupWithManager(mgr ctrl.Manager) error { Watches( &viewv1beta1.ManagedClusterView{}, handler.EnqueueRequestsFromMapFunc(r.managedClusterViewRequestMapper), - builder.WithPredicates(predicate.ResourceVersionChangedPredicate{}, predicate.GenerationChangedPredicate{}), + builder.WithPredicates(managedClusterViewStatusChangePredicate, predicate.GenerationChangedPredicate{}), ). Complete(r) } @@ -64,17 +67,17 @@ func (r *ManagedClusterReconciler) managedClusterViewRequestMapper(ctx context.C mcv, ok := obj.(*viewv1beta1.ManagedClusterView) if !ok { - r.Logger.Error("Something unexpected occurred when casting obj into ManagedClusterView") + r.Logger.Error("Received object is not a ManagedClusterView object") return []reconcile.Request{} } - logger := r.Logger.With("ManagedClusterView", mcv) + logger := r.Logger.With("ManagedClusterView", mcv.Name) mcName, ok := mcv.Labels[utils.MCVLabelKey] if !ok { return []reconcile.Request{} } - logger.Info("ManagedClusterView of interest just got updated. Raising a reconcile request for ManagedCluster", "ManagedCluster", mcName) + logger.Debug("ManagedClusterView of interest just got updated. Raising a reconcile request for ManagedCluster", "ManagedCluster", mcName) return []reconcile.Request{ { @@ -85,39 +88,37 @@ func (r *ManagedClusterReconciler) managedClusterViewRequestMapper(ctx context.C } } -func (r *ManagedClusterReconciler) ensureManagedClusterViews(ctx context.Context, clusterName string) error { +func (r *ManagedClusterReconciler) processManagedClusterViews(ctx context.Context, clusterName string) error { namespace := "openshift-storage" resourceType := "ConfigMap" - r.Logger.Info("Processing PeerRef", "ClusterName", clusterName) - mcv, err := utils.GetManagedClusterViewByLabel(r.Client, utils.MCVLabelKey, clusterName, clusterName) + + mcv, or, err := utils.CreateOrUpdateManagedClusterView(ctx, r.Client, odfConfigMapName, namespace, resourceType, clusterName) if err != nil { - r.Logger.Info("ManagedClusterView does not exist, creating a new one") - mcv, err = utils.CreateOrUpdateManagedClusterView(ctx, r.Client, odfConfigMapName, namespace, resourceType, clusterName) - if err != nil { - r.Logger.Error("Failed to create or update ManagedClusterView", "error", err) - return err - } - r.Logger.Info("ManagedClusterView created or updated successfully", "ManagedClusterView", mcv.Name) - } else { - desiredSpec := viewv1beta1.ViewSpec{ - Scope: viewv1beta1.ViewScope{ - Name: odfConfigMapName, - Namespace: namespace, - Resource: resourceType, - }, - } - if mcv.Spec != desiredSpec { - r.Logger.Info("ManagedClusterView spec does not match, updating", "ClusterName", clusterName) - mcv, err = utils.CreateOrUpdateManagedClusterView(ctx, r.Client, odfConfigMapName, namespace, resourceType, clusterName) - if err != nil { - r.Logger.Error("Failed to update ManagedClusterView", "error", err) - return err - } - r.Logger.Info("ManagedClusterView updated successfully", "ManagedClusterView", mcv.Name) - } else { - r.Logger.Info("ManagedClusterView spec matches, no update needed", "ManagedClusterView", mcv.Name) - } + r.Logger.Error("Failed to create or update ManagedClusterView", "error", err) + return err } + r.Logger.Info(fmt.Sprintf("ManagedClusterView was %s", or), "ManagedClusterView", mcv.Name) return nil } + +var managedClusterViewStatusChangePredicate = predicate.Funcs{ + UpdateFunc: func(e event.UpdateEvent) bool { + oldObj, okOld := e.ObjectOld.(*viewv1beta1.ManagedClusterView) + newObj, okNew := e.ObjectNew.(*viewv1beta1.ManagedClusterView) + if !okOld || !okNew { + return false + } + + return !reflect.DeepEqual(oldObj.Status, newObj.Status) + }, + CreateFunc: func(e event.CreateEvent) bool { + return true + }, + DeleteFunc: func(e event.DeleteEvent) bool { + return true + }, + GenericFunc: func(e event.GenericEvent) bool { + return true + }, +} diff --git a/controllers/managedcluster_controller_test.go b/controllers/managedcluster_controller_test.go index 089f6905..145e89c5 100644 --- a/controllers/managedcluster_controller_test.go +++ b/controllers/managedcluster_controller_test.go @@ -56,7 +56,7 @@ func TestManagedClusterReconcile(t *testing.T) { }) } -func TestEnsureManagedClusterViews(t *testing.T) { +func TestProcessManagedClusterViews(t *testing.T) { scheme := runtime.NewScheme() _ = clusterv1.AddToScheme(scheme) _ = viewv1beta1.AddToScheme(scheme) @@ -71,13 +71,13 @@ func TestEnsureManagedClusterViews(t *testing.T) { } t.Run("ManagedClusterView does not exist. It should create it", func(t *testing.T) { - err := reconciler.ensureManagedClusterViews(context.TODO(), "test-cluster") + err := reconciler.processManagedClusterViews(context.TODO(), "test-cluster") assert.NoError(t, err) createdMCV := &viewv1beta1.ManagedClusterView{} - err = client.Get(context.TODO(), types.NamespacedName{Name: "test-cluster-mcv", Namespace: "test-cluster"}, createdMCV) + err = client.Get(context.TODO(), types.NamespacedName{Name: utils.GetManagedClusterViewName("test-cluster"), Namespace: "test-cluster"}, createdMCV) assert.NoError(t, err) - assert.Equal(t, "test-cluster-mcv", createdMCV.Name) + assert.Equal(t, utils.GetManagedClusterViewName("test-cluster"), createdMCV.Name) assert.Equal(t, "test-cluster", createdMCV.Namespace) assert.Equal(t, "odf-info", createdMCV.Spec.Scope.Name) assert.Equal(t, "openshift-storage", createdMCV.Spec.Scope.Namespace) @@ -86,7 +86,7 @@ func TestEnsureManagedClusterViews(t *testing.T) { t.Run("ManagedClusterView exists but spec does not match. Desired spec should be reached", func(t *testing.T) { existingMCV := &viewv1beta1.ManagedClusterView{ ObjectMeta: metav1.ObjectMeta{ - Name: "test-cluster-mcv", + Name: utils.GetManagedClusterViewName("test-cluster"), Namespace: "test-cluster", }, Spec: viewv1beta1.ViewSpec{ @@ -99,11 +99,11 @@ func TestEnsureManagedClusterViews(t *testing.T) { } _ = client.Create(context.TODO(), existingMCV) - err := reconciler.ensureManagedClusterViews(context.TODO(), "test-cluster") + err := reconciler.processManagedClusterViews(context.TODO(), "test-cluster") assert.NoError(t, err) updatedMCV := &viewv1beta1.ManagedClusterView{} - _ = client.Get(context.TODO(), types.NamespacedName{Name: "test-cluster-mcv", Namespace: "test-cluster"}, updatedMCV) + _ = client.Get(context.TODO(), types.NamespacedName{Name: utils.GetManagedClusterViewName("test-cluster"), Namespace: "test-cluster"}, updatedMCV) assert.Equal(t, "odf-info", updatedMCV.Spec.Scope.Name) assert.Equal(t, "openshift-storage", updatedMCV.Spec.Scope.Namespace) }) @@ -111,7 +111,7 @@ func TestEnsureManagedClusterViews(t *testing.T) { t.Run("ManagedClusterView exists and spec matches. Nothing should change. It should be error free", func(t *testing.T) { existingMCV := &viewv1beta1.ManagedClusterView{ ObjectMeta: metav1.ObjectMeta{ - Name: "test-cluster-mcv", + Name: utils.GetManagedClusterViewName("test-cluster"), Namespace: "test-cluster", }, Spec: viewv1beta1.ViewSpec{ @@ -124,7 +124,7 @@ func TestEnsureManagedClusterViews(t *testing.T) { } _ = client.Create(context.TODO(), existingMCV) - err := reconciler.ensureManagedClusterViews(context.TODO(), "test-cluster") + err := reconciler.processManagedClusterViews(context.TODO(), "test-cluster") assert.NoError(t, err) }) } diff --git a/controllers/mirrorpeer_controller.go b/controllers/mirrorpeer_controller.go index f59a3123..fa3d83a0 100644 --- a/controllers/mirrorpeer_controller.go +++ b/controllers/mirrorpeer_controller.go @@ -70,8 +70,8 @@ const spokeClusterRoleBindingName = "spoke-clusterrole-bindings" //+kubebuilder:rbac:groups=addon.open-cluster-management.io,resources=managedclusteraddons;clustermanagementaddons,verbs=* //+kubebuilder:rbac:groups=addon.open-cluster-management.io,resources=managedclusteraddons/status,verbs=* //+kubebuilder:rbac:groups=apps,resources=deployments,verbs=get;list;create;update;watch -//+kubebuilder:rbac:groups=view.open-cluster-management.io,resources=managedclusterviews,verbs=* //+kubebuilder:rbac:groups=console.openshift.io,resources=consoleplugins,verbs=get;list;create;update;watch +// +kubebuilder:rbac:groups=view.open-cluster-management.io,resources=managedclusterviews,verbs=get;list;watch;create;update //+kubebuilder:rbac:groups=core,resources=services,verbs=get;list;create;update;watch //+kubebuilder:rbac:groups=ramendr.openshift.io,resources=drclusters;drpolicies,verbs=get;list;create;update;watch //+kubebuilder:rbac:groups=rbac.authorization.k8s.io,resources=clusterrolebindings,verbs=get;list;create;update;delete;watch,resourceNames=spoke-clusterrole-bindings diff --git a/controllers/utils/acm.go b/controllers/utils/acm.go index 4405831e..b98c549d 100644 --- a/controllers/utils/acm.go +++ b/controllers/utils/acm.go @@ -12,16 +12,21 @@ import ( ) const MCVLabelKey = "multicluster.odf.openshift.io/cluster" +const MCVNameTemplate = "odf-multicluster-mcv-%s" -func CreateOrUpdateManagedClusterView(ctx context.Context, client ctrlClient.Client, resourceToFindName string, resourceToFindNamespace string, resourceToFindType string, clusterName string) (*viewv1beta1.ManagedClusterView, error) { +func GetManagedClusterViewName(clusterName string) string { + return fmt.Sprintf(MCVNameTemplate, clusterName) +} + +func CreateOrUpdateManagedClusterView(ctx context.Context, client ctrlClient.Client, resourceToFindName string, resourceToFindNamespace string, resourceToFindType string, clusterName string) (*viewv1beta1.ManagedClusterView, controllerutil.OperationResult, error) { mcv := &viewv1beta1.ManagedClusterView{ ObjectMeta: metav1.ObjectMeta{ - Name: fmt.Sprintf("%s-mcv", clusterName), + Name: GetManagedClusterViewName(clusterName), Namespace: clusterName, }, } - _, err := controllerutil.CreateOrUpdate(ctx, client, mcv, func() error { + or, err := controllerutil.CreateOrUpdate(ctx, client, mcv, func() error { mcv.Spec = viewv1beta1.ViewSpec{ Scope: viewv1beta1.ViewScope{ Name: resourceToFindName, @@ -40,10 +45,10 @@ func CreateOrUpdateManagedClusterView(ctx context.Context, client ctrlClient.Cli }) if err != nil { - return nil, fmt.Errorf("failed to create or update ManagedClusterView: %w", err) + return nil, controllerutil.OperationResultNone, fmt.Errorf("failed to create or update ManagedClusterView. %w", err) } - return mcv, nil + return mcv, or, nil } func GetManagedClusterViewByLabel(client ctrlClient.Client, labelKey, labelValue, namespace string) (*viewv1beta1.ManagedClusterView, error) { @@ -55,7 +60,7 @@ func GetManagedClusterViewByLabel(client ctrlClient.Client, labelKey, labelValue err := client.List(context.TODO(), mcvList, listOptions) if err != nil { - return nil, fmt.Errorf("failed to list ManagedClusterViews: %w", err) + return nil, fmt.Errorf("failed to list ManagedClusterViews. %w", err) } if len(mcvList.Items) == 0 { @@ -66,7 +71,5 @@ func GetManagedClusterViewByLabel(client ctrlClient.Client, labelKey, labelValue } func getLabelSelector(labelKey, labelValue string) labels.Selector { - return labels.SelectorFromSet(labels.Set{ - labelKey: labelValue, - }) + return labels.SelectorFromSet(labels.Set{labelKey: labelValue}) } diff --git a/controllers/utils/acm_test.go b/controllers/utils/acm_test.go index 2b2a74d9..d0bb861f 100644 --- a/controllers/utils/acm_test.go +++ b/controllers/utils/acm_test.go @@ -18,10 +18,10 @@ func TestCreateOrUpdateManagedClusterView(t *testing.T) { client := fake.NewClientBuilder().WithScheme(s).Build() t.Run("Success", func(t *testing.T) { - mcv, err := CreateOrUpdateManagedClusterView(context.TODO(), client, "example-configmap", "default", "ConfigMap", "managed-cluster-1") + mcv, _, err := CreateOrUpdateManagedClusterView(context.TODO(), client, "example-configmap", "default", "ConfigMap", "managed-cluster-1") assert.NoError(t, err) assert.NotNil(t, mcv) - assert.Equal(t, "managed-cluster-1-mcv", mcv.Name) + assert.Equal(t, GetManagedClusterViewName("managed-cluster-1"), mcv.Name) assert.Equal(t, "example-configmap", mcv.Spec.Scope.Name) assert.Equal(t, "default", mcv.Spec.Scope.Namespace) assert.Equal(t, "ConfigMap", mcv.Spec.Scope.Resource)