diff --git a/pkg/controllers/rollout/controller.go b/pkg/controllers/rollout/controller.go index 464ce37cf..f91c43f56 100644 --- a/pkg/controllers/rollout/controller.go +++ b/pkg/controllers/rollout/controller.go @@ -626,6 +626,26 @@ func (r *Reconciler) SetupWithManager(mgr runtime.Manager) error { handleResourceSnapshot(e.Object, q) }, }). + Watches(&fleetv1alpha1.ClusterResourceOverrideSnapshot{}, handler.Funcs{ + CreateFunc: func(ctx context.Context, e event.CreateEvent, q workqueue.RateLimitingInterface) { + klog.V(2).InfoS("Handling a clusterResourceOverrideSnapshot create event", "clusterResourceOverrideSnapshot", klog.KObj(e.Object)) + handleClusterResourceOverrideSnapshot(e.Object, q) + }, + GenericFunc: func(ctx context.Context, e event.GenericEvent, q workqueue.RateLimitingInterface) { + klog.V(2).InfoS("Handling a clusterResourceOverrideSnapshot generic event", "clusterResourceOverrideSnapshot", klog.KObj(e.Object)) + handleClusterResourceOverrideSnapshot(e.Object, q) + }, + }). + Watches(&fleetv1alpha1.ResourceOverrideSnapshot{}, handler.Funcs{ + CreateFunc: func(ctx context.Context, e event.CreateEvent, q workqueue.RateLimitingInterface) { + klog.V(2).InfoS("Handling a resourceOverrideSnapshot create event", "resourceOverrideSnapshot", klog.KObj(e.Object)) + handleResourceOverrideSnapshot(e.Object, q) + }, + GenericFunc: func(ctx context.Context, e event.GenericEvent, q workqueue.RateLimitingInterface) { + klog.V(2).InfoS("Handling a resourceOverrideSnapshot generic event", "resourceOverrideSnapshot", klog.KObj(e.Object)) + handleResourceOverrideSnapshot(e.Object, q) + }, + }). Watches(&fleetv1beta1.ClusterResourceBinding{}, handler.Funcs{ CreateFunc: func(ctx context.Context, e event.CreateEvent, q workqueue.RateLimitingInterface) { klog.V(2).InfoS("Handling a resourceBinding create event", "resourceBinding", klog.KObj(e.Object)) @@ -643,6 +663,72 @@ func (r *Reconciler) SetupWithManager(mgr runtime.Manager) error { Complete(r) } +// handleClusterResourceOverrideSnapshot parse the clusterResourceOverrideSnapshot label and enqueue the CRP name associated +// with the clusterResourceOverrideSnapshot if set. +func handleClusterResourceOverrideSnapshot(o client.Object, q workqueue.RateLimitingInterface) { + snapshot, ok := o.(*fleetv1alpha1.ClusterResourceOverrideSnapshot) + if !ok { + klog.ErrorS(controller.NewUnexpectedBehaviorError(fmt.Errorf("non ClusterResourceOverrideSnapshot type resource: %+v", o)), + "Rollout controller received invalid ClusterResourceOverrideSnapshot event", "object", klog.KObj(o)) + return + } + + snapshotKRef := klog.KObj(snapshot) + // check if it is the latest resource resourceBinding + isLatest, err := strconv.ParseBool(snapshot.GetLabels()[fleetv1beta1.IsLatestSnapshotLabel]) + if err != nil { + klog.ErrorS(controller.NewUnexpectedBehaviorError(fmt.Errorf("invalid annotation value %s : %w", fleetv1beta1.IsLatestSnapshotLabel, err)), + "Resource clusterResourceOverrideSnapshot has does not have a valid islatest label", "clusterResourceOverrideSnapshot", snapshotKRef) + return + } + if !isLatest { + // All newly created resource snapshots should start with the latest label to be true. + // However, this can happen if the label is removed fast by the time this reconcile loop is triggered. + klog.V(2).InfoS("Newly created resource clusterResourceOverrideSnapshot %s is not the latest", "clusterResourceOverrideSnapshot", snapshotKRef) + return + } + if snapshot.Spec.OverrideSpec.Placement == nil { + return + } + // enqueue the CRP to the rollout controller queue + q.Add(reconcile.Request{ + NamespacedName: types.NamespacedName{Name: snapshot.Spec.OverrideSpec.Placement.Name}, + }) +} + +// handleResourceOverrideSnapshot parse the resourceOverrideSnapshot label and enqueue the CRP name associated with the +// resourceOverrideSnapshot if set. +func handleResourceOverrideSnapshot(o client.Object, q workqueue.RateLimitingInterface) { + snapshot, ok := o.(*fleetv1alpha1.ResourceOverrideSnapshot) + if !ok { + klog.ErrorS(controller.NewUnexpectedBehaviorError(fmt.Errorf("non ResourceOverrideSnapshot type resource: %+v", o)), + "Rollout controller received invalid ResourceOverrideSnapshot event", "object", klog.KObj(o)) + return + } + + snapshotKRef := klog.KObj(snapshot) + // check if it is the latest resource resourceBinding + isLatest, err := strconv.ParseBool(snapshot.GetLabels()[fleetv1beta1.IsLatestSnapshotLabel]) + if err != nil { + klog.ErrorS(controller.NewUnexpectedBehaviorError(fmt.Errorf("invalid annotation value %s : %w", fleetv1beta1.IsLatestSnapshotLabel, err)), + "Resource resourceOverrideSnapshot has does not have a valid islatest annotation", "resourceOverrideSnapshot", snapshotKRef) + return + } + if !isLatest { + // All newly created resource snapshots should start with the latest label to be true. + // However, this can happen if the label is removed fast by the time this reconcile loop is triggered. + klog.V(2).InfoS("Newly created resource resourceOverrideSnapshot %s is not the latest", "resourceOverrideSnapshot", snapshotKRef) + return + } + if snapshot.Spec.OverrideSpec.Placement == nil { + return + } + // enqueue the CRP to the rollout controller queue + q.Add(reconcile.Request{ + NamespacedName: types.NamespacedName{Name: snapshot.Spec.OverrideSpec.Placement.Name}, + }) +} + // handleResourceSnapshot parse the resourceBinding label and annotation and enqueue the CRP name associated with the resource resourceBinding func handleResourceSnapshot(snapshot client.Object, q workqueue.RateLimitingInterface) { snapshotKRef := klog.KObj(snapshot) @@ -656,14 +742,14 @@ func handleResourceSnapshot(snapshot client.Object, q workqueue.RateLimitingInte // check if it is the latest resource resourceBinding isLatest, err := strconv.ParseBool(snapshot.GetLabels()[fleetv1beta1.IsLatestSnapshotLabel]) if err != nil { - klog.ErrorS(controller.NewUnexpectedBehaviorError(fmt.Errorf("invalid annotation value %s : %w", fleetv1beta1.IsLatestSnapshotLabel, err)), - "Resource resourceBinding has does not have a valid islatest annotation", "clusterResourceSnapshot", snapshotKRef) + klog.ErrorS(controller.NewUnexpectedBehaviorError(fmt.Errorf("invalid label value %s : %w", fleetv1beta1.IsLatestSnapshotLabel, err)), + "Resource clusterResourceSnapshot has does not have a valid islatest annotation", "clusterResourceSnapshot", snapshotKRef) return } if !isLatest { // All newly created resource snapshots should start with the latest label to be true. // However, this can happen if the label is removed fast by the time this reconcile loop is triggered. - klog.V(2).InfoS("Newly created resource resourceBinding %s is not the latest", "clusterResourceSnapshot", snapshotKRef) + klog.V(2).InfoS("Newly created resource clusterResourceSnapshot %s is not the latest", "clusterResourceSnapshot", snapshotKRef) return } // get the CRP name from the label diff --git a/test/e2e/placement_cro_test.go b/test/e2e/placement_cro_test.go index e89a30b25..09bd07c4a 100644 --- a/test/e2e/placement_cro_test.go +++ b/test/e2e/placement_cro_test.go @@ -93,6 +93,54 @@ var _ = Describe("creating clusterResourceOverride (selecting all clusters) to o want := map[string]string{croTestAnnotationKey: croTestAnnotationValue} checkIfOverrideAnnotationsOnAllMemberClusters(true, want) }) + + It("update cro attached to this CRP only and annotation value", func() { + Eventually(func() error { + cro := &placementv1alpha1.ClusterResourceOverride{} + if err := hubClient.Get(ctx, types.NamespacedName{Name: croName}, cro); err != nil { + return err + } + cro.Spec = placementv1alpha1.ClusterResourceOverrideSpec{ + Placement: &placementv1alpha1.PlacementRef{ + Name: crpName, // assigned CRP name + }, + ClusterResourceSelectors: workResourceSelector(), + Policy: &placementv1alpha1.OverridePolicy{ + OverrideRules: []placementv1alpha1.OverrideRule{ + { + ClusterSelector: &placementv1beta1.ClusterSelector{ + ClusterSelectorTerms: []placementv1beta1.ClusterSelectorTerm{}, + }, + JSONPatchOverrides: []placementv1alpha1.JSONPatchOverride{ + { + Operator: placementv1alpha1.JSONPatchOverrideOpAdd, + Path: "/metadata/annotations", + // changed the annotation value to croTestAnnotationValue1 + Value: apiextensionsv1.JSON{Raw: []byte(fmt.Sprintf(`{"%s": "%s"}`, croTestAnnotationKey, croTestAnnotationValue1))}, + }, + }, + }, + }, + }, + } + return hubClient.Update(ctx, cro) + }, eventuallyDuration, eventuallyInterval).Should(Succeed(), "Failed to update cro as expected", crpName) + }) + + It("should update CRP status as expected", func() { + wantCRONames := []string{fmt.Sprintf(placementv1alpha1.OverrideSnapshotNameFmt, croName, 1)} + crpStatusUpdatedActual := crpStatusWithOverrideUpdatedActual(workResourceIdentifiers(), allMemberClusterNames, "0", wantCRONames, nil) + // use the long duration to wait until the rollout is finished. + Eventually(crpStatusUpdatedActual, longEventuallyDuration, eventuallyInterval).Should(Succeed(), "Failed to update CRP %s status as expected", crpName) + }) + + // This check will ignore the annotation of resources. + It("should place the selected resources on member clusters", checkIfPlacedWorkResourcesOnAllMemberClusters) + + It("should have new override annotation value on the placed resources", func() { + want := map[string]string{croTestAnnotationKey: croTestAnnotationValue1} + checkIfOverrideAnnotationsOnAllMemberClusters(true, want) + }) }) var _ = Describe("creating clusterResourceOverride with multiple jsonPatchOverrides", Ordered, func() { @@ -173,6 +221,25 @@ var _ = Describe("creating clusterResourceOverride with multiple jsonPatchOverri wantAnnotations := map[string]string{croTestAnnotationKey: croTestAnnotationValue, croTestAnnotationKey1: croTestAnnotationValue1} checkIfOverrideAnnotationsOnAllMemberClusters(true, wantAnnotations) }) + + It("update cro attached to an invalid CRP", func() { + Eventually(func() error { + cro := &placementv1alpha1.ClusterResourceOverride{} + if err := hubClient.Get(ctx, types.NamespacedName{Name: croName}, cro); err != nil { + return err + } + cro.Spec.Placement = &placementv1alpha1.PlacementRef{ + Name: "invalid-crp", // assigned CRP name + } + return hubClient.Update(ctx, cro) + }, eventuallyDuration, eventuallyInterval).Should(Succeed(), "Failed to update cro as expected", crpName) + }) + + It("CRP status should not be changed", func() { + wantCRONames := []string{fmt.Sprintf(placementv1alpha1.OverrideSnapshotNameFmt, croName, 0)} + crpStatusUpdatedActual := crpStatusWithOverrideUpdatedActual(workResourceIdentifiers(), allMemberClusterNames, "0", wantCRONames, nil) + Consistently(crpStatusUpdatedActual, consistentlyDuration, consistentlyInterval).Should(Succeed(), "CRP %s status has been changed", crpName) + }) }) var _ = Describe("creating clusterResourceOverride with different rules for each cluster", Ordered, func() { diff --git a/test/e2e/placement_ro_test.go b/test/e2e/placement_ro_test.go index 5cabd278b..6c67a1e4d 100644 --- a/test/e2e/placement_ro_test.go +++ b/test/e2e/placement_ro_test.go @@ -97,6 +97,55 @@ var _ = Describe("creating resourceOverride (selecting all clusters) to override want := map[string]string{roTestAnnotationKey: roTestAnnotationValue} checkIfOverrideAnnotationsOnAllMemberClusters(false, want) }) + + It("update ro attached to this CRP only and annotation value", func() { + Eventually(func() error { + ro := &placementv1alpha1.ResourceOverride{} + if err := hubClient.Get(ctx, types.NamespacedName{Name: roName, Namespace: roNamespace}, ro); err != nil { + return err + } + ro.Spec = placementv1alpha1.ResourceOverrideSpec{ + Placement: &placementv1alpha1.PlacementRef{ + Name: crpName, + }, + ResourceSelectors: configMapSelector(), + Policy: &placementv1alpha1.OverridePolicy{ + OverrideRules: []placementv1alpha1.OverrideRule{ + { + ClusterSelector: &placementv1beta1.ClusterSelector{ + ClusterSelectorTerms: []placementv1beta1.ClusterSelectorTerm{}, + }, + JSONPatchOverrides: []placementv1alpha1.JSONPatchOverride{ + { + Operator: placementv1alpha1.JSONPatchOverrideOpAdd, + Path: "/metadata/annotations", + Value: apiextensionsv1.JSON{Raw: []byte(fmt.Sprintf(`{"%s": "%s"}`, roTestAnnotationKey, roTestAnnotationValue1))}, + }, + }, + }, + }, + }, + } + return hubClient.Update(ctx, ro) + }, eventuallyDuration, eventuallyInterval).Should(Succeed(), "Failed to update ro as expected", crpName) + }) + + It("should update CRP status as expected", func() { + wantRONames := []placementv1beta1.NamespacedName{ + {Namespace: roNamespace, Name: fmt.Sprintf(placementv1alpha1.OverrideSnapshotNameFmt, roName, 1)}, + } + crpStatusUpdatedActual := crpStatusWithOverrideUpdatedActual(workResourceIdentifiers(), allMemberClusterNames, "0", nil, wantRONames) + // use the long duration to wait until the rollout is finished. + Eventually(crpStatusUpdatedActual, longEventuallyDuration, eventuallyInterval).Should(Succeed(), "Failed to update CRP %s status as expected", crpName) + }) + + // This check will ignore the annotation of resources. + It("should place the selected resources on member clusters", checkIfPlacedWorkResourcesOnAllMemberClusters) + + It("should have override annotations on the configmap", func() { + want := map[string]string{roTestAnnotationKey: roTestAnnotationValue1} + checkIfOverrideAnnotationsOnAllMemberClusters(false, want) + }) }) var _ = Describe("creating resourceOverride with multiple jsonPatchOverrides to override configMap", Ordered, func() { @@ -181,6 +230,28 @@ var _ = Describe("creating resourceOverride with multiple jsonPatchOverrides to wantAnnotations := map[string]string{roTestAnnotationKey: roTestAnnotationValue, roTestAnnotationKey1: roTestAnnotationValue1} checkIfOverrideAnnotationsOnAllMemberClusters(false, wantAnnotations) }) + + It("update ro attached to an invalid CRP", func() { + Eventually(func() error { + ro := &placementv1alpha1.ResourceOverride{} + if err := hubClient.Get(ctx, types.NamespacedName{Name: roName, Namespace: roNamespace}, ro); err != nil { + return err + } + ro.Spec.Placement = &placementv1alpha1.PlacementRef{ + Name: "invalid-crp", // assigned CRP name + } + return hubClient.Update(ctx, ro) + }, eventuallyDuration, eventuallyInterval).Should(Succeed(), "Failed to update ro as expected", crpName) + }) + + It("CRP status should not be changed", func() { + wantRONames := []placementv1beta1.NamespacedName{ + {Namespace: roNamespace, Name: fmt.Sprintf(placementv1alpha1.OverrideSnapshotNameFmt, roName, 0)}, + } + crpStatusUpdatedActual := crpStatusWithOverrideUpdatedActual(workResourceIdentifiers(), allMemberClusterNames, "0", nil, wantRONames) + Consistently(crpStatusUpdatedActual, consistentlyDuration, consistentlyInterval).Should(Succeed(), "CRP %s status has been changed", crpName) + }) + }) var _ = Describe("creating resourceOverride with different rules for each cluster to override configMap", Ordered, func() {