diff --git a/.github/workflows/codespell.yml b/.github/workflows/codespell.yml index dab5ca43f..0f950d9d2 100644 --- a/.github/workflows/codespell.yml +++ b/.github/workflows/codespell.yml @@ -12,7 +12,7 @@ jobs: runs-on: ubuntu-latest steps: - name: Harden Runner - uses: step-security/harden-runner@c6295a65d1254861815972266d5933fd6e532bdf # v2.11.1 + uses: step-security/harden-runner@0634a2670c59f64b4a01f0f96f84700a4088b9f0 # v2.12.0 with: egress-policy: audit diff --git a/pkg/controllers/rollout/controller.go b/pkg/controllers/rollout/controller.go index fa4356398..0c78f1a91 100644 --- a/pkg/controllers/rollout/controller.go +++ b/pkg/controllers/rollout/controller.go @@ -1095,6 +1095,15 @@ func handleCRP(newCRPObj, oldCRPObj client.Object, q workqueue.TypedRateLimiting return } + // Check if the rollout strategy type has been updated. + if newCRP.Spec.Strategy.Type != oldCRP.Spec.Strategy.Type { + klog.V(2).InfoS("Detected an update to the rollout strategy type on the CRP", "clusterResourcePlacement", klog.KObj(newCRP)) + q.Add(reconcile.Request{ + NamespacedName: types.NamespacedName{Name: newCRP.GetName()}, + }) + return + } + // Check if the apply strategy has been updated. newApplyStrategy := newCRP.Spec.Strategy.ApplyStrategy oldApplyStrategy := oldCRP.Spec.Strategy.ApplyStrategy @@ -1103,6 +1112,7 @@ func handleCRP(newCRPObj, oldCRPObj client.Object, q workqueue.TypedRateLimiting q.Add(reconcile.Request{ NamespacedName: types.NamespacedName{Name: newCRP.GetName()}, }) + return } klog.V(2).InfoS("No update to apply strategy detected; ignore the CRP Update event", "clusterResourcePlacement", klog.KObj(newCRP)) diff --git a/pkg/controllers/rollout/controller_integration_test.go b/pkg/controllers/rollout/controller_integration_test.go index 54e53ec18..f9d3d4bef 100644 --- a/pkg/controllers/rollout/controller_integration_test.go +++ b/pkg/controllers/rollout/controller_integration_test.go @@ -91,29 +91,18 @@ var _ = Describe("Test the rollout Controller", func() { // create master resource snapshot that is latest masterSnapshot := generateResourceSnapshot(rolloutCRP.Name, 0, true) Expect(k8sClient.Create(ctx, masterSnapshot)).Should(Succeed()) - By(fmt.Sprintf("master resource snapshot %s created", masterSnapshot.Name)) + By(fmt.Sprintf("master resource snapshot %s created", masterSnapshot.Name)) // create scheduled bindings for master snapshot on target clusters clusters := make([]string, targetCluster) for i := 0; i < int(targetCluster); i++ { clusters[i] = "cluster-" + utils.RandStr() binding := generateClusterResourceBinding(fleetv1beta1.BindingStateScheduled, masterSnapshot.Name, clusters[i]) Expect(k8sClient.Create(ctx, binding)).Should(Succeed()) - By(fmt.Sprintf("resource binding %s created", binding.Name)) + By(fmt.Sprintf("resource binding %s created", binding.Name)) bindings = append(bindings, binding) } - // check that all bindings are bound - Eventually(func() bool { - for _, binding := range bindings { - err := k8sClient.Get(ctx, types.NamespacedName{Name: binding.GetName()}, binding) - if err != nil { - return false - } - if binding.Spec.State != fleetv1beta1.BindingStateBound || binding.Spec.ResourceSnapshotName != masterSnapshot.Name { - return false - } - } - return true - }, timeout, interval).Should(BeTrue(), "rollout controller should roll all the bindings to Bound state") + // Check that all bindings are bound. + verifyBindingsRolledOut(bindings, masterSnapshot) }) It("should push apply strategy changes to all the bindings (if applicable) and refresh their status", func() { @@ -340,22 +329,11 @@ var _ = Describe("Test the rollout Controller", func() { clusters[i] = "cluster-" + utils.RandStr() binding := generateClusterResourceBinding(fleetv1beta1.BindingStateScheduled, masterSnapshot.Name, clusters[i]) Expect(k8sClient.Create(ctx, binding)).Should(Succeed()) - By(fmt.Sprintf("resource binding %s created", binding.Name)) + By(fmt.Sprintf("resource binding %s created", binding.Name)) bindings = append(bindings, binding) } - // check that all bindings are scheduled - Eventually(func() bool { - for _, binding := range bindings { - err := k8sClient.Get(ctx, types.NamespacedName{Name: binding.GetName()}, binding) - if err != nil { - return false - } - if binding.Spec.State != fleetv1beta1.BindingStateBound || binding.Spec.ResourceSnapshotName != masterSnapshot.Name { - return false - } - } - return true - }, timeout, interval).Should(BeTrue(), "rollout controller should roll all the bindings to Bound state") + // Check that all bindings are bound. + verifyBindingsRolledOut(bindings, masterSnapshot) }) It("Should rollout the selected and unselected bindings (not trackable resources)", func() { @@ -368,29 +346,19 @@ var _ = Describe("Test the rollout Controller", func() { // create master resource snapshot that is latest masterSnapshot := generateResourceSnapshot(rolloutCRP.Name, 0, true) Expect(k8sClient.Create(ctx, masterSnapshot)).Should(Succeed()) - By(fmt.Sprintf("master resource snapshot %s created", masterSnapshot.Name)) + By(fmt.Sprintf("master resource snapshot %s created", masterSnapshot.Name)) // create scheduled bindings for master snapshot on target clusters clusters := make([]string, initTargetClusterNum) for i := 0; i < int(initTargetClusterNum); i++ { clusters[i] = "cluster-" + strconv.Itoa(i) binding := generateClusterResourceBinding(fleetv1beta1.BindingStateScheduled, masterSnapshot.Name, clusters[i]) Expect(k8sClient.Create(ctx, binding)).Should(Succeed()) - By(fmt.Sprintf("resource binding %s created", binding.Name)) + By(fmt.Sprintf("resource binding %s created", binding.Name)) bindings = append(bindings, binding) } - // check that all bindings are scheduled - Eventually(func() bool { - for _, binding := range bindings { - err := k8sClient.Get(ctx, types.NamespacedName{Name: binding.GetName()}, binding) - if err != nil { - return false - } - if binding.Spec.State != fleetv1beta1.BindingStateBound || binding.Spec.ResourceSnapshotName != masterSnapshot.Name { - return false - } - } - return true - }, timeout, interval).Should(BeTrue(), "rollout controller should roll all the bindings to Bound state") + // Check that all bindings are bound. + verifyBindingsRolledOut(bindings, masterSnapshot) + // simulate that some of the bindings are available and not trackable. firstApplied := 3 for i := 0; i < firstApplied; i++ { @@ -426,7 +394,7 @@ var _ = Describe("Test the rollout Controller", func() { for i := 0; i < newlyScheduledClusterNum; i++ { binding := generateClusterResourceBinding(fleetv1beta1.BindingStateScheduled, masterSnapshot.Name, "cluster-"+strconv.Itoa(int(initTargetClusterNum)+i)) Expect(k8sClient.Create(ctx, binding)).Should(Succeed()) - By(fmt.Sprintf("resource binding %s created", binding.Name)) + By(fmt.Sprintf("resource binding %s created", binding.Name)) bindings = append(bindings, binding) secondRoundBindings = append(secondRoundBindings, binding) } @@ -468,29 +436,19 @@ var _ = Describe("Test the rollout Controller", func() { // create master resource snapshot that is latest masterSnapshot := generateResourceSnapshot(rolloutCRP.Name, 0, true) Expect(k8sClient.Create(ctx, masterSnapshot)).Should(Succeed()) - By(fmt.Sprintf("master resource snapshot %s created", masterSnapshot.Name)) + By(fmt.Sprintf("master resource snapshot %s created", masterSnapshot.Name)) // create scheduled bindings for master snapshot on target clusters clusters := make([]string, targetCluster) for i := 0; i < int(targetCluster); i++ { clusters[i] = "cluster-" + strconv.Itoa(i) binding := generateClusterResourceBinding(fleetv1beta1.BindingStateScheduled, masterSnapshot.Name, clusters[i]) Expect(k8sClient.Create(ctx, binding)).Should(Succeed()) - By(fmt.Sprintf("resource binding %s created", binding.Name)) + By(fmt.Sprintf("resource binding %s created", binding.Name)) bindings = append(bindings, binding) } - // check that all bindings are scheduled - Eventually(func() bool { - for _, binding := range bindings { - err := k8sClient.Get(ctx, types.NamespacedName{Name: binding.GetName()}, binding) - if err != nil { - return false - } - if binding.Spec.State != fleetv1beta1.BindingStateBound { - return false - } - } - return true - }, timeout, interval).Should(BeTrue(), "rollout controller should roll all the bindings to Bound state") + // Check that all bindings are bound. + verifyBindingsRolledOut(bindings, masterSnapshot) + // simulate that some of the bindings are available firstApplied := 3 for i := 0; i < firstApplied; i++ { @@ -528,7 +486,7 @@ var _ = Describe("Test the rollout Controller", func() { for i := 0; i < newScheduled; i++ { binding := generateClusterResourceBinding(fleetv1beta1.BindingStateScheduled, masterSnapshot.Name, "cluster-"+strconv.Itoa(int(targetCluster)+i)) Expect(k8sClient.Create(ctx, binding)).Should(Succeed()) - By(fmt.Sprintf("resource binding %s created", binding.Name)) + By(fmt.Sprintf("resource binding %s created", binding.Name)) bindings = append(bindings, binding) secondRoundBindings = append(secondRoundBindings, binding) } @@ -621,29 +579,12 @@ var _ = Describe("Test the rollout Controller", func() { // create the normal binding after the deleting one for _, binding := range bindings { Expect(k8sClient.Create(ctx, binding)).Should(Succeed()) - By(fmt.Sprintf("resource binding %s created", binding.Name)) + By(fmt.Sprintf("resource binding %s created", binding.Name)) } - // wait until the client informer is populated - Eventually(func() error { - for _, binding := range bindings { - err := k8sClient.Get(ctx, types.NamespacedName{Name: binding.GetName()}, binding) - if err != nil { - return err - } - } - return nil - }, timeout, interval).Should(Succeed(), "make sure the cache is populated") + // check that no bindings are rolled out - Consistently(func(g Gomega) bool { - for _, binding := range bindings { - err := k8sClient.Get(ctx, types.NamespacedName{Name: binding.GetName()}, binding) - g.Expect(err).Should(Succeed()) - if binding.Spec.State == fleetv1beta1.BindingStateBound { - return false - } - } - return true - }, consistentTimeout, consistentInterval).Should(BeTrue(), "rollout controller should not roll the bindings") + verifyBindingsNotRolledOutConsistently(bindings) + By("Verified that the rollout is blocked") // now we remove the finalizer of the first deleting binding Expect(k8sClient.Get(ctx, types.NamespacedName{Name: firstDeleteBinding.GetName()}, firstDeleteBinding)).Should(Succeed()) @@ -653,19 +594,10 @@ var _ = Describe("Test the rollout Controller", func() { return apierrors.IsNotFound(k8sClient.Get(ctx, types.NamespacedName{Name: firstDeleteBinding.GetName()}, firstDeleteBinding)) }, timeout, interval).Should(BeTrue(), "the first deleting binding should now be deleted") By("Verified that the first deleting binding is deleted") + // check that no bindings are rolled out - Consistently(func() bool { - for _, binding := range bindings { - err := k8sClient.Get(ctx, types.NamespacedName{Name: binding.GetName()}, binding) - if err != nil { - return false - } - if binding.Spec.State == fleetv1beta1.BindingStateBound { - return false - } - } - return true - }, consistentTimeout, consistentInterval).Should(BeTrue(), "rollout controller should not roll the bindings") + verifyBindingsNotRolledOutConsistently(bindings) + By("Verified that the rollout is still blocked") // now we remove the finalizer of the second deleting binding Expect(k8sClient.Get(ctx, types.NamespacedName{Name: secondDeleteBinding.GetName()}, secondDeleteBinding)).Should(Succeed()) @@ -675,19 +607,8 @@ var _ = Describe("Test the rollout Controller", func() { return apierrors.IsNotFound(k8sClient.Get(ctx, types.NamespacedName{Name: secondDeleteBinding.GetName()}, secondDeleteBinding)) }, timeout, interval).Should(BeTrue(), "the second deleting binding should now be deleted") By("Verified that the second deleting binding is deleted") - // check that the bindings are rolledout - Eventually(func() bool { - for _, binding := range bindings { - err := k8sClient.Get(ctx, types.NamespacedName{Name: binding.GetName()}, binding) - if err != nil { - return false - } - if binding.Spec.State != fleetv1beta1.BindingStateBound { - return false - } - } - return true - }, consistentTimeout, consistentInterval).Should(BeTrue(), "rollout controller should roll all the bindings to Bound state") + // Check that the bindings are rolledout. + verifyBindingsRolledOut(bindings, latestSnapshot) By("Verified that the rollout is finally unblocked") }) @@ -701,29 +622,19 @@ var _ = Describe("Test the rollout Controller", func() { // create master resource snapshot that is latest masterSnapshot := generateResourceSnapshot(rolloutCRP.Name, 0, true) Expect(k8sClient.Create(ctx, masterSnapshot)).Should(Succeed()) - By(fmt.Sprintf("master resource snapshot %s created", masterSnapshot.Name)) + By(fmt.Sprintf("master resource snapshot %s created", masterSnapshot.Name)) // create scheduled bindings for master snapshot on target clusters clusters := make([]string, targetCluster) for i := 0; i < int(targetCluster); i++ { clusters[i] = "cluster-" + strconv.Itoa(i) binding := generateClusterResourceBinding(fleetv1beta1.BindingStateScheduled, masterSnapshot.Name, clusters[i]) Expect(k8sClient.Create(ctx, binding)).Should(Succeed()) - By(fmt.Sprintf("resource binding %s created", binding.Name)) + By(fmt.Sprintf("resource binding %s created", binding.Name)) bindings = append(bindings, binding) } - // check that all bindings are scheduled - Eventually(func() bool { - for _, binding := range bindings { - err := k8sClient.Get(ctx, types.NamespacedName{Name: binding.GetName()}, binding) - if err != nil { - return false - } - if binding.Spec.State != fleetv1beta1.BindingStateBound { - return false - } - } - return true - }, timeout, interval).Should(BeTrue(), "rollout controller should roll all the bindings to Bound state") + // Check that all bindings are bound. + verifyBindingsRolledOut(bindings, masterSnapshot) + // simulate that some of the bindings are available successfully applySuccessfully := 3 for i := 0; i < applySuccessfully; i++ { @@ -779,22 +690,11 @@ var _ = Describe("Test the rollout Controller", func() { clusters[i] = "cluster-" + utils.RandStr() binding := generateClusterResourceBinding(fleetv1beta1.BindingStateScheduled, masterSnapshot.Name, clusters[i]) Expect(k8sClient.Create(ctx, binding)).Should(Succeed()) - By(fmt.Sprintf("resource binding %s created", binding.Name)) + By(fmt.Sprintf("resource binding %s created", binding.Name)) bindings = append(bindings, binding) } - // check that all bindings are scheduled - Eventually(func() bool { - for _, binding := range bindings { - err := k8sClient.Get(ctx, types.NamespacedName{Name: binding.GetName()}, binding) - if err != nil { - return false - } - if binding.Spec.State != fleetv1beta1.BindingStateBound || binding.Spec.ResourceSnapshotName != masterSnapshot.Name { - return false - } - } - return true - }, timeout, interval).Should(BeTrue(), "rollout controller should roll all the bindings to Bound state") + // Check that all bindings are bound. + verifyBindingsRolledOut(bindings, masterSnapshot) // simulate that some of the bindings are available successfully applySuccessfully := 3 @@ -868,23 +768,12 @@ var _ = Describe("Test the rollout Controller", func() { clusters[i] = "cluster-" + utils.RandStr() binding := generateClusterResourceBinding(fleetv1beta1.BindingStateScheduled, masterSnapshot.Name, clusters[i]) Expect(k8sClient.Create(ctx, binding)).Should(Succeed()) - By(fmt.Sprintf("resource binding %s created", binding.Name)) + By(fmt.Sprintf("resource binding %s created", binding.Name)) bindings = append(bindings, binding) } - // check that all bindings are bound. - Eventually(func() bool { - for _, binding := range bindings { - err := k8sClient.Get(ctx, types.NamespacedName{Name: binding.GetName()}, binding) - if err != nil { - return false - } - if binding.Spec.State != fleetv1beta1.BindingStateBound || binding.Spec.ResourceSnapshotName != masterSnapshot.Name { - return false - } - } - return true - }, timeout, interval).Should(BeTrue(), "rollout controller should roll all the bindings to Bound state") + // Check that all bindings are bound.. + verifyBindingsRolledOut(bindings, masterSnapshot) // mark one binding as ready i.e. applied and available. availableBinding := 1 @@ -897,7 +786,7 @@ var _ = Describe("Test the rollout Controller", func() { cluster3 = "cluster-" + utils.RandStr() newScheduledBinding := generateClusterResourceBinding(fleetv1beta1.BindingStateScheduled, masterSnapshot.Name, cluster3) Expect(k8sClient.Create(ctx, newScheduledBinding)).Should(Succeed()) - By(fmt.Sprintf("resource binding %s created", newScheduledBinding.Name)) + By(fmt.Sprintf("resource binding %s created", newScheduledBinding.Name)) // add new scheduled binding to list of bindings. bindings = append(bindings, newScheduledBinding) @@ -979,23 +868,12 @@ var _ = Describe("Test the rollout Controller", func() { clusters[i] = "cluster-" + utils.RandStr() binding := generateClusterResourceBinding(fleetv1beta1.BindingStateScheduled, masterSnapshot.Name, clusters[i]) Expect(k8sClient.Create(ctx, binding)).Should(Succeed()) - By(fmt.Sprintf("resource binding %s created", binding.Name)) + By(fmt.Sprintf("resource binding %s created", binding.Name)) bindings = append(bindings, binding) } - // check that all bindings are bound. - Eventually(func() bool { - for _, binding := range bindings { - err := k8sClient.Get(ctx, types.NamespacedName{Name: binding.GetName()}, binding) - if err != nil { - return false - } - if binding.Spec.State != fleetv1beta1.BindingStateBound || binding.Spec.ResourceSnapshotName != masterSnapshot.Name { - return false - } - } - return true - }, timeout, interval).Should(BeTrue(), "rollout controller should roll all the bindings to Bound state") + // Check that all bindings are bound. + verifyBindingsRolledOut(bindings, masterSnapshot) // Note: This scenario is very unlikely in production user has to change the target from 2->3->2, // where scheduler created new scheduled binding but user changed the target number from 3->2 again, before rollout controller reads CRP. @@ -1003,7 +881,7 @@ var _ = Describe("Test the rollout Controller", func() { cluster3 = "cluster-" + utils.RandStr() newScheduledBinding := generateClusterResourceBinding(fleetv1beta1.BindingStateScheduled, masterSnapshot.Name, cluster3) Expect(k8sClient.Create(ctx, newScheduledBinding)).Should(Succeed()) - By(fmt.Sprintf("resource binding %s created", newScheduledBinding.Name)) + By(fmt.Sprintf("resource binding %s created", newScheduledBinding.Name)) // add new scheduled binding to list of bindings. bindings = append(bindings, newScheduledBinding) @@ -1046,12 +924,185 @@ var _ = Describe("Test the rollout Controller", func() { } }) + It("Should rollout all the selected bindings when strategy type is changed from External to RollingUpdate", func() { + By("Creating CRP with External strategy") + var targetCluster int32 = 10 + rolloutCRP = clusterResourcePlacementForTest(testCRPName, + createPlacementPolicyForTest(fleetv1beta1.PickNPlacementType, targetCluster), + createPlacementRolloutStrategyForTest(fleetv1beta1.ExternalRolloutStrategyType, nil, nil)) + Expect(k8sClient.Create(ctx, rolloutCRP)).Should(Succeed()) + + By("Creating the latest master resource snapshot") + masterSnapshot := generateResourceSnapshot(rolloutCRP.Name, 0, true) + Expect(k8sClient.Create(ctx, masterSnapshot)).Should(Succeed()) + By(fmt.Sprintf("master resource snapshot %s created", masterSnapshot.Name)) + + By("Creating scheduled bindings for master snapshot on target clusters") + clusters := make([]string, targetCluster) + for i := 0; i < int(targetCluster); i++ { + clusters[i] = "cluster-" + utils.RandStr() + binding := generateClusterResourceBinding(fleetv1beta1.BindingStateScheduled, masterSnapshot.Name, clusters[i]) + Expect(k8sClient.Create(ctx, binding)).Should(Succeed()) + By(fmt.Sprintf("resource binding %s created", binding.Name)) + bindings = append(bindings, binding) + } + + By("Checking bindings are not rolled out consistently") + verifyBindingsNotRolledOutConsistently(bindings) + + By("Updating CRP rollout strategy type to RollingUpdate") + rolloutCRP.Spec.Strategy.Type = fleetv1beta1.RollingUpdateRolloutStrategyType + rolloutCRP.Spec.Strategy.RollingUpdate = generateDefaultRollingUpdateConfig() + Expect(k8sClient.Update(ctx, rolloutCRP)).Should(Succeed(), "Failed to update CRP") + + By("Verifying that rollout is unblocked") + verifyBindingsRolledOut(bindings, masterSnapshot) + }) + + It("Should rollout all the selected bindings when strategy type is changed from External to empty", func() { + By("Creating CRP with External strategy") + var targetCluster int32 = 10 + rolloutCRP = clusterResourcePlacementForTest(testCRPName, + createPlacementPolicyForTest(fleetv1beta1.PickNPlacementType, targetCluster), + createPlacementRolloutStrategyForTest(fleetv1beta1.ExternalRolloutStrategyType, nil, nil)) + Expect(k8sClient.Create(ctx, rolloutCRP)).Should(Succeed()) + + By("Creating the latest master resource snapshot") + masterSnapshot := generateResourceSnapshot(rolloutCRP.Name, 0, true) + Expect(k8sClient.Create(ctx, masterSnapshot)).Should(Succeed()) + By(fmt.Sprintf("master resource snapshot %s created", masterSnapshot.Name)) + + By("Creating scheduled bindings for master snapshot on target clusters") + clusters := make([]string, targetCluster) + for i := 0; i < int(targetCluster); i++ { + clusters[i] = "cluster-" + utils.RandStr() + binding := generateClusterResourceBinding(fleetv1beta1.BindingStateScheduled, masterSnapshot.Name, clusters[i]) + Expect(k8sClient.Create(ctx, binding)).Should(Succeed()) + By(fmt.Sprintf("resource binding %s created", binding.Name)) + bindings = append(bindings, binding) + } + + By("Checking bindings are not rolled out consistently") + verifyBindingsNotRolledOutConsistently(bindings) + + By("Updating CRP rollout strategy type to empty") + rolloutCRP.Spec.Strategy.Type = "" + rolloutCRP.Spec.Strategy.RollingUpdate = nil + Expect(k8sClient.Update(ctx, rolloutCRP)).Should(Succeed(), "Failed to update CRP") + + By("Verifying that rollout is unblocked") + verifyBindingsRolledOut(bindings, masterSnapshot) + }) + + It("Should not rollout anymore if the rollout strategy type is changed from RollingUpdate to External", func() { + By("Creating CRP with RollingUpdate strategy") + var targetCluster int32 = 10 + rolloutCRP = clusterResourcePlacementForTest(testCRPName, + createPlacementPolicyForTest(fleetv1beta1.PickNPlacementType, targetCluster), + createPlacementRolloutStrategyForTest(fleetv1beta1.RollingUpdateRolloutStrategyType, generateDefaultRollingUpdateConfig(), nil)) + Expect(k8sClient.Create(ctx, rolloutCRP)).Should(Succeed()) + + By("Creating the latest master resource snapshot") + masterSnapshot := generateResourceSnapshot(rolloutCRP.Name, 0, true) + Expect(k8sClient.Create(ctx, masterSnapshot)).Should(Succeed()) + By(fmt.Sprintf("master resource snapshot %s created", masterSnapshot.Name)) + + By("Creating scheduled bindings for master snapshot on target clusters") + clusters := make([]string, targetCluster) + for i := 0; i < int(targetCluster); i++ { + clusters[i] = "cluster-" + strconv.Itoa(i) + binding := generateClusterResourceBinding(fleetv1beta1.BindingStateScheduled, masterSnapshot.Name, clusters[i]) + Expect(k8sClient.Create(ctx, binding)).Should(Succeed()) + By(fmt.Sprintf("resource binding %s created", binding.Name)) + bindings = append(bindings, binding) + } + + By("Checking bindings are rolled out") + verifyBindingsRolledOut(bindings, masterSnapshot) + + By("Updating CRP rollout strategy type to External") + rolloutCRP.Spec.Strategy.Type = fleetv1beta1.ExternalRolloutStrategyType + rolloutCRP.Spec.Strategy.RollingUpdate = nil + Expect(k8sClient.Update(ctx, rolloutCRP)).Should(Succeed(), "Failed to update CRP") + + By("Creating a new master resource snapshot") + // Mark the master snapshot as not latest. + masterSnapshot.SetLabels(map[string]string{ + fleetv1beta1.CRPTrackingLabel: testCRPName, + fleetv1beta1.IsLatestSnapshotLabel: "false"}, + ) + Expect(k8sClient.Update(ctx, masterSnapshot)).Should(Succeed()) + // Create a new master resource snapshot. + newMasterSnapshot := generateResourceSnapshot(rolloutCRP.Name, 1, true) + Expect(k8sClient.Create(ctx, newMasterSnapshot)).Should(Succeed()) + + By("Checking bindings are not updated") + // Check that resource snapshot is not updated on the bindings. + Consistently(func() error { + for _, binding := range bindings { + err := k8sClient.Get(ctx, types.NamespacedName{Name: binding.GetName()}, binding) + if err != nil { + return fmt.Errorf("failed to get binding %s: %w", binding.GetName(), err) + } + if binding.Spec.ResourceSnapshotName == newMasterSnapshot.Name { + return fmt.Errorf("binding %s is updated to the new snapshot, which is unwanted", binding.GetName()) + } + } + return nil + }, consistentTimeout, consistentInterval).Should(Succeed(), "rollout controller should not roll all the bindings to latest resource snapshot") + }) + // TODO: should update scheduled bindings to the latest snapshot when it is updated to bound state. // TODO: should count the deleting bindings as can be Unavailable. }) +func verifyBindingsNotRolledOutConsistently(bindings []*fleetv1beta1.ClusterResourceBinding) { + // Wait until the client informer is populated. + Eventually(func() error { + for _, binding := range bindings { + err := k8sClient.Get(ctx, types.NamespacedName{Name: binding.GetName()}, binding) + if err != nil { + return fmt.Errorf("failed to get binding %s: %w", binding.GetName(), err) + } + } + return nil + }, timeout, interval).Should(Succeed(), "make sure the cache is populated") + // Check that none of the bindings is bound. + Consistently(func() error { + for _, binding := range bindings { + err := k8sClient.Get(ctx, types.NamespacedName{Name: binding.GetName()}, binding) + if err != nil { + return fmt.Errorf("failed to get binding %s: %w", binding.GetName(), err) + } + if binding.Spec.State == fleetv1beta1.BindingStateBound { + return fmt.Errorf("binding %s is in bound state, which is unwanted", binding.GetName()) + } + } + return nil + }, consistentTimeout, consistentInterval).Should(Succeed(), "rollout controller should not roll any binding to Bound state") +} + +func verifyBindingsRolledOut(bindings []*fleetv1beta1.ClusterResourceBinding, masterSnapshot *fleetv1beta1.ClusterResourceSnapshot) { + // Check that all bindings are bound and updated to the latest snapshot. + Eventually(func() error { + for _, binding := range bindings { + err := k8sClient.Get(ctx, types.NamespacedName{Name: binding.GetName()}, binding) + if err != nil { + return fmt.Errorf("failed to get binding %s: %w", binding.GetName(), err) + } + if binding.Spec.State != fleetv1beta1.BindingStateBound { + return fmt.Errorf("binding %s is not updated to Bound state, got: %s", binding.GetName(), binding.Spec.State) + } + if binding.Spec.ResourceSnapshotName != masterSnapshot.Name { + return fmt.Errorf("binding %s is not updated to the latest snapshot, got: %s, want: %s", binding.GetName(), binding.Spec.ResourceSnapshotName, masterSnapshot.Name) + } + } + return nil + }, timeout, interval).Should(Succeed(), "rollout controller should roll out all the bindings") +} + func markBindingAvailable(binding *fleetv1beta1.ClusterResourceBinding, trackable bool) { Eventually(func() error { reason := "trackable" diff --git a/pkg/controllers/workgenerator/controller.go b/pkg/controllers/workgenerator/controller.go index 67c5f4ddd..048b71481 100644 --- a/pkg/controllers/workgenerator/controller.go +++ b/pkg/controllers/workgenerator/controller.go @@ -1280,8 +1280,11 @@ func extractResFromConfigMap(uConfigMap *unstructured.Unstructured) ([]fleetv1be klog.ErrorS(unMarshallErr, "manifest has invalid content", "manifestKey", key, "envelopeResource", klog.KObj(uConfigMap)) return nil, fmt.Errorf("the object with manifest key `%s` in envelope config `%s` is malformatted, err: %w", key, klog.KObj(uConfigMap), unMarshallErr) } + if len(uManifest.GetNamespace()) == 0 { + // Block cluster-scoped resources. + return nil, fmt.Errorf("cannot wrap cluster-scoped resource %s in the envelope %s", uManifest.GetName(), klog.KObj(uConfigMap)) + } if len(uManifest.GetNamespace()) != 0 && uManifest.GetNamespace() != configMap.Namespace { - // if the manifest is a namespaced object but no namespace is specified, it will fail at the apply time instead of here. return nil, fmt.Errorf("the namespaced object `%s` in envelope config `%s` is placed in a different namespace `%s` ", uManifest.GetName(), klog.KObj(uConfigMap), uManifest.GetNamespace()) } manifests = append(manifests, fleetv1beta1.Manifest{ diff --git a/pkg/controllers/workgenerator/controller_integration_test.go b/pkg/controllers/workgenerator/controller_integration_test.go index 50dd79521..9e0481fc8 100644 --- a/pkg/controllers/workgenerator/controller_integration_test.go +++ b/pkg/controllers/workgenerator/controller_integration_test.go @@ -698,7 +698,6 @@ var _ = Describe("Test Work Generator Controller", func() { Workload: placementv1beta1.WorkloadTemplate{ Manifests: []placementv1beta1.Manifest{ {RawExtension: runtime.RawExtension{Raw: testEnvelopeResourceQuota}}, - {RawExtension: runtime.RawExtension{Raw: testEnvelopeWebhook}}, }, }, }, @@ -823,7 +822,7 @@ var _ = Describe("Test Work Generator Controller", func() { Spec: placementv1beta1.WorkSpec{ Workload: placementv1beta1.WorkloadTemplate{ Manifests: []placementv1beta1.Manifest{ - {RawExtension: runtime.RawExtension{Raw: testEnvelopeWebhook}}, + {RawExtension: runtime.RawExtension{Raw: testEnvelopeResourceQuota2}}, }, }, }, diff --git a/pkg/controllers/workgenerator/controller_test.go b/pkg/controllers/workgenerator/controller_test.go index 77cd05b57..7e55113da 100644 --- a/pkg/controllers/workgenerator/controller_test.go +++ b/pkg/controllers/workgenerator/controller_test.go @@ -208,6 +208,23 @@ func TestExtractResFromConfigMap(t *testing.T) { want: nil, wantErr: true, }, + "config map with cluster scoped resource should fail": { + uConfigMap: &unstructured.Unstructured{ + Object: map[string]interface{}{ + "apiVersion": "v1", + "kind": "ConfigMap", + "metadata": map[string]interface{}{ + "name": "test-config", + "namespace": "default", + }, + "data": map[string]interface{}{ + "resource": `{"apiVersion": "admissionregistration.k8s.io/v1", "kind": "ValidatingWebhookConfiguration", "metadata": {"name": "test-webhook"}}`, + }, + }, + }, + want: nil, + wantErr: true, + }, "config map with valid and invalid entries should fail": { uConfigMap: &unstructured.Unstructured{ Object: map[string]interface{}{ @@ -226,7 +243,7 @@ func TestExtractResFromConfigMap(t *testing.T) { want: nil, wantErr: true, }, - "config map with cluster and namespace scoped data in the correct namespace should pass": { + "config map with cluster and namespace scoped data in the correct namespace should fail": { uConfigMap: &unstructured.Unstructured{ Object: map[string]interface{}{ "apiVersion": "v1", @@ -241,11 +258,8 @@ func TestExtractResFromConfigMap(t *testing.T) { }, }, }, - want: []fleetv1beta1.Manifest{ - {RawExtension: runtime.RawExtension{Raw: []byte(`{"apiVersion": "v1", "kind": "Pod", "metadata": {"name": "test-pod", "namespace": "default"}}`)}}, - {RawExtension: runtime.RawExtension{Raw: []byte(`{"apiVersion": "v1", "kind": "ClusterRole", "metadata": {"name": "test-role"}}`)}}, - }, - wantErr: false, + want: nil, + wantErr: true, }, "config map with cluster scoped and cross namespaced resources data in a different namespace should fail": { uConfigMap: &unstructured.Unstructured{ diff --git a/pkg/controllers/workgenerator/manifests/resourcequota2.yaml b/pkg/controllers/workgenerator/manifests/resourcequota2.yaml new file mode 100644 index 000000000..4e3224799 --- /dev/null +++ b/pkg/controllers/workgenerator/manifests/resourcequota2.yaml @@ -0,0 +1,11 @@ +apiVersion: v1 +kind: ResourceQuota +metadata: + name: mem-cpu-demo + namespace: app +spec: + hard: + requests.cpu: "2" + requests.memory: 2Gi + limits.cpu: "4" + limits.memory: 4Gi diff --git a/pkg/controllers/workgenerator/manifests/test-envelop-configmap.yaml b/pkg/controllers/workgenerator/manifests/test-envelop-configmap.yaml index 451947aae..c88877618 100644 --- a/pkg/controllers/workgenerator/manifests/test-envelop-configmap.yaml +++ b/pkg/controllers/workgenerator/manifests/test-envelop-configmap.yaml @@ -18,34 +18,3 @@ data: requests.memory: 1Gi limits.cpu: "2" limits.memory: 2Gi - webhook.yaml: | - apiVersion: admissionregistration.k8s.io/v1 - kind: MutatingWebhookConfiguration - metadata: - creationTimestamp: null - labels: - azure-workload-identity.io/system: "true" - name: azure-wi-webhook-mutating-webhook-configuration - webhooks: - - admissionReviewVersions: - - v1 - - v1beta1 - clientConfig: - service: - name: azure-wi-webhook-webhook-service - namespace: app - path: /mutate-v1-pod - failurePolicy: Fail - matchPolicy: Equivalent - name: mutation.azure-workload-identity.io - rules: - - apiGroups: - - "" - apiVersions: - - v1 - operations: - - CREATE - - UPDATE - resources: - - pods - sideEffects: None diff --git a/pkg/controllers/workgenerator/manifests/test-envelop-configmap2.yaml b/pkg/controllers/workgenerator/manifests/test-envelop-configmap2.yaml index a580d6971..3692ad470 100644 --- a/pkg/controllers/workgenerator/manifests/test-envelop-configmap2.yaml +++ b/pkg/controllers/workgenerator/manifests/test-envelop-configmap2.yaml @@ -6,34 +6,15 @@ metadata: annotations: kubernetes-fleet.io/envelope-configmap: "true" data: - webhook.yaml: | - apiVersion: admissionregistration.k8s.io/v1 - kind: MutatingWebhookConfiguration + resourceQuota.yaml: | + apiVersion: v1 + kind: ResourceQuota metadata: - creationTimestamp: null - labels: - azure-workload-identity.io/system: "true" - name: azure-wi-webhook-mutating-webhook-configuration - webhooks: - - admissionReviewVersions: - - v1 - - v1beta1 - clientConfig: - service: - name: azure-wi-webhook-webhook-service - namespace: app - path: /mutate-v1-pod - failurePolicy: Fail - matchPolicy: Equivalent - name: mutation.azure-workload-identity.io - rules: - - apiGroups: - - "" - apiVersions: - - v1 - operations: - - CREATE - - UPDATE - resources: - - pods - sideEffects: None + name: mem-cpu-demo + namespace: app + spec: + hard: + requests.cpu: "2" + requests.memory: 2Gi + limits.cpu: "4" + limits.memory: 4Gi diff --git a/pkg/controllers/workgenerator/suite_test.go b/pkg/controllers/workgenerator/suite_test.go index 2b8c188fd..c3156b425 100644 --- a/pkg/controllers/workgenerator/suite_test.go +++ b/pkg/controllers/workgenerator/suite_test.go @@ -65,7 +65,7 @@ var ( wantOverriddenTestResource []byte // the content of the enveloped resources - testEnvelopeWebhook, testEnvelopeResourceQuota []byte + testEnvelopeResourceQuota, testEnvelopeResourceQuota2 []byte ) func TestAPIs(t *testing.T) { @@ -365,15 +365,15 @@ func readTestManifests() { testPdb, err = yaml.ToJSON(rawByte) Expect(err).Should(Succeed()) - By("Read EnvelopeWebhook") - rawByte, err = os.ReadFile("manifests/webhook.yaml") - Expect(err).Should(Succeed()) - testEnvelopeWebhook, err = yaml.ToJSON(rawByte) - Expect(err).Should(Succeed()) - By("Read ResourceQuota") rawByte, err = os.ReadFile("manifests/resourcequota.yaml") Expect(err).Should(Succeed()) testEnvelopeResourceQuota, err = yaml.ToJSON(rawByte) Expect(err).Should(Succeed()) + + By("Read ResourceQuota2") + rawByte, err = os.ReadFile("manifests/resourcequota2.yaml") + Expect(err).Should(Succeed()) + testEnvelopeResourceQuota2, err = yaml.ToJSON(rawByte) + Expect(err).Should(Succeed()) } diff --git a/test/e2e/actuals_test.go b/test/e2e/actuals_test.go index e72f17e63..3d47be1d0 100644 --- a/test/e2e/actuals_test.go +++ b/test/e2e/actuals_test.go @@ -613,7 +613,11 @@ func resourcePlacementOverrideFailedConditions(generation int64) []metav1.Condit } } -func resourcePlacementWorkSynchronizedFailedConditions(generation int64) []metav1.Condition { +func resourcePlacementWorkSynchronizedFailedConditions(generation int64, hasOverrides bool) []metav1.Condition { + overridenCondReason := condition.OverrideNotSpecifiedReason + if hasOverrides { + overridenCondReason = condition.OverriddenSucceededReason + } return []metav1.Condition{ { Type: string(placementv1beta1.ResourceScheduledConditionType), @@ -631,7 +635,7 @@ func resourcePlacementWorkSynchronizedFailedConditions(generation int64) []metav Type: string(placementv1beta1.ResourceOverriddenConditionType), Status: metav1.ConditionTrue, ObservedGeneration: generation, - Reason: condition.OverriddenSucceededReason, + Reason: overridenCondReason, }, { Type: string(placementv1beta1.ResourceWorkSynchronizedConditionType), @@ -642,7 +646,11 @@ func resourcePlacementWorkSynchronizedFailedConditions(generation int64) []metav } } -func crpWorkSynchronizedFailedConditions(generation int64) []metav1.Condition { +func crpWorkSynchronizedFailedConditions(generation int64, hasOverrides bool) []metav1.Condition { + overridenCondReason := condition.OverrideNotSpecifiedReason + if hasOverrides { + overridenCondReason = condition.OverriddenSucceededReason + } return []metav1.Condition{ { Type: string(placementv1beta1.ClusterResourcePlacementScheduledConditionType), @@ -659,7 +667,7 @@ func crpWorkSynchronizedFailedConditions(generation int64) []metav1.Condition { { Type: string(placementv1beta1.ClusterResourcePlacementOverriddenConditionType), Status: metav1.ConditionTrue, - Reason: condition.OverriddenSucceededReason, + Reason: overridenCondReason, ObservedGeneration: generation, }, { @@ -782,17 +790,18 @@ func crpStatusWithWorkSynchronizedUpdatedFailedActual( } var wantPlacementStatus []placementv1beta1.ResourcePlacementStatus + hasOverrides := len(wantResourceOverrides) > 0 || len(wantClusterResourceOverrides) > 0 for _, name := range wantSelectedClusters { wantPlacementStatus = append(wantPlacementStatus, placementv1beta1.ResourcePlacementStatus{ ClusterName: name, - Conditions: resourcePlacementWorkSynchronizedFailedConditions(crp.Generation), + Conditions: resourcePlacementWorkSynchronizedFailedConditions(crp.Generation, hasOverrides), ApplicableResourceOverrides: wantResourceOverrides, ApplicableClusterResourceOverrides: wantClusterResourceOverrides, }) } wantStatus := placementv1beta1.ClusterResourcePlacementStatus{ - Conditions: crpWorkSynchronizedFailedConditions(crp.Generation), + Conditions: crpWorkSynchronizedFailedConditions(crp.Generation, hasOverrides), PlacementStatuses: wantPlacementStatus, SelectedResources: wantSelectedResourceIdentifiers, ObservedResourceIndex: wantObservedResourceIndex, diff --git a/test/e2e/enveloped_object_placement_test.go b/test/e2e/enveloped_object_placement_test.go index fa7032f93..3f678cd1a 100644 --- a/test/e2e/enveloped_object_placement_test.go +++ b/test/e2e/enveloped_object_placement_test.go @@ -22,12 +22,11 @@ import ( "strings" "github.com/google/go-cmp/cmp" - "github.com/google/go-cmp/cmp/cmpopts" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" - admv1 "k8s.io/api/admissionregistration/v1" appv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" + rbacv1 "k8s.io/api/rbac/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" "k8s.io/utils/ptr" @@ -43,10 +42,16 @@ import ( var ( // pre loaded test manifests testConfigMap, testEnvelopConfigMap corev1.ConfigMap - testEnvelopeWebhook admv1.MutatingWebhookConfiguration testEnvelopeResourceQuota corev1.ResourceQuota ) +const ( + wrapperCMName = "wrapper" + + cmDataKey = "foo" + cmDataVal = "bar" +) + // Note that this container will run in parallel with other containers. var _ = Describe("placing wrapped resources using a CRP", func() { Context("Test a CRP place enveloped objects successfully", Ordered, func() { @@ -103,14 +108,14 @@ var _ = Describe("placing wrapped resources using a CRP", func() { It("should update CRP status as expected", func() { // resourceQuota is enveloped so it's not trackable yet - crpStatusUpdatedActual := customizedCRPStatusUpdatedActual(crpName, wantSelectedResources, allMemberClusterNames, nil, "0", false) + crpStatusUpdatedActual := customizedCRPStatusUpdatedActual(crpName, wantSelectedResources, allMemberClusterNames, nil, "0", true) Eventually(crpStatusUpdatedActual, eventuallyDuration, eventuallyInterval).Should(Succeed(), "Failed to update CRP status as expected") }) It("should place the resources on all member clusters", func() { for idx := range allMemberClusters { memberCluster := allMemberClusters[idx] - workResourcesPlacedActual := checkEnvelopQuotaAndMutationWebhookPlacement(memberCluster) + workResourcesPlacedActual := checkEnvelopQuotaPlacement(memberCluster) Eventually(workResourcesPlacedActual, eventuallyDuration, eventuallyInterval).Should(Succeed(), "Failed to place work resources on member cluster %s", memberCluster.ClusterName) } }) @@ -147,14 +152,14 @@ var _ = Describe("placing wrapped resources using a CRP", func() { }) It("should update CRP status as success again", func() { - crpStatusUpdatedActual := customizedCRPStatusUpdatedActual(crpName, wantSelectedResources, allMemberClusterNames, nil, "2", false) + crpStatusUpdatedActual := customizedCRPStatusUpdatedActual(crpName, wantSelectedResources, allMemberClusterNames, nil, "2", true) Eventually(crpStatusUpdatedActual, longEventuallyDuration, eventuallyInterval).Should(Succeed(), "Failed to update CRP status as expected") }) It("should place the resources on all member clusters again", func() { for idx := range allMemberClusters { memberCluster := allMemberClusters[idx] - workResourcesPlacedActual := checkEnvelopQuotaAndMutationWebhookPlacement(memberCluster) + workResourcesPlacedActual := checkEnvelopQuotaPlacement(memberCluster) Eventually(workResourcesPlacedActual, eventuallyDuration, eventuallyInterval).Should(Succeed(), "Failed to place work resources on member cluster %s", memberCluster.ClusterName) } }) @@ -339,6 +344,144 @@ var _ = Describe("placing wrapped resources using a CRP", func() { ensureCRPAndRelatedResourcesDeleted(crpName, allMemberClusters) }) }) + + Context("Block envelopes that wrap cluster-scoped resources", Ordered, func() { + crpName := fmt.Sprintf(crpNameTemplate, GinkgoParallelProcess()) + workNamespaceName := fmt.Sprintf(workNamespaceNameTemplate, GinkgoParallelProcess()) + + wrappedCMName := "app" + wrappedCBName := "standard" + + BeforeAll(func() { + // Use an envelope to create duplicate resource entries. + ns := appNamespace() + Expect(hubClient.Create(ctx, &ns)).To(Succeed(), "Failed to create namespace %s", ns.Name) + + // Create an envelope config map. + wrapperCM := &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: wrapperCMName, + Namespace: ns.Name, + Annotations: map[string]string{ + placementv1beta1.EnvelopeConfigMapAnnotation: "true", + }, + }, + Data: map[string]string{}, + } + + // Create a configMap and a clusterRole as wrapped resources. + wrappedCM := &corev1.ConfigMap{ + TypeMeta: metav1.TypeMeta{ + APIVersion: corev1.SchemeGroupVersion.String(), + Kind: "ConfigMap", + }, + ObjectMeta: metav1.ObjectMeta{ + Namespace: ns.Name, + Name: wrappedCMName, + }, + Data: map[string]string{ + cmDataKey: cmDataVal, + }, + } + wrappedCMBytes, err := json.Marshal(wrappedCM) + Expect(err).To(BeNil(), "Failed to marshal configMap %s", wrappedCM.Name) + wrapperCM.Data["cm.yaml"] = string(wrappedCMBytes) + + wrappedCB := &rbacv1.ClusterRole{ + TypeMeta: metav1.TypeMeta{ + APIVersion: rbacv1.SchemeGroupVersion.String(), + Kind: "ClusterRole", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: wrappedCBName, + }, + Rules: []rbacv1.PolicyRule{ + { + APIGroups: []string{""}, + Resources: []string{"pods"}, + Verbs: []string{"get", "list", "watch"}, + }, + }, + } + wrappedCBBytes, err := json.Marshal(wrappedCB) + Expect(err).To(BeNil(), "Failed to marshal clusterRole %s", wrappedCB.Name) + wrapperCM.Data["cb.yaml"] = string(wrappedCBBytes) + + Expect(hubClient.Create(ctx, wrapperCM)).To(Succeed(), "Failed to create configMap %s", wrapperCM.Name) + + // Create a CRP. + crp := &placementv1beta1.ClusterResourcePlacement{ + ObjectMeta: metav1.ObjectMeta{ + Name: crpName, + // Add a custom finalizer; this would allow us to better observe + // the behavior of the controllers. + Finalizers: []string{customDeletionBlockerFinalizer}, + }, + Spec: placementv1beta1.ClusterResourcePlacementSpec{ + ResourceSelectors: workResourceSelector(), + Policy: &placementv1beta1.PlacementPolicy{ + PlacementType: placementv1beta1.PickFixedPlacementType, + ClusterNames: []string{ + memberCluster1EastProdName, + }, + }, + Strategy: placementv1beta1.RolloutStrategy{ + Type: placementv1beta1.RollingUpdateRolloutStrategyType, + RollingUpdate: &placementv1beta1.RollingUpdateConfig{ + UnavailablePeriodSeconds: ptr.To(2), + }, + }, + }, + } + Expect(hubClient.Create(ctx, crp)).To(Succeed(), "Failed to create CRP") + }) + + It("should update CRP status as expected", func() { + Eventually(func() error { + crp := &placementv1beta1.ClusterResourcePlacement{} + if err := hubClient.Get(ctx, types.NamespacedName{Name: crpName}, crp); err != nil { + return err + } + + wantStatus := placementv1beta1.ClusterResourcePlacementStatus{ + Conditions: crpWorkSynchronizedFailedConditions(crp.Generation, false), + PlacementStatuses: []placementv1beta1.ResourcePlacementStatus{ + { + ClusterName: memberCluster1EastProdName, + Conditions: resourcePlacementWorkSynchronizedFailedConditions(crp.Generation, false), + }, + }, + SelectedResources: []placementv1beta1.ResourceIdentifier{ + { + Kind: "Namespace", + Name: workNamespaceName, + Version: "v1", + }, + { + Kind: "ConfigMap", + Name: wrapperCMName, + Version: "v1", + Namespace: workNamespaceName, + }, + }, + ObservedResourceIndex: "0", + } + if diff := cmp.Diff(crp.Status, wantStatus, crpStatusCmpOptions...); diff != "" { + return fmt.Errorf("CRP status diff (-got, +want): %s", diff) + } + return nil + }, eventuallyDuration, eventuallyInterval).Should(Succeed(), "Failed to update CRP status as expected") + }) + + // Note that due to the order in which the work generator handles resources, the synchronization error is + // triggered before the primary work object is applied; that is, the namespace itself will not be created + // either. + + AfterAll(func() { + // Remove the CRP and the namespace from the hub cluster. + ensureCRPAndRelatedResourcesDeleted(crpName, []*framework.Cluster{memberCluster1EastProd}) + }) + }) }) var _ = Describe("Process objects with generate name", Ordered, func() { @@ -346,7 +489,6 @@ var _ = Describe("Process objects with generate name", Ordered, func() { workNamespaceName := fmt.Sprintf(workNamespaceNameTemplate, GinkgoParallelProcess()) nsGenerateName := "application-" - wrapperCMName := "wrapper" wrappedCMGenerateName := "wrapped-foo-" BeforeAll(func() { @@ -377,7 +519,7 @@ var _ = Describe("Process objects with generate name", Ordered, func() { Namespace: ns.Name, }, Data: map[string]string{ - "foo": "bar", + cmDataKey: cmDataVal, }, } wrappedCMByte, err := json.Marshal(wrappedCM) @@ -497,7 +639,7 @@ var _ = Describe("Process objects with generate name", Ordered, func() { }) }) -func checkEnvelopQuotaAndMutationWebhookPlacement(memberCluster *framework.Cluster) func() error { +func checkEnvelopQuotaPlacement(memberCluster *framework.Cluster) func() error { workNamespaceName := appNamespace().Name return func() error { if err := validateWorkNamespaceOnCluster(memberCluster, types.NamespacedName{Name: workNamespaceName}); err != nil { @@ -523,29 +665,6 @@ func checkEnvelopQuotaAndMutationWebhookPlacement(memberCluster *framework.Clust if diff := cmp.Diff(placedResourceQuota.Spec, testEnvelopeResourceQuota.Spec); diff != "" { return fmt.Errorf("resource quota diff (-got, +want): %s", diff) } - By("check the cluster scoped envelope objects") - placedEnvelopeWebhook := &admv1.MutatingWebhookConfiguration{} - if err := memberCluster.KubeClient.Get(ctx, types.NamespacedName{Name: testEnvelopeWebhook.Name}, placedEnvelopeWebhook); err != nil { - return err - } - // the two webhooks are very different since one is a client side yaml and the other is server side generated - if placedEnvelopeWebhook.Webhooks == nil || len(placedEnvelopeWebhook.Webhooks) != 1 { - return fmt.Errorf("webhook size does not match") - } - if placedEnvelopeWebhook.Webhooks[0].Name != testEnvelopeWebhook.Webhooks[0].Name || - *placedEnvelopeWebhook.Webhooks[0].FailurePolicy != *testEnvelopeWebhook.Webhooks[0].FailurePolicy || - *placedEnvelopeWebhook.Webhooks[0].SideEffects != *testEnvelopeWebhook.Webhooks[0].SideEffects || - *placedEnvelopeWebhook.Webhooks[0].MatchPolicy != *testEnvelopeWebhook.Webhooks[0].MatchPolicy { - return fmt.Errorf("webhook config does not match") - } - if len(placedEnvelopeWebhook.Webhooks[0].Rules) != 1 { - return fmt.Errorf("webhook rule size does not match") - } - if diff := cmp.Diff(placedEnvelopeWebhook.Webhooks[0].Rules[0], - testEnvelopeWebhook.Webhooks[0].Rules[0], - cmpopts.IgnoreFields(admv1.Rule{}, "Scope")); diff != "" { - return fmt.Errorf("webhook rule diff (-got, +want): %s", diff) - } return nil } } @@ -632,11 +751,6 @@ func readEnvelopTestManifests() { err = utils.GetObjectFromManifest("resources/test-envelop-configmap.yaml", &testEnvelopConfigMap) Expect(err).Should(Succeed()) - By("Read EnvelopeWebhook") - testEnvelopeWebhook = admv1.MutatingWebhookConfiguration{} - err = utils.GetObjectFromManifest("resources/webhook.yaml", &testEnvelopeWebhook) - Expect(err).Should(Succeed()) - By("Read ResourceQuota") testEnvelopeResourceQuota = corev1.ResourceQuota{} err = utils.GetObjectFromManifest("resources/resourcequota.yaml", &testEnvelopeResourceQuota) diff --git a/test/e2e/join_and_leave_test.go b/test/e2e/join_and_leave_test.go index f9318a31a..155053237 100644 --- a/test/e2e/join_and_leave_test.go +++ b/test/e2e/join_and_leave_test.go @@ -99,14 +99,14 @@ var _ = Describe("Test member cluster join and leave flow", Ordered, Serial, fun It("should update CRP status as expected", func() { // resourceQuota is not trackable yet - crpStatusUpdatedActual := customizedCRPStatusUpdatedActual(crpName, wantSelectedResources, allMemberClusterNames, nil, "0", false) + crpStatusUpdatedActual := customizedCRPStatusUpdatedActual(crpName, wantSelectedResources, allMemberClusterNames, nil, "0", true) Eventually(crpStatusUpdatedActual, eventuallyDuration, eventuallyInterval).Should(Succeed(), "Failed to update CRP status as expected") }) It("should place the resources on all member clusters", func() { for idx := range allMemberClusters { memberCluster := allMemberClusters[idx] - workResourcesPlacedActual := checkEnvelopQuotaAndMutationWebhookPlacement(memberCluster) + workResourcesPlacedActual := checkEnvelopQuotaPlacement(memberCluster) Eventually(workResourcesPlacedActual, eventuallyDuration, eventuallyInterval).Should(Succeed(), "Failed to place work resources on member cluster %s", memberCluster.ClusterName) } }) @@ -208,7 +208,7 @@ var _ = Describe("Test member cluster join and leave flow", Ordered, Serial, fun }) It("should update CRP status to applied to all clusters again automatically after rejoining", func() { - crpStatusUpdatedActual := customizedCRPStatusUpdatedActual(crpName, wantSelectedResources, allMemberClusterNames, nil, "0", false) + crpStatusUpdatedActual := customizedCRPStatusUpdatedActual(crpName, wantSelectedResources, allMemberClusterNames, nil, "0", true) Eventually(crpStatusUpdatedActual, eventuallyDuration, eventuallyInterval).Should(Succeed(), "Failed to update CRP status as expected") }) }) diff --git a/test/e2e/placement_cro_test.go b/test/e2e/placement_cro_test.go index 6d1e8d675..32030520d 100644 --- a/test/e2e/placement_cro_test.go +++ b/test/e2e/placement_cro_test.go @@ -37,6 +37,7 @@ import ( var _ = Context("creating clusterResourceOverride (selecting all clusters) to override all resources under the namespace", Ordered, func() { crpName := fmt.Sprintf(crpNameTemplate, GinkgoParallelProcess()) croName := fmt.Sprintf(croNameTemplate, GinkgoParallelProcess()) + croSnapShotName := fmt.Sprintf(placementv1alpha1.OverrideSnapshotNameFmt, croName, 0) BeforeAll(func() { By("creating work resources") @@ -69,21 +70,14 @@ var _ = Context("creating clusterResourceOverride (selecting all clusters) to ov } By(fmt.Sprintf("creating clusterResourceOverride %s", croName)) Expect(hubClient.Create(ctx, cro)).To(Succeed(), "Failed to create clusterResourceOverride %s", croName) + //this is to make sure the cro snapshot is created before the CRP + Eventually(func() error { + croSnap := &placementv1alpha1.ClusterResourceOverrideSnapshot{} + return hubClient.Get(ctx, types.NamespacedName{Name: croSnapShotName}, croSnap) + }, eventuallyDuration, eventuallyInterval).Should(Succeed(), "Failed to update ro as expected", crpName) // Create the CRP. - crp := &placementv1beta1.ClusterResourcePlacement{ - ObjectMeta: metav1.ObjectMeta{ - Name: crpName, - // Add a custom finalizer; this would allow us to better observe - // the behavior of the controllers. - Finalizers: []string{customDeletionBlockerFinalizer}, - }, - Spec: placementv1beta1.ClusterResourcePlacementSpec{ - ResourceSelectors: workResourceSelector(), - }, - } - By(fmt.Sprintf("creating placement %s", crpName)) - Expect(hubClient.Create(ctx, crp)).To(Succeed(), "Failed to create CRP %s", crpName) + createCRP(crpName) }) AfterAll(func() { @@ -95,7 +89,7 @@ var _ = Context("creating clusterResourceOverride (selecting all clusters) to ov }) It("should update CRP status as expected", func() { - wantCRONames := []string{fmt.Sprintf(placementv1alpha1.OverrideSnapshotNameFmt, croName, 0)} + wantCRONames := []string{croSnapShotName} crpStatusUpdatedActual := crpStatusWithOverrideUpdatedActual(workResourceIdentifiers(), allMemberClusterNames, "0", wantCRONames, nil) Eventually(crpStatusUpdatedActual, eventuallyDuration, eventuallyInterval).Should(Succeed(), "Failed to update CRP %s status as expected", crpName) }) @@ -213,6 +207,7 @@ var _ = Context("creating clusterResourceOverride (selecting all clusters) to ov var _ = Context("creating clusterResourceOverride with multiple jsonPatchOverrides", Ordered, func() { crpName := fmt.Sprintf(crpNameTemplate, GinkgoParallelProcess()) croName := fmt.Sprintf(croNameTemplate, GinkgoParallelProcess()) + croSnapShotName := fmt.Sprintf(placementv1alpha1.OverrideSnapshotNameFmt, croName, 0) BeforeAll(func() { By("creating work resources") @@ -251,20 +246,14 @@ var _ = Context("creating clusterResourceOverride with multiple jsonPatchOverrid By(fmt.Sprintf("creating clusterResourceOverride %s", croName)) Expect(hubClient.Create(ctx, cro)).To(Succeed(), "Failed to create clusterResourceOverride %s", croName) + //this is to make sure the cro snapshot is created before the CRP + Eventually(func() error { + croSnap := &placementv1alpha1.ClusterResourceOverrideSnapshot{} + return hubClient.Get(ctx, types.NamespacedName{Name: croSnapShotName}, croSnap) + }, eventuallyDuration, eventuallyInterval).Should(Succeed(), "Failed to update ro as expected", crpName) + // Create the CRP. - crp := &placementv1beta1.ClusterResourcePlacement{ - ObjectMeta: metav1.ObjectMeta{ - Name: crpName, - // Add a custom finalizer; this would allow us to better observe - // the behavior of the controllers. - Finalizers: []string{customDeletionBlockerFinalizer}, - }, - Spec: placementv1beta1.ClusterResourcePlacementSpec{ - ResourceSelectors: workResourceSelector(), - }, - } - By(fmt.Sprintf("creating placement %s", crpName)) - Expect(hubClient.Create(ctx, crp)).To(Succeed(), "Failed to create CRP %s", crpName) + createCRP(crpName) }) AfterAll(func() { @@ -276,7 +265,7 @@ var _ = Context("creating clusterResourceOverride with multiple jsonPatchOverrid }) It("should update CRP status as expected", func() { - wantCRONames := []string{fmt.Sprintf(placementv1alpha1.OverrideSnapshotNameFmt, croName, 0)} + wantCRONames := []string{croSnapShotName} crpStatusUpdatedActual := crpStatusWithOverrideUpdatedActual(workResourceIdentifiers(), allMemberClusterNames, "0", wantCRONames, nil) Eventually(crpStatusUpdatedActual, eventuallyDuration, eventuallyInterval).Should(Succeed(), "Failed to update CRP %s status as expected", crpName) }) @@ -312,6 +301,7 @@ var _ = Context("creating clusterResourceOverride with multiple jsonPatchOverrid var _ = Context("creating clusterResourceOverride with different rules for each cluster", Ordered, func() { crpName := fmt.Sprintf(crpNameTemplate, GinkgoParallelProcess()) croName := fmt.Sprintf(croNameTemplate, GinkgoParallelProcess()) + croSnapShotName := fmt.Sprintf(placementv1alpha1.OverrideSnapshotNameFmt, croName, 0) BeforeAll(func() { By("creating work resources") @@ -387,20 +377,14 @@ var _ = Context("creating clusterResourceOverride with different rules for each By(fmt.Sprintf("creating clusterResourceOverride %s", croName)) Expect(hubClient.Create(ctx, cro)).To(Succeed(), "Failed to create clusterResourceOverride %s", croName) + //this is to make sure the cro snapshot is created before the CRP + Eventually(func() error { + croSnap := &placementv1alpha1.ClusterResourceOverrideSnapshot{} + return hubClient.Get(ctx, types.NamespacedName{Name: croSnapShotName}, croSnap) + }, eventuallyDuration, eventuallyInterval).Should(Succeed(), "Failed to update ro as expected", crpName) + // Create the CRP. - crp := &placementv1beta1.ClusterResourcePlacement{ - ObjectMeta: metav1.ObjectMeta{ - Name: crpName, - // Add a custom finalizer; this would allow us to better observe - // the behavior of the controllers. - Finalizers: []string{customDeletionBlockerFinalizer}, - }, - Spec: placementv1beta1.ClusterResourcePlacementSpec{ - ResourceSelectors: workResourceSelector(), - }, - } - By(fmt.Sprintf("creating placement %s", crpName)) - Expect(hubClient.Create(ctx, crp)).To(Succeed(), "Failed to create CRP %s", crpName) + createCRP(crpName) }) AfterAll(func() { @@ -412,7 +396,7 @@ var _ = Context("creating clusterResourceOverride with different rules for each }) It("should update CRP status as expected", func() { - wantCRONames := []string{fmt.Sprintf(placementv1alpha1.OverrideSnapshotNameFmt, croName, 0)} + wantCRONames := []string{croSnapShotName} crpStatusUpdatedActual := crpStatusWithOverrideUpdatedActual(workResourceIdentifiers(), allMemberClusterNames, "0", wantCRONames, nil) Eventually(crpStatusUpdatedActual, eventuallyDuration, eventuallyInterval).Should(Succeed(), "Failed to update CRP %s status as expected", crpName) }) @@ -429,7 +413,7 @@ var _ = Context("creating clusterResourceOverride with different rules for each }) }) -var _ = Context("creating clusterResourceOverride with different rules for each cluster", Ordered, func() { +var _ = Context("creating clusterResourceOverride with different rules for each cluster that is attached to a CRP", Ordered, func() { crpName := fmt.Sprintf(crpNameTemplate, GinkgoParallelProcess()) croName := fmt.Sprintf(croNameTemplate, GinkgoParallelProcess()) @@ -507,6 +491,7 @@ var _ = Context("creating clusterResourceOverride with different rules for each var _ = Context("creating clusterResourceOverride with incorrect path", Ordered, func() { crpName := fmt.Sprintf(crpNameTemplate, GinkgoParallelProcess()) croName := fmt.Sprintf(croNameTemplate, GinkgoParallelProcess()) + croSnapShotName := fmt.Sprintf(placementv1alpha1.OverrideSnapshotNameFmt, croName, 0) BeforeAll(func() { By("creating work resources") @@ -540,20 +525,14 @@ var _ = Context("creating clusterResourceOverride with incorrect path", Ordered, By(fmt.Sprintf("creating clusterResourceOverride %s", croName)) Expect(hubClient.Create(ctx, cro)).To(Succeed(), "Failed to create clusterResourceOverride %s", croName) + //this is to make sure the cro snapshot is created before the CRP + Eventually(func() error { + croSnap := &placementv1alpha1.ClusterResourceOverrideSnapshot{} + return hubClient.Get(ctx, types.NamespacedName{Name: croSnapShotName}, croSnap) + }, eventuallyDuration, eventuallyInterval).Should(Succeed(), "Failed to update ro as expected", crpName) + // Create the CRP. - crp := &placementv1beta1.ClusterResourcePlacement{ - ObjectMeta: metav1.ObjectMeta{ - Name: crpName, - // Add a custom finalizer; this would allow us to better observe - // the behavior of the controllers. - Finalizers: []string{customDeletionBlockerFinalizer}, - }, - Spec: placementv1beta1.ClusterResourcePlacementSpec{ - ResourceSelectors: workResourceSelector(), - }, - } - By(fmt.Sprintf("creating placement %s", crpName)) - Expect(hubClient.Create(ctx, crp)).To(Succeed(), "Failed to create CRP %s", crpName) + createCRP(crpName) }) AfterAll(func() { @@ -565,7 +544,7 @@ var _ = Context("creating clusterResourceOverride with incorrect path", Ordered, }) It("should update CRP status as expected", func() { - wantCRONames := []string{fmt.Sprintf(placementv1alpha1.OverrideSnapshotNameFmt, croName, 0)} + wantCRONames := []string{croSnapShotName} crpStatusUpdatedActual := crpStatusWithOverrideUpdatedFailedActual(workResourceIdentifiers(), allMemberClusterNames, "0", wantCRONames, nil) Eventually(crpStatusUpdatedActual, eventuallyDuration, eventuallyInterval).Should(Succeed(), "Failed to update CRP %s status as expected", crpName) }) @@ -577,6 +556,7 @@ var _ = Context("creating clusterResourceOverride with incorrect path", Ordered, var _ = Context("creating clusterResourceOverride with and resource becomes invalid after override", Ordered, func() { crpName := fmt.Sprintf(crpNameTemplate, GinkgoParallelProcess()) croName := fmt.Sprintf(croNameTemplate, GinkgoParallelProcess()) + croSnapShotName := fmt.Sprintf(placementv1alpha1.OverrideSnapshotNameFmt, croName, 0) BeforeAll(func() { By("creating work resources") @@ -610,20 +590,14 @@ var _ = Context("creating clusterResourceOverride with and resource becomes inva By(fmt.Sprintf("creating clusterResourceOverride %s", croName)) Expect(hubClient.Create(ctx, cro)).To(Succeed(), "Failed to create clusterResourceOverride %s", croName) + //this is to make sure the cro snapshot is created before the CRP + Eventually(func() error { + croSnap := &placementv1alpha1.ClusterResourceOverrideSnapshot{} + return hubClient.Get(ctx, types.NamespacedName{Name: croSnapShotName}, croSnap) + }, eventuallyDuration, eventuallyInterval).Should(Succeed(), "Failed to update ro as expected", crpName) + // Create the CRP. - crp := &placementv1beta1.ClusterResourcePlacement{ - ObjectMeta: metav1.ObjectMeta{ - Name: crpName, - // Add a custom finalizer; this would allow us to better observe - // the behavior of the controllers. - Finalizers: []string{customDeletionBlockerFinalizer}, - }, - Spec: placementv1beta1.ClusterResourcePlacementSpec{ - ResourceSelectors: workResourceSelector(), - }, - } - By(fmt.Sprintf("creating placement %s", crpName)) - Expect(hubClient.Create(ctx, crp)).To(Succeed(), "Failed to create CRP %s", crpName) + createCRP(crpName) }) AfterAll(func() { @@ -635,7 +609,7 @@ var _ = Context("creating clusterResourceOverride with and resource becomes inva }) It("should update CRP status as expected", func() { - wantCRONames := []string{fmt.Sprintf(placementv1alpha1.OverrideSnapshotNameFmt, croName, 0)} + wantCRONames := []string{croSnapShotName} crpStatusUpdatedActual := crpStatusWithWorkSynchronizedUpdatedFailedActual(workResourceIdentifiers(), allMemberClusterNames, "0", wantCRONames, nil) Eventually(crpStatusUpdatedActual, eventuallyDuration, eventuallyInterval).Should(Succeed(), "Failed to update CRP %s status as expected", crpName) }) @@ -658,6 +632,9 @@ var _ = Context("creating clusterResourceOverride with delete rules for one clus Name: croName, }, Spec: placementv1alpha1.ClusterResourceOverrideSpec{ + Placement: &placementv1alpha1.PlacementRef{ + Name: crpName, // assigned CRP name + }, ClusterResourceSelectors: workResourceSelector(), Policy: &placementv1alpha1.OverridePolicy{ OverrideRules: []placementv1alpha1.OverrideRule{ @@ -700,19 +677,7 @@ var _ = Context("creating clusterResourceOverride with delete rules for one clus Expect(hubClient.Create(ctx, cro)).To(Succeed(), "Failed to create clusterResourceOverride %s", croName) // Create the CRP. - crp := &placementv1beta1.ClusterResourcePlacement{ - ObjectMeta: metav1.ObjectMeta{ - Name: crpName, - // Add a custom finalizer; this would allow us to better observe - // the behavior of the controllers. - Finalizers: []string{customDeletionBlockerFinalizer}, - }, - Spec: placementv1beta1.ClusterResourcePlacementSpec{ - ResourceSelectors: workResourceSelector(), - }, - } - By(fmt.Sprintf("creating placement %s", crpName)) - Expect(hubClient.Create(ctx, crp)).To(Succeed(), "Failed to create CRP %s", crpName) + createCRP(crpName) }) AfterAll(func() { diff --git a/test/e2e/placement_ro_test.go b/test/e2e/placement_ro_test.go index 9fe982d49..49b71b39b 100644 --- a/test/e2e/placement_ro_test.go +++ b/test/e2e/placement_ro_test.go @@ -18,6 +18,7 @@ package e2e import ( "fmt" + "github.com/google/go-cmp/cmp" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" corev1 "k8s.io/api/core/v1" @@ -28,6 +29,8 @@ import ( placementv1alpha1 "go.goms.io/fleet/apis/placement/v1alpha1" placementv1beta1 "go.goms.io/fleet/apis/placement/v1beta1" + scheduler "go.goms.io/fleet/pkg/scheduler/framework" + "go.goms.io/fleet/pkg/utils/condition" ) // Note that this container will run in parallel with other containers. @@ -200,12 +203,12 @@ var _ = Context("creating resourceOverride with multiple jsonPatchOverrides to o crpName := fmt.Sprintf(crpNameTemplate, GinkgoParallelProcess()) roName := fmt.Sprintf(roNameTemplate, GinkgoParallelProcess()) roNamespace := fmt.Sprintf(workNamespaceNameTemplate, GinkgoParallelProcess()) + roSnapShotName := fmt.Sprintf(placementv1alpha1.OverrideSnapshotNameFmt, roName, 0) BeforeAll(func() { By("creating work resources") createWorkResources() - // Create the ro before crp so that the observed resource index is predictable. ro := &placementv1alpha1.ResourceOverride{ ObjectMeta: metav1.ObjectMeta{ Name: roName, @@ -238,6 +241,11 @@ var _ = Context("creating resourceOverride with multiple jsonPatchOverrides to o } By(fmt.Sprintf("creating resourceOverride %s", roName)) Expect(hubClient.Create(ctx, ro)).To(Succeed(), "Failed to create resourceOverride %s", roName) + // wait until the snapshot is created so that the observed resource index is predictable. + Eventually(func() error { + roSnap := &placementv1alpha1.ResourceOverrideSnapshot{} + return hubClient.Get(ctx, types.NamespacedName{Name: roSnapShotName, Namespace: roNamespace}, roSnap) + }, eventuallyDuration, eventuallyInterval).Should(Succeed(), "Failed to update ro as expected", crpName) // Create the CRP. createCRP(crpName) @@ -253,7 +261,7 @@ var _ = Context("creating resourceOverride with multiple jsonPatchOverrides to o It("should update CRP status as expected", func() { wantRONames := []placementv1beta1.NamespacedName{ - {Namespace: roNamespace, Name: fmt.Sprintf(placementv1alpha1.OverrideSnapshotNameFmt, roName, 0)}, + {Namespace: roNamespace, Name: roSnapShotName}, } crpStatusUpdatedActual := crpStatusWithOverrideUpdatedActual(workResourceIdentifiers(), allMemberClusterNames, "0", nil, wantRONames) Eventually(crpStatusUpdatedActual, eventuallyDuration, eventuallyInterval).Should(Succeed(), "Failed to update CRP %s status as expected", crpName) @@ -407,6 +415,8 @@ var _ = Context("creating resourceOverride and clusterResourceOverride, resource croName := fmt.Sprintf(croNameTemplate, GinkgoParallelProcess()) roName := fmt.Sprintf(roNameTemplate, GinkgoParallelProcess()) roNamespace := fmt.Sprintf(workNamespaceNameTemplate, GinkgoParallelProcess()) + roSnapShotName := fmt.Sprintf(placementv1alpha1.OverrideSnapshotNameFmt, roName, 0) + croSnapShotName := fmt.Sprintf(placementv1alpha1.OverrideSnapshotNameFmt, croName, 0) BeforeAll(func() { By("creating work resources") @@ -437,7 +447,6 @@ var _ = Context("creating resourceOverride and clusterResourceOverride, resource } By(fmt.Sprintf("creating clusterResourceOverride %s", croName)) Expect(hubClient.Create(ctx, cro)).To(Succeed(), "Failed to create clusterResourceOverride %s", croName) - // Create the ro before crp so that the observed resource index is predictable. ro := &placementv1alpha1.ResourceOverride{ ObjectMeta: metav1.ObjectMeta{ Name: roName, @@ -465,6 +474,15 @@ var _ = Context("creating resourceOverride and clusterResourceOverride, resource } By(fmt.Sprintf("creating resourceOverride %s", roName)) Expect(hubClient.Create(ctx, ro)).To(Succeed(), "Failed to create resourceOverride %s", roName) + // wait until the snapshot is created so that the observed resource index is predictable. + Eventually(func() error { + roSnap := &placementv1alpha1.ResourceOverrideSnapshot{} + return hubClient.Get(ctx, types.NamespacedName{Name: roSnapShotName, Namespace: roNamespace}, roSnap) + }, eventuallyDuration, eventuallyInterval).Should(Succeed(), "Failed to update ro as expected", crpName) + Eventually(func() error { + croSnap := &placementv1alpha1.ClusterResourceOverrideSnapshot{} + return hubClient.Get(ctx, types.NamespacedName{Name: croSnapShotName}, croSnap) + }, eventuallyDuration, eventuallyInterval).Should(Succeed(), "Failed to update ro as expected", crpName) // Create the CRP. createCRP(crpName) @@ -483,9 +501,9 @@ var _ = Context("creating resourceOverride and clusterResourceOverride, resource It("should update CRP status as expected", func() { wantRONames := []placementv1beta1.NamespacedName{ - {Namespace: roNamespace, Name: fmt.Sprintf(placementv1alpha1.OverrideSnapshotNameFmt, roName, 0)}, + {Namespace: roNamespace, Name: roSnapShotName}, } - wantCRONames := []string{fmt.Sprintf(placementv1alpha1.OverrideSnapshotNameFmt, croName, 0)} + wantCRONames := []string{croSnapShotName} crpStatusUpdatedActual := crpStatusWithOverrideUpdatedActual(workResourceIdentifiers(), allMemberClusterNames, "0", wantCRONames, wantRONames) Eventually(crpStatusUpdatedActual, eventuallyDuration, eventuallyInterval).Should(Succeed(), "Failed to update CRP %s status as expected", crpName) }) @@ -511,6 +529,7 @@ var _ = Context("creating resourceOverride with incorrect path", Ordered, func() crpName := fmt.Sprintf(crpNameTemplate, GinkgoParallelProcess()) roName := fmt.Sprintf(croNameTemplate, GinkgoParallelProcess()) roNamespace := fmt.Sprintf(workNamespaceNameTemplate, GinkgoParallelProcess()) + roSnapShotName := fmt.Sprintf(placementv1alpha1.OverrideSnapshotNameFmt, roName, 0) BeforeAll(func() { By("creating work resources") @@ -546,8 +565,13 @@ var _ = Context("creating resourceOverride with incorrect path", Ordered, func() } By(fmt.Sprintf("creating the bad resourceOverride %s", roName)) Expect(hubClient.Create(ctx, ro)).To(Succeed(), "Failed to create resourceOverride %s", roName) + // wait until the snapshot is created so that failed override won't block the rollout + Eventually(func() error { + roSnap := &placementv1alpha1.ResourceOverrideSnapshot{} + return hubClient.Get(ctx, types.NamespacedName{Name: roSnapShotName, Namespace: roNamespace}, roSnap) + }, eventuallyDuration, eventuallyInterval).Should(Succeed(), "Failed to update ro as expected", crpName) - // Create the CRP later so that failed override won't block the rollout + // Create the CRP later createCRP(crpName) }) @@ -561,7 +585,7 @@ var _ = Context("creating resourceOverride with incorrect path", Ordered, func() It("should update CRP status as failed to override", func() { wantRONames := []placementv1beta1.NamespacedName{ - {Namespace: roNamespace, Name: fmt.Sprintf(placementv1alpha1.OverrideSnapshotNameFmt, roName, 0)}, + {Namespace: roNamespace, Name: roSnapShotName}, } crpStatusUpdatedActual := crpStatusWithOverrideUpdatedFailedActual(workResourceIdentifiers(), allMemberClusterNames, "0", nil, wantRONames) Eventually(crpStatusUpdatedActual, eventuallyDuration, eventuallyInterval).Should(Succeed(), "Failed to update CRP %s status as expected", crpName) @@ -638,6 +662,7 @@ var _ = Context("creating resourceOverride with a templated rules with cluster n crpName := fmt.Sprintf(crpNameTemplate, GinkgoParallelProcess()) roName := fmt.Sprintf(roNameTemplate, GinkgoParallelProcess()) roNamespace := fmt.Sprintf(workNamespaceNameTemplate, GinkgoParallelProcess()) + roSnapShotName := fmt.Sprintf(placementv1alpha1.OverrideSnapshotNameFmt, roName, 0) BeforeAll(func() { By("creating work resources") @@ -687,6 +712,10 @@ var _ = Context("creating resourceOverride with a templated rules with cluster n } By(fmt.Sprintf("creating resourceOverride %s", roName)) Expect(hubClient.Create(ctx, ro)).To(Succeed(), "Failed to create resourceOverride %s", roName) + Eventually(func() error { + roSnap := &placementv1alpha1.ResourceOverrideSnapshot{} + return hubClient.Get(ctx, types.NamespacedName{Name: roSnapShotName, Namespace: roNamespace}, roSnap) + }, eventuallyDuration, eventuallyInterval).Should(Succeed(), "Failed to update ro as expected", crpName) // Create the CRP. createCRP(crpName) @@ -702,7 +731,7 @@ var _ = Context("creating resourceOverride with a templated rules with cluster n It("should update CRP status as expected", func() { wantRONames := []placementv1beta1.NamespacedName{ - {Namespace: roNamespace, Name: fmt.Sprintf(placementv1alpha1.OverrideSnapshotNameFmt, roName, 0)}, + {Namespace: roNamespace, Name: roSnapShotName}, } crpStatusUpdatedActual := crpStatusWithOverrideUpdatedActual(workResourceIdentifiers(), allMemberClusterNames, "0", nil, wantRONames) Eventually(crpStatusUpdatedActual, eventuallyDuration, eventuallyInterval).Should(Succeed(), "Failed to update CRP %s status as expected", crpName) @@ -732,6 +761,7 @@ var _ = Context("creating resourceOverride with delete configMap", Ordered, func crpName := fmt.Sprintf(crpNameTemplate, GinkgoParallelProcess()) roName := fmt.Sprintf(roNameTemplate, GinkgoParallelProcess()) roNamespace := fmt.Sprintf(workNamespaceNameTemplate, GinkgoParallelProcess()) + roSnapShotName := fmt.Sprintf(placementv1alpha1.OverrideSnapshotNameFmt, roName, 0) BeforeAll(func() { By("creating work resources") @@ -783,6 +813,10 @@ var _ = Context("creating resourceOverride with delete configMap", Ordered, func } By(fmt.Sprintf("creating resourceOverride %s", roName)) Expect(hubClient.Create(ctx, ro)).To(Succeed(), "Failed to create resourceOverride %s", roName) + Eventually(func() error { + roSnap := &placementv1alpha1.ResourceOverrideSnapshot{} + return hubClient.Get(ctx, types.NamespacedName{Name: roSnapShotName, Namespace: roNamespace}, roSnap) + }, eventuallyDuration, eventuallyInterval).Should(Succeed(), "Failed to update ro as expected", crpName) // Create the CRP. createCRP(crpName) @@ -798,7 +832,7 @@ var _ = Context("creating resourceOverride with delete configMap", Ordered, func It("should update CRP status as expected", func() { wantRONames := []placementv1beta1.NamespacedName{ - {Namespace: roNamespace, Name: fmt.Sprintf(placementv1alpha1.OverrideSnapshotNameFmt, roName, 0)}, + {Namespace: roNamespace, Name: roSnapShotName}, } crpStatusUpdatedActual := crpStatusWithOverrideUpdatedActual(workResourceIdentifiers(), allMemberClusterNames, "0", nil, wantRONames) Eventually(crpStatusUpdatedActual, eventuallyDuration, eventuallyInterval).Should(Succeed(), "Failed to update CRP %s status as expected", crpName) @@ -953,7 +987,30 @@ var _ = Context("creating resourceOverride with a templated rules with cluster l }, eventuallyDuration, eventuallyInterval).Should(Succeed(), "Failed to update resourceOverride %s with non-existent label key", roName) By("Verify the CRP status should have one cluster failed to override while the rest stuck in rollout") - // TODO: need to construct the expected status + Eventually(func() error { + crp := &placementv1beta1.ClusterResourcePlacement{} + if err := hubClient.Get(ctx, types.NamespacedName{Name: crpName}, crp); err != nil { + return err + } + wantCondition := []metav1.Condition{ + { + Type: string(placementv1beta1.ClusterResourcePlacementScheduledConditionType), + Status: metav1.ConditionTrue, + Reason: scheduler.FullyScheduledReason, + ObservedGeneration: crp.Generation, + }, + { + Type: string(placementv1beta1.ClusterResourcePlacementRolloutStartedConditionType), + Status: metav1.ConditionFalse, + Reason: condition.RolloutNotStartedYetReason, + ObservedGeneration: crp.Generation, + }, + } + if diff := cmp.Diff(crp.Status.Conditions, wantCondition, crpStatusCmpOptions...); diff != "" { + return fmt.Errorf("CRP condition diff (-got, +want): %s", diff) + } + return nil + }, eventuallyDuration, eventuallyInterval).Should(Succeed(), "CRP %s failed to show the override failed and stuck in rollout", crpName) By("Verify the configMap remains unchanged") cmName := fmt.Sprintf(appConfigMapNameTemplate, GinkgoParallelProcess()) @@ -979,6 +1036,7 @@ var _ = Context("creating resourceOverride with non-exist label", Ordered, func( crpName := fmt.Sprintf(crpNameTemplate, GinkgoParallelProcess()) roName := fmt.Sprintf(croNameTemplate, GinkgoParallelProcess()) roNamespace := fmt.Sprintf(workNamespaceNameTemplate, GinkgoParallelProcess()) + roSnapShotName := fmt.Sprintf(placementv1alpha1.OverrideSnapshotNameFmt, roName, 0) BeforeAll(func() { By("creating work resources") @@ -1034,6 +1092,10 @@ var _ = Context("creating resourceOverride with non-exist label", Ordered, func( } By(fmt.Sprintf("creating the bad resourceOverride %s", roName)) Expect(hubClient.Create(ctx, ro)).To(Succeed(), "Failed to create resourceOverride %s", roName) + Eventually(func() error { + roSnap := &placementv1alpha1.ResourceOverrideSnapshot{} + return hubClient.Get(ctx, types.NamespacedName{Name: roSnapShotName, Namespace: roNamespace}, roSnap) + }, eventuallyDuration, eventuallyInterval).Should(Succeed(), "Failed to update ro as expected", crpName) // Create the CRP later so that failed override won't block the rollout createCRP(crpName) @@ -1049,7 +1111,7 @@ var _ = Context("creating resourceOverride with non-exist label", Ordered, func( It("should update CRP status as failed to override", func() { wantRONames := []placementv1beta1.NamespacedName{ - {Namespace: roNamespace, Name: fmt.Sprintf(placementv1alpha1.OverrideSnapshotNameFmt, roName, 0)}, + {Namespace: roNamespace, Name: roSnapShotName}, } crpStatusUpdatedActual := crpStatusWithOverrideUpdatedFailedActual(workResourceIdentifiers(), allMemberClusterNames, "0", nil, wantRONames) Eventually(crpStatusUpdatedActual, eventuallyDuration, eventuallyInterval).Should(Succeed(), "Failed to update CRP %s status as expected", crpName) diff --git a/test/e2e/resources/test-envelop-configmap.yaml b/test/e2e/resources/test-envelop-configmap.yaml index 4b948ea02..c88877618 100644 --- a/test/e2e/resources/test-envelop-configmap.yaml +++ b/test/e2e/resources/test-envelop-configmap.yaml @@ -18,35 +18,3 @@ data: requests.memory: 1Gi limits.cpu: "2" limits.memory: 2Gi - webhook.yaml: | - apiVersion: admissionregistration.k8s.io/v1 - kind: MutatingWebhookConfiguration - metadata: - creationTimestamp: null - labels: - azure-workload-identity.io/system: "true" - name: azure-wi-webhook-mutating-webhook-configuration - webhooks: - - admissionReviewVersions: - - v1 - - v1beta1 - clientConfig: - service: - name: azure-wi-webhook-webhook-service - namespace: app - path: /mutate-v1-pod - failurePolicy: Ignore - matchPolicy: Equivalent - name: mutation.azure-workload-identity.io - rules: - - apiGroups: - - "" - apiVersions: - - v1 - operations: - - CREATE - - UPDATE - resources: - - pods - sideEffects: None - timeoutSeconds: 1