From cb9a7a0b305ae8a2875fa9e3ea9fb9f8d134b27b Mon Sep 17 00:00:00 2001 From: Ryan Zhang Date: Wed, 30 Oct 2024 10:55:00 -0700 Subject: [PATCH] fix: handle resource snapshot missing but work already synced and add cro/ro annotation to all the works (#936) Co-authored-by: Ryan Zhang --- apis/cluster/v1beta1/zz_generated.deepcopy.go | 2 +- .../v1alpha1/zz_generated.deepcopy.go | 5 +- apis/placement/v1beta1/commons.go | 9 + .../v1beta1/zz_generated.deepcopy.go | 2 +- apis/v1alpha1/zz_generated.deepcopy.go | 2 +- pkg/controllers/rollout/controller.go | 2 + pkg/controllers/workgenerator/controller.go | 101 +++++++++-- .../controller_integration_test.go | 144 +++++++++++++-- test/apis/v1alpha1/zz_generated.deepcopy.go | 2 +- test/e2e/actuals_test.go | 4 +- test/e2e/rollout_test.go | 168 +++++++++++++++++- 11 files changed, 398 insertions(+), 43 deletions(-) diff --git a/apis/cluster/v1beta1/zz_generated.deepcopy.go b/apis/cluster/v1beta1/zz_generated.deepcopy.go index 6d06cb15b..6c25f0189 100644 --- a/apis/cluster/v1beta1/zz_generated.deepcopy.go +++ b/apis/cluster/v1beta1/zz_generated.deepcopy.go @@ -10,7 +10,7 @@ Licensed under the MIT license. package v1beta1 import ( - "k8s.io/api/core/v1" + v1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" runtime "k8s.io/apimachinery/pkg/runtime" ) diff --git a/apis/placement/v1alpha1/zz_generated.deepcopy.go b/apis/placement/v1alpha1/zz_generated.deepcopy.go index d26fe49a5..1a3f72658 100644 --- a/apis/placement/v1alpha1/zz_generated.deepcopy.go +++ b/apis/placement/v1alpha1/zz_generated.deepcopy.go @@ -10,10 +10,11 @@ Licensed under the MIT license. package v1alpha1 import ( - "go.goms.io/fleet/apis/placement/v1beta1" - "k8s.io/apimachinery/pkg/apis/meta/v1" + v1 "k8s.io/apimachinery/pkg/apis/meta/v1" runtime "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/util/intstr" + + "go.goms.io/fleet/apis/placement/v1beta1" ) // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. diff --git a/apis/placement/v1beta1/commons.go b/apis/placement/v1beta1/commons.go index dce5bb8e8..e805051a0 100644 --- a/apis/placement/v1beta1/commons.go +++ b/apis/placement/v1beta1/commons.go @@ -50,6 +50,15 @@ const ( // The format is {workPrefix}-configMap-uuid WorkNameWithConfigEnvelopeFmt = "%s-configmap-%s" + // ParentClusterResourceOverrideSnapshotHashAnnotation is the annotation to work that contains the hash of the parent cluster resource override snapshot list. + ParentClusterResourceOverrideSnapshotHashAnnotation = fleetPrefix + "parent-cluster-resource-override-snapshot-hash" + + // ParentResourceOverrideSnapshotHashAnnotation is the annotation to work that contains the hash of the parent resource override snapshot list. + ParentResourceOverrideSnapshotHashAnnotation = fleetPrefix + "parent-resource-override-snapshot-hash" + + // ParentResourceSnapshotNameAnnotation is the annotation applied to work that contains the name of the master resource snapshot that generates the work. + ParentResourceSnapshotNameAnnotation = fleetPrefix + "parent-resource-snapshot-name" + // ParentResourceSnapshotIndexLabel is the label applied to work that contains the index of the resource snapshot that generates the work. ParentResourceSnapshotIndexLabel = fleetPrefix + "parent-resource-snapshot-index" diff --git a/apis/placement/v1beta1/zz_generated.deepcopy.go b/apis/placement/v1beta1/zz_generated.deepcopy.go index f20cf8be5..754effe01 100644 --- a/apis/placement/v1beta1/zz_generated.deepcopy.go +++ b/apis/placement/v1beta1/zz_generated.deepcopy.go @@ -10,7 +10,7 @@ Licensed under the MIT license. package v1beta1 import ( - "k8s.io/apimachinery/pkg/apis/meta/v1" + v1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/util/intstr" ) diff --git a/apis/v1alpha1/zz_generated.deepcopy.go b/apis/v1alpha1/zz_generated.deepcopy.go index 0d4061551..ac4844274 100644 --- a/apis/v1alpha1/zz_generated.deepcopy.go +++ b/apis/v1alpha1/zz_generated.deepcopy.go @@ -11,7 +11,7 @@ package v1alpha1 import ( corev1 "k8s.io/api/core/v1" - "k8s.io/apimachinery/pkg/apis/meta/v1" + v1 "k8s.io/apimachinery/pkg/apis/meta/v1" runtime "k8s.io/apimachinery/pkg/runtime" ) diff --git a/pkg/controllers/rollout/controller.go b/pkg/controllers/rollout/controller.go index e94e67189..d2b1674d0 100644 --- a/pkg/controllers/rollout/controller.go +++ b/pkg/controllers/rollout/controller.go @@ -281,6 +281,7 @@ func createUpdateInfo(binding *fleetv1beta1.ClusterResourceBinding, crp *fleetv1 desiredBinding.Spec.ResourceSnapshotName = latestResourceSnapshot.Name // update the resource apply strategy when controller rolls out the new changes desiredBinding.Spec.ApplyStrategy = crp.Spec.Strategy.ApplyStrategy + // TODO: check the size of the cro and ro to not exceed the limit desiredBinding.Spec.ClusterResourceOverrideSnapshots = cro desiredBinding.Spec.ResourceOverrideSnapshots = ro return toBeUpdatedBinding{ @@ -520,6 +521,7 @@ func calculateMaxToAdd(crp *fleetv1beta1.ClusterResourcePlacement, targetNumber upperBoundReadyNumber, "maxNumberOfBindingsToAdd", maxNumberToAdd) return maxNumberToAdd } + func (r *Reconciler) calculateRealTarget(crp *fleetv1beta1.ClusterResourcePlacement, schedulerTargetedBinds []*fleetv1beta1.ClusterResourceBinding) int { crpKObj := klog.KObj(crp) // calculate the target number of bindings diff --git a/pkg/controllers/workgenerator/controller.go b/pkg/controllers/workgenerator/controller.go index 97bb4720f..2c9d367c1 100644 --- a/pkg/controllers/workgenerator/controller.go +++ b/pkg/controllers/workgenerator/controller.go @@ -49,13 +49,14 @@ import ( "go.goms.io/fleet/pkg/utils/controller" "go.goms.io/fleet/pkg/utils/informer" "go.goms.io/fleet/pkg/utils/labels" + "go.goms.io/fleet/pkg/utils/resource" ) var ( // maxFailedResourcePlacementLimit indicates the max number of failed resource placements to include in the status. maxFailedResourcePlacementLimit = 100 - errResourceSnapshotNotFound = errors.New("the master resource snapshot is not found") + errResourceSnapshotNotFound = fmt.Errorf("the master resource snapshot is not found") ) // Reconciler watches binding objects and generate work objects in the designated cluster namespace @@ -135,18 +136,17 @@ func (r *Reconciler) Reconcile(ctx context.Context, req controllerruntime.Reques workUpdated := false overrideSucceeded := false - // Reset the conditions and failed placements. - for i := condition.OverriddenCondition; i < condition.TotalCondition; i++ { - resourceBinding.RemoveCondition(string(i.ResourceBindingConditionType())) - } - resourceBinding.Status.FailedPlacements = nil // list all the corresponding works works, syncErr := r.listAllWorksAssociated(ctx, &resourceBinding) if syncErr == nil { // generate and apply the workUpdated works if we have all the works overrideSucceeded, workUpdated, syncErr = r.syncAllWork(ctx, &resourceBinding, works, cluster) } - + // Reset the conditions and failed placements. + for i := condition.OverriddenCondition; i < condition.TotalCondition; i++ { + resourceBinding.RemoveCondition(string(i.ResourceBindingConditionType())) + } + resourceBinding.Status.FailedPlacements = nil if overrideSucceeded { overrideReason := condition.OverriddenSucceededReason overrideMessage := "Successfully applied the override rules on the resources" @@ -375,10 +375,28 @@ func (r *Reconciler) listAllWorksAssociated(ctx context.Context, resourceBinding func (r *Reconciler) syncAllWork(ctx context.Context, resourceBinding *fleetv1beta1.ClusterResourceBinding, existingWorks map[string]*fleetv1beta1.Work, cluster clusterv1beta1.MemberCluster) (bool, bool, error) { updateAny := atomic.NewBool(false) resourceBindingRef := klog.KObj(resourceBinding) + // the hash256 function can can handle empty list https://go.dev/play/p/_4HW17fooXM + resourceOverrideSnapshotHash, err := resource.HashOf(resourceBinding.Spec.ResourceOverrideSnapshots) + if err != nil { + return false, false, controller.NewUnexpectedBehaviorError(err) + } + clusterResourceOverrideSnapshotHash, err := resource.HashOf(resourceBinding.Spec.ClusterResourceOverrideSnapshots) + if err != nil { + return false, false, controller.NewUnexpectedBehaviorError(err) + } + // TODO: check all work synced first before fetching the snapshots after we put ParentResourceOverrideSnapshotHashAnnotation and ParentClusterResourceOverrideSnapshotHashAnnotation in all the work objects // Gather all the resource resourceSnapshots resourceSnapshots, err := r.fetchAllResourceSnapshots(ctx, resourceBinding) if err != nil { + if errors.Is(err, errResourceSnapshotNotFound) { + // the resourceIndex is deleted but the works might still be up to date with the binding. + if areAllWorkSynced(existingWorks, resourceBinding, resourceOverrideSnapshotHash, clusterResourceOverrideSnapshotHash) { + klog.V(2).InfoS("All the works are synced with the resourceBinding even if the resource snapshot index is removed", "resourceBinding", resourceBindingRef) + return true, false, nil + } + return false, false, controller.NewUserError(err) + } // TODO(RZ): handle errResourceNotFullyCreated error so we don't need to wait for all the snapshots to be created return false, false, err } @@ -422,7 +440,7 @@ func (r *Reconciler) syncAllWork(ctx context.Context, resourceBinding *fleetv1be if uResource.GetObjectKind().GroupVersionKind() == utils.ConfigMapGVK && len(uResource.GetAnnotations()[fleetv1beta1.EnvelopeConfigMapAnnotation]) != 0 { // get a work object for the enveloped configMap - work, err := r.getConfigMapEnvelopWorkObj(ctx, workNamePrefix, resourceBinding, snapshot, &uResource) + work, err := r.getConfigMapEnvelopWorkObj(ctx, workNamePrefix, resourceBinding, snapshot, &uResource, resourceOverrideSnapshotHash, clusterResourceOverrideSnapshotHash) if err != nil { return true, false, err } @@ -438,7 +456,7 @@ func (r *Reconciler) syncAllWork(ctx context.Context, resourceBinding *fleetv1be // generate a work object for the manifests even if there is nothing to place // to allow CRP to collect the status of the placement // TODO (RZ): revisit to see if we need this hack - work := generateSnapshotWorkObj(workNamePrefix, resourceBinding, snapshot, simpleManifests) + work := generateSnapshotWorkObj(workNamePrefix, resourceBinding, snapshot, simpleManifests, resourceOverrideSnapshotHash, clusterResourceOverrideSnapshotHash) activeWork[work.Name] = work newWork = append(newWork, work) @@ -485,6 +503,32 @@ func (r *Reconciler) syncAllWork(ctx context.Context, resourceBinding *fleetv1be return true, updateAny.Load(), nil } +// areAllWorkSynced checks if all the works are synced with the resource binding. +func areAllWorkSynced(existingWorks map[string]*fleetv1beta1.Work, resourceBinding *fleetv1beta1.ClusterResourceBinding, _, _ string) bool { + syncedCondition := resourceBinding.GetCondition(string(fleetv1beta1.ResourceBindingWorkSynchronized)) + if !condition.IsConditionStatusTrue(syncedCondition, resourceBinding.Generation) { + // The binding has to be synced first before we can check the works + return false + } + // TODO: check resourceOverrideSnapshotHash and clusterResourceOverrideSnapshotHash after all the work has the ParentResourceOverrideSnapshotHashAnnotation and ParentClusterResourceOverrideSnapshotHashAnnotation + resourceSnapshotName := resourceBinding.Spec.ResourceSnapshotName + for _, work := range existingWorks { + recordedName, exist := work.Annotations[fleetv1beta1.ParentResourceSnapshotNameAnnotation] + if !exist { + // TODO: remove this block after all the work has the ParentResourceSnapshotNameAnnotation + // the parent resource snapshot name is not recorded in the work, we need to construct it from the labels + crpName := resourceBinding.Labels[fleetv1beta1.CRPTrackingLabel] + index, _ := labels.ExtractResourceSnapshotIndexFromWork(work) + recordedName = fmt.Sprintf(fleetv1beta1.ResourceSnapshotNameFmt, crpName, index) + } + if recordedName != resourceSnapshotName { + klog.V(2).InfoS("The work is not synced with the resourceBinding", "work", klog.KObj(work), "resourceBinding", klog.KObj(resourceBinding), "annotationExist", exist, "recordedName", recordedName, "resourceSnapshotName", resourceSnapshotName) + return false + } + } + return true +} + // fetchAllResourceSnapshots gathers all the resource snapshots for the resource binding. func (r *Reconciler) fetchAllResourceSnapshots(ctx context.Context, resourceBinding *fleetv1beta1.ClusterResourceBinding) (map[string]*fleetv1beta1.ClusterResourceSnapshot, error) { // fetch the master snapshot first @@ -504,7 +548,7 @@ func (r *Reconciler) fetchAllResourceSnapshots(ctx context.Context, resourceBind // getConfigMapEnvelopWorkObj first try to locate a work object for the corresponding envelopObj of type configMap. // we create a new one if the work object doesn't exist. We do this to avoid repeatedly delete and create the same work object. func (r *Reconciler) getConfigMapEnvelopWorkObj(ctx context.Context, workNamePrefix string, resourceBinding *fleetv1beta1.ClusterResourceBinding, - resourceSnapshot *fleetv1beta1.ClusterResourceSnapshot, envelopeObj *unstructured.Unstructured) (*fleetv1beta1.Work, error) { + resourceSnapshot *fleetv1beta1.ClusterResourceSnapshot, envelopeObj *unstructured.Unstructured, resourceOverrideSnapshotHash, clusterResourceOverrideSnapshotHash string) (*fleetv1beta1.Work, error) { // we group all the resources in one configMap to one work manifest, err := extractResFromConfigMap(envelopeObj) if err != nil { @@ -514,6 +558,7 @@ func (r *Reconciler) getConfigMapEnvelopWorkObj(ctx context.Context, workNamePre } klog.V(2).InfoS("Successfully extract the enveloped resources from the configMap", "numOfResources", len(manifest), "snapshot", klog.KObj(resourceSnapshot), "resourceBinding", klog.KObj(resourceBinding), "configMapWrapper", klog.KObj(envelopeObj)) + // Try to see if we already have a work represent the same enveloped object for this CRP in the same cluster // The ParentResourceSnapshotIndexLabel can change between snapshots so we have to exclude that label in the match envelopWorkLabelMatcher := client.MatchingLabels{ @@ -544,6 +589,11 @@ func (r *Reconciler) getConfigMapEnvelopWorkObj(ctx context.Context, workNamePre fleetv1beta1.EnvelopeNameLabel: envelopeObj.GetName(), fleetv1beta1.EnvelopeNamespaceLabel: envelopeObj.GetNamespace(), }, + Annotations: map[string]string{ + fleetv1beta1.ParentResourceSnapshotNameAnnotation: resourceBinding.Spec.ResourceSnapshotName, + fleetv1beta1.ParentResourceOverrideSnapshotHashAnnotation: resourceOverrideSnapshotHash, + fleetv1beta1.ParentClusterResourceOverrideSnapshotHashAnnotation: clusterResourceOverrideSnapshotHash, + }, OwnerReferences: []metav1.OwnerReference{ { APIVersion: fleetv1beta1.GroupVersion.String(), @@ -567,16 +617,19 @@ func (r *Reconciler) getConfigMapEnvelopWorkObj(ctx context.Context, workNamePre klog.ErrorS(controller.NewUnexpectedBehaviorError(fmt.Errorf("find %d work representing configMap", len(workList.Items))), "snapshot", klog.KObj(resourceSnapshot), "resourceBinding", klog.KObj(resourceBinding), "configMapWrapper", klog.KObj(envelopeObj)) } - // we just pick the first one if there are more than one. work := workList.Items[0] work.Labels[fleetv1beta1.ParentResourceSnapshotIndexLabel] = resourceSnapshot.Labels[fleetv1beta1.ResourceIndexLabel] + work.Annotations[fleetv1beta1.ParentResourceSnapshotNameAnnotation] = resourceBinding.Spec.ResourceSnapshotName + work.Annotations[fleetv1beta1.ParentResourceOverrideSnapshotHashAnnotation] = resourceOverrideSnapshotHash + work.Annotations[fleetv1beta1.ParentClusterResourceOverrideSnapshotHashAnnotation] = clusterResourceOverrideSnapshotHash work.Spec.Workload.Manifests = manifest work.Spec.ApplyStrategy = resourceBinding.Spec.ApplyStrategy return &work, nil } // generateSnapshotWorkObj generates the work object for the corresponding snapshot -func generateSnapshotWorkObj(workName string, resourceBinding *fleetv1beta1.ClusterResourceBinding, resourceSnapshot *fleetv1beta1.ClusterResourceSnapshot, manifest []fleetv1beta1.Manifest) *fleetv1beta1.Work { +func generateSnapshotWorkObj(workName string, resourceBinding *fleetv1beta1.ClusterResourceBinding, resourceSnapshot *fleetv1beta1.ClusterResourceSnapshot, + manifest []fleetv1beta1.Manifest, resourceOverrideSnapshotHash, clusterResourceOverrideSnapshotHash string) *fleetv1beta1.Work { return &fleetv1beta1.Work{ ObjectMeta: metav1.ObjectMeta{ Name: workName, @@ -586,6 +639,11 @@ func generateSnapshotWorkObj(workName string, resourceBinding *fleetv1beta1.Clus fleetv1beta1.CRPTrackingLabel: resourceBinding.Labels[fleetv1beta1.CRPTrackingLabel], fleetv1beta1.ParentResourceSnapshotIndexLabel: resourceSnapshot.Labels[fleetv1beta1.ResourceIndexLabel], }, + Annotations: map[string]string{ + fleetv1beta1.ParentResourceSnapshotNameAnnotation: resourceBinding.Spec.ResourceSnapshotName, + fleetv1beta1.ParentResourceOverrideSnapshotHashAnnotation: resourceOverrideSnapshotHash, + fleetv1beta1.ParentClusterResourceOverrideSnapshotHashAnnotation: clusterResourceOverrideSnapshotHash, + }, OwnerReferences: []metav1.OwnerReference{ { APIVersion: fleetv1beta1.GroupVersion.String(), @@ -619,6 +677,7 @@ func (r *Reconciler) upsertWork(ctx context.Context, newWork, existingWork *flee "resourceSnapshot", resourceSnapshotObj, "work", workObj) return true, nil } + // TODO: remove the compare after we did the check on all work in the sync all // check if we need to update the existing work object workResourceIndex, err := labels.ExtractResourceSnapshotIndexFromWork(existingWork) if err != nil { @@ -628,12 +687,20 @@ func (r *Reconciler) upsertWork(ctx context.Context, newWork, existingWork *flee // we already checked the label in fetchAllResourceSnapShots function so no need to check again resourceIndex, _ := labels.ExtractResourceIndexFromClusterResourceSnapshot(resourceSnapshot) if workResourceIndex == resourceIndex { - // no need to do anything if the work is generated from the same resource snapshot group since the resource snapshot is immutable. - klog.V(2).InfoS("Work is already associated with the desired resourceSnapshot", "resourceIndex", resourceIndex, "work", workObj, "resourceSnapshot", resourceSnapshotObj) - return false, nil + // no need to do anything if the work is generated from the same snapshots + if existingWork.Annotations[fleetv1beta1.ParentResourceOverrideSnapshotHashAnnotation] == newWork.Annotations[fleetv1beta1.ParentResourceOverrideSnapshotHashAnnotation] && + existingWork.Annotations[fleetv1beta1.ParentClusterResourceOverrideSnapshotHashAnnotation] == newWork.Annotations[fleetv1beta1.ParentClusterResourceOverrideSnapshotHashAnnotation] { + klog.V(2).InfoS("Work is not associated with the desired override snapshots", "existingROHash", existingWork.Annotations[fleetv1beta1.ParentResourceOverrideSnapshotHashAnnotation], + "existingCROHash", existingWork.Annotations[fleetv1beta1.ParentClusterResourceOverrideSnapshotHashAnnotation], "work", workObj) + return false, nil + } + klog.V(2).InfoS("Work is already associated with the desired resourceSnapshot but still not having the right override snapshots", "resourceIndex", resourceIndex, "work", workObj, "resourceSnapshot", resourceSnapshotObj) } - // need to update the existing work, only two possible changes: - existingWork.Labels[fleetv1beta1.ParentResourceSnapshotIndexLabel] = resourceSnapshot.Labels[fleetv1beta1.ResourceIndexLabel] + // need to copy the new work to the existing work, only 5 possible changes: + existingWork.Labels[fleetv1beta1.ParentResourceSnapshotIndexLabel] = newWork.Labels[fleetv1beta1.ParentResourceSnapshotIndexLabel] + existingWork.Annotations[fleetv1beta1.ParentResourceSnapshotNameAnnotation] = newWork.Annotations[fleetv1beta1.ParentResourceSnapshotNameAnnotation] + existingWork.Annotations[fleetv1beta1.ParentResourceOverrideSnapshotHashAnnotation] = newWork.Annotations[fleetv1beta1.ParentResourceOverrideSnapshotHashAnnotation] + existingWork.Annotations[fleetv1beta1.ParentClusterResourceOverrideSnapshotHashAnnotation] = newWork.Annotations[fleetv1beta1.ParentClusterResourceOverrideSnapshotHashAnnotation] existingWork.Spec.Workload.Manifests = newWork.Spec.Workload.Manifests if err := r.Client.Update(ctx, existingWork); err != nil { klog.ErrorS(err, "Failed to update the work associated with the resourceSnapshot", "resourceSnapshot", resourceSnapshotObj, "work", workObj) diff --git a/pkg/controllers/workgenerator/controller_integration_test.go b/pkg/controllers/workgenerator/controller_integration_test.go index cf2e39925..40e60c726 100644 --- a/pkg/controllers/workgenerator/controller_integration_test.go +++ b/pkg/controllers/workgenerator/controller_integration_test.go @@ -6,6 +6,8 @@ Licensed under the MIT license. package workgenerator import ( + "crypto/sha256" + "encoding/json" "fmt" "strconv" "time" @@ -64,13 +66,14 @@ const ( ) var _ = Describe("Test Work Generator Controller", func() { - Context("Test Bound ClusterResourceBinding", func() { var binding *placementv1beta1.ClusterResourceBinding ignoreTypeMeta := cmpopts.IgnoreFields(metav1.TypeMeta{}, "Kind", "APIVersion") ignoreWorkOption := cmpopts.IgnoreFields(metav1.ObjectMeta{}, "UID", "ResourceVersion", "ManagedFields", "CreationTimestamp", "Generation") - + var emptyArray []string + jsonBytes, _ := json.Marshal(emptyArray) + emptyHash := fmt.Sprintf("%x", sha256.Sum256(jsonBytes)) BeforeEach(func() { memberClusterName = "cluster-" + utils.RandStr() testCRPName = "crp" + utils.RandStr() @@ -211,7 +214,7 @@ var _ = Describe("Test Work Generator Controller", func() { By(fmt.Sprintf("work %s is created in %s", work.Name, work.Namespace)) }) - It("Should handle the case that the snapshot is deleted", func() { + It("Should handle the case that the binding is deleted", func() { // generate master resource snapshot masterSnapshot := generateResourceSnapshot(1, 1, 0, [][]byte{ testResourceCRD, testNameSpace, testResource, @@ -306,6 +309,11 @@ var _ = Describe("Test Work Generator Controller", func() { placementv1beta1.ParentBindingLabel: binding.Name, placementv1beta1.ParentResourceSnapshotIndexLabel: "1", }, + Annotations: map[string]string{ + placementv1beta1.ParentResourceSnapshotNameAnnotation: binding.Spec.ResourceSnapshotName, + placementv1beta1.ParentClusterResourceOverrideSnapshotHashAnnotation: emptyHash, + placementv1beta1.ParentResourceOverrideSnapshotHashAnnotation: emptyHash, + }, }, Spec: placementv1beta1.WorkSpec{ Workload: placementv1beta1.WorkloadTemplate{ @@ -359,7 +367,7 @@ var _ = Describe("Test Work Generator Controller", func() { verifyBindingStatusSyncedNotApplied(binding, false, false) }) - Context("Test Bound ClusterResourceBinding with failed to apply manifests", func() { + Context("Test Bound ClusterResourceBinding with manifests go from not applied to available", func() { work := placementv1beta1.Work{} BeforeEach(func() { // check the binding status till the bound condition is true @@ -395,6 +403,11 @@ var _ = Describe("Test Work Generator Controller", func() { placementv1beta1.ParentBindingLabel: binding.Name, placementv1beta1.ParentResourceSnapshotIndexLabel: "1", }, + Annotations: map[string]string{ + placementv1beta1.ParentResourceSnapshotNameAnnotation: binding.Spec.ResourceSnapshotName, + placementv1beta1.ParentClusterResourceOverrideSnapshotHashAnnotation: emptyHash, + placementv1beta1.ParentResourceOverrideSnapshotHashAnnotation: emptyHash, + }, }, Spec: placementv1beta1.WorkSpec{ Workload: placementv1beta1.WorkloadTemplate{ @@ -441,6 +454,59 @@ var _ = Describe("Test Work Generator Controller", func() { // check the binding status that it should be marked as available true eventually verifyBindStatusAvail(binding, false) }) + + It("Should continue to update the binding status even if the master resource snapshot is deleted after the work is synced", func() { + // delete the snapshot after the work is synced with binding + Expect(k8sClient.Delete(ctx, masterSnapshot)).Should(SatisfyAny(Succeed(), utils.NotFoundMatcher{})) + // mark the work applied which should trigger a reconcile loop and copy the status from the work to the binding + markWorkApplied(&work) + // check the binding status that it should be marked as applied true eventually + verifyBindStatusAppliedNotAvailable(binding, false) + // delete the ParentResourceSnapshotNameAnnotation after the work is synced with binding + Expect(k8sClient.Get(ctx, types.NamespacedName{Name: fmt.Sprintf(placementv1beta1.FirstWorkNameFmt, testCRPName), Namespace: memberClusterNamespaceName}, &work)).Should(Succeed()) + delete(work.Annotations, placementv1beta1.ParentResourceSnapshotNameAnnotation) + Expect(k8sClient.Update(ctx, &work)).Should(Succeed()) + // mark the work available which should trigger a reconcile loop and copy the status from the work to the binding even if the work has no annotation + markWorkAvailable(&work) + // check the binding status that it should be marked as available true eventually + verifyBindStatusAvail(binding, false) + }) + + It("Should mark the binding as failed to sync if the master resource snapshot does not exist and the work do not sync ", func() { + // delete the snapshot after the work is synced with binding + Expect(k8sClient.Delete(ctx, masterSnapshot)).Should(SatisfyAny(Succeed(), utils.NotFoundMatcher{})) + // mark the work applied which should trigger a reconcile loop and copy the status from the work to the binding + markWorkApplied(&work) + // check the binding status that it should be marked as applied true eventually + verifyBindStatusAppliedNotAvailable(binding, false) + Expect(k8sClient.Get(ctx, types.NamespacedName{Name: binding.Name}, binding)).Should(Succeed()) + // update the resource snapshot to the next version that doesn't exist + binding.Spec.ResourceSnapshotName = "next" + Expect(k8sClient.Update(ctx, binding)).Should(Succeed()) + updateRolloutStartedGeneration(&binding) + // check the binding status that it should be marked as override succeed but not synced + Eventually(func() string { + Expect(k8sClient.Get(ctx, types.NamespacedName{Name: binding.Name}, binding)).Should(Succeed()) + wantStatus := placementv1beta1.ResourceBindingStatus{ + Conditions: []metav1.Condition{ + { + Type: string(placementv1beta1.ResourceBindingRolloutStarted), + Status: metav1.ConditionTrue, + Reason: condition.RolloutStartedReason, + ObservedGeneration: binding.GetGeneration(), + }, + { + Type: string(placementv1beta1.ResourceBindingOverridden), + Status: metav1.ConditionFalse, + Reason: condition.OverriddenFailedReason, + ObservedGeneration: binding.GetGeneration(), + }, + }, + FailedPlacements: nil, + } + return cmp.Diff(wantStatus, binding.Status, cmpConditionOption) + }, timeout, interval).Should(BeEmpty(), fmt.Sprintf("binding(%s) mismatch (-want +got)", binding.Name)) + }) }) }) @@ -492,6 +558,11 @@ var _ = Describe("Test Work Generator Controller", func() { placementv1beta1.ParentBindingLabel: binding.Name, placementv1beta1.ParentResourceSnapshotIndexLabel: "1", }, + Annotations: map[string]string{ + placementv1beta1.ParentResourceSnapshotNameAnnotation: binding.Spec.ResourceSnapshotName, + placementv1beta1.ParentClusterResourceOverrideSnapshotHashAnnotation: emptyHash, + placementv1beta1.ParentResourceOverrideSnapshotHashAnnotation: emptyHash, + }, }, Spec: placementv1beta1.WorkSpec{ Workload: placementv1beta1.WorkloadTemplate{ @@ -531,6 +602,11 @@ var _ = Describe("Test Work Generator Controller", func() { placementv1beta1.EnvelopeNameLabel: "envelop-configmap", placementv1beta1.EnvelopeNamespaceLabel: "app", }, + Annotations: map[string]string{ + placementv1beta1.ParentResourceSnapshotNameAnnotation: binding.Spec.ResourceSnapshotName, + placementv1beta1.ParentClusterResourceOverrideSnapshotHashAnnotation: emptyHash, + placementv1beta1.ParentResourceOverrideSnapshotHashAnnotation: emptyHash, + }, }, Spec: placementv1beta1.WorkSpec{ Workload: placementv1beta1.WorkloadTemplate{ @@ -609,6 +685,11 @@ var _ = Describe("Test Work Generator Controller", func() { placementv1beta1.ParentBindingLabel: binding.Name, placementv1beta1.ParentResourceSnapshotIndexLabel: "2", }, + Annotations: map[string]string{ + placementv1beta1.ParentResourceSnapshotNameAnnotation: binding.Spec.ResourceSnapshotName, + placementv1beta1.ParentClusterResourceOverrideSnapshotHashAnnotation: emptyHash, + placementv1beta1.ParentResourceOverrideSnapshotHashAnnotation: emptyHash, + }, }, Spec: placementv1beta1.WorkSpec{ Workload: placementv1beta1.WorkloadTemplate{ @@ -647,6 +728,11 @@ var _ = Describe("Test Work Generator Controller", func() { placementv1beta1.EnvelopeNameLabel: "envelop-configmap", placementv1beta1.EnvelopeNamespaceLabel: "app", }, + Annotations: map[string]string{ + placementv1beta1.ParentResourceSnapshotNameAnnotation: binding.Spec.ResourceSnapshotName, + placementv1beta1.ParentClusterResourceOverrideSnapshotHashAnnotation: emptyHash, + placementv1beta1.ParentResourceOverrideSnapshotHashAnnotation: emptyHash, + }, }, Spec: placementv1beta1.WorkSpec{ Workload: placementv1beta1.WorkloadTemplate{ @@ -787,6 +873,11 @@ var _ = Describe("Test Work Generator Controller", func() { placementv1beta1.ParentResourceSnapshotIndexLabel: "2", placementv1beta1.ParentBindingLabel: binding.Name, }, + Annotations: map[string]string{ + placementv1beta1.ParentResourceSnapshotNameAnnotation: binding.Spec.ResourceSnapshotName, + placementv1beta1.ParentClusterResourceOverrideSnapshotHashAnnotation: emptyHash, + placementv1beta1.ParentResourceOverrideSnapshotHashAnnotation: emptyHash, + }, }, Spec: placementv1beta1.WorkSpec{ Workload: placementv1beta1.WorkloadTemplate{ @@ -857,6 +948,11 @@ var _ = Describe("Test Work Generator Controller", func() { placementv1beta1.ParentResourceSnapshotIndexLabel: "2", placementv1beta1.ParentBindingLabel: binding.Name, }, + Annotations: map[string]string{ + placementv1beta1.ParentResourceSnapshotNameAnnotation: binding.Spec.ResourceSnapshotName, + placementv1beta1.ParentClusterResourceOverrideSnapshotHashAnnotation: emptyHash, + placementv1beta1.ParentResourceOverrideSnapshotHashAnnotation: emptyHash, + }, }, Spec: placementv1beta1.WorkSpec{ Workload: placementv1beta1.WorkloadTemplate{ @@ -1082,6 +1178,7 @@ var _ = Describe("Test Work Generator Controller", func() { Context("Test Bound ClusterResourceBinding with a single resource snapshot and valid overrides", func() { var masterSnapshot *placementv1beta1.ClusterResourceSnapshot + var roHash, croHash string BeforeEach(func() { masterSnapshot = generateResourceSnapshot(1, 1, 0, [][]byte{ @@ -1089,21 +1186,25 @@ var _ = Describe("Test Work Generator Controller", func() { }) Expect(k8sClient.Create(ctx, masterSnapshot)).Should(Succeed()) By(fmt.Sprintf("master resource snapshot %s created", masterSnapshot.Name)) - spec := placementv1beta1.ResourceBindingSpec{ - State: placementv1beta1.BindingStateBound, - ResourceSnapshotName: masterSnapshot.Name, - TargetCluster: memberClusterName, - ClusterResourceOverrideSnapshots: []string{ - validClusterResourceOverrideSnapshotName, - }, - ResourceOverrideSnapshots: []placementv1beta1.NamespacedName{ - { - Name: validResourceOverrideSnapshotName, - Namespace: appNamespaceName, - }, + crolist := []string{validClusterResourceOverrideSnapshotName} + roList := []placementv1beta1.NamespacedName{ + { + Name: validResourceOverrideSnapshotName, + Namespace: appNamespaceName, }, } + spec := placementv1beta1.ResourceBindingSpec{ + State: placementv1beta1.BindingStateBound, + ResourceSnapshotName: masterSnapshot.Name, + TargetCluster: memberClusterName, + ClusterResourceOverrideSnapshots: crolist, + ResourceOverrideSnapshots: roList, + } createClusterResourceBinding(&binding, spec) + jsonBytes, _ := json.Marshal(roList) + roHash = fmt.Sprintf("%x", sha256.Sum256(jsonBytes)) + jsonBytes, _ = json.Marshal(crolist) + croHash = fmt.Sprintf("%x", sha256.Sum256(jsonBytes)) }) AfterEach(func() { @@ -1146,6 +1247,11 @@ var _ = Describe("Test Work Generator Controller", func() { placementv1beta1.ParentBindingLabel: binding.Name, placementv1beta1.ParentResourceSnapshotIndexLabel: "1", }, + Annotations: map[string]string{ + placementv1beta1.ParentResourceSnapshotNameAnnotation: binding.Spec.ResourceSnapshotName, + placementv1beta1.ParentClusterResourceOverrideSnapshotHashAnnotation: croHash, + placementv1beta1.ParentResourceOverrideSnapshotHashAnnotation: roHash, + }, }, Spec: placementv1beta1.WorkSpec{ Workload: placementv1beta1.WorkloadTemplate{ @@ -1293,7 +1399,6 @@ var _ = Describe("Test Work Generator Controller", func() { // binding should have a finalizer Expect(k8sClient.Get(ctx, types.NamespacedName{Name: binding.Name}, binding)).Should(Succeed()) Expect(len(binding.Finalizers)).Should(Equal(1)) - Eventually(func() string { Expect(k8sClient.Get(ctx, types.NamespacedName{Name: binding.Name}, binding)).Should(Succeed()) wantStatus := placementv1beta1.ResourceBindingStatus{ @@ -1374,6 +1479,11 @@ var _ = Describe("Test Work Generator Controller", func() { placementv1beta1.ParentBindingLabel: binding.Name, placementv1beta1.ParentResourceSnapshotIndexLabel: "1", }, + Annotations: map[string]string{ + placementv1beta1.ParentResourceSnapshotNameAnnotation: binding.Spec.ResourceSnapshotName, + placementv1beta1.ParentClusterResourceOverrideSnapshotHashAnnotation: emptyHash, + placementv1beta1.ParentResourceOverrideSnapshotHashAnnotation: emptyHash, + }, }, Spec: placementv1beta1.WorkSpec{ Workload: placementv1beta1.WorkloadTemplate{ diff --git a/test/apis/v1alpha1/zz_generated.deepcopy.go b/test/apis/v1alpha1/zz_generated.deepcopy.go index 0b5d2e30b..ef7e4433a 100644 --- a/test/apis/v1alpha1/zz_generated.deepcopy.go +++ b/test/apis/v1alpha1/zz_generated.deepcopy.go @@ -10,7 +10,7 @@ Licensed under the MIT license. package v1alpha1 import ( - "k8s.io/apimachinery/pkg/apis/meta/v1" + v1 "k8s.io/apimachinery/pkg/apis/meta/v1" runtime "k8s.io/apimachinery/pkg/runtime" ) diff --git a/test/e2e/actuals_test.go b/test/e2e/actuals_test.go index a9698c4e0..dddf87dd4 100644 --- a/test/e2e/actuals_test.go +++ b/test/e2e/actuals_test.go @@ -420,7 +420,7 @@ func resourcePlacementRolloutCompletedConditions(generation int64, resourceIsTra } } -func resourcePlacementRolloutFailedConditions(generation int64) []metav1.Condition { +func resourcePlacementScheduleFailedConditions(generation int64) []metav1.Condition { return []metav1.Condition{ { Type: string(placementv1beta1.ResourceScheduledConditionType), @@ -689,7 +689,7 @@ func customizedCRPStatusUpdatedActual(crpName string, } for i := 0; i < len(wantUnselectedClusters); i++ { wantPlacementStatus = append(wantPlacementStatus, placementv1beta1.ResourcePlacementStatus{ - Conditions: resourcePlacementRolloutFailedConditions(crp.Generation), + Conditions: resourcePlacementScheduleFailedConditions(crp.Generation), }) } diff --git a/test/e2e/rollout_test.go b/test/e2e/rollout_test.go index 1db441dfa..7537c9dd9 100644 --- a/test/e2e/rollout_test.go +++ b/test/e2e/rollout_test.go @@ -17,6 +17,7 @@ import ( batchv1 "k8s.io/api/batch/v1" corev1 "k8s.io/api/core/v1" apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" + "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/intstr" @@ -472,6 +473,162 @@ var _ = Describe("placing wrapped resources using a CRP", Ordered, func() { }) }) + Context("Test a CRP place workload successful and update it to be failed and then delete the resource snapshot,"+ + "rollout should eventually be successful after we correct the image", Ordered, func() { + crpName := fmt.Sprintf(crpNameTemplate, GinkgoParallelProcess()) + workNamespace := appNamespace() + var wantSelectedResources []placementv1beta1.ResourceIdentifier + var testDeployment appv1.Deployment + + BeforeAll(func() { + // Create the test resources. + readDeploymentTestManifest(&testDeployment) + wantSelectedResources = []placementv1beta1.ResourceIdentifier{ + { + Kind: utils.NamespaceKind, + Name: workNamespace.Name, + Version: corev1.SchemeGroupVersion.Version, + }, + { + Group: appv1.SchemeGroupVersion.Group, + Version: appv1.SchemeGroupVersion.Version, + Kind: utils.DeploymentKind, + Name: testDeployment.Name, + Namespace: workNamespace.Name, + }, + } + }) + + It("create the deployment resource in the namespace", func() { + Expect(hubClient.Create(ctx, &workNamespace)).To(Succeed(), "Failed to create namespace %s", workNamespace.Name) + testDeployment.Namespace = workNamespace.Name + Expect(hubClient.Create(ctx, &testDeployment)).To(Succeed(), "Failed to create test deployment %s", testDeployment.Name) + }) + + It("create the CRP that select the namespace", func() { + crp := buildCRPForSafeRollout() + crp.Spec.RevisionHistoryLimit = ptr.To(int32(1)) + Expect(hubClient.Create(ctx, crp)).To(Succeed(), "Failed to create CRP") + }) + + It("should update CRP status as expected", func() { + crpStatusUpdatedActual := crpStatusUpdatedActual(wantSelectedResources, allMemberClusterNames, nil, "0") + 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 := waitForDeploymentPlacementToReady(memberCluster, &testDeployment) + Eventually(workResourcesPlacedActual, eventuallyDuration, eventuallyInterval).Should(Succeed(), "Failed to place work resources on member cluster %s", memberCluster.ClusterName) + } + }) + + It("change the image name in deployment, to make it unavailable", func() { + Eventually(func() error { + var dep appv1.Deployment + err := hubClient.Get(ctx, types.NamespacedName{Name: testDeployment.Name, Namespace: testDeployment.Namespace}, &dep) + if err != nil { + return err + } + dep.Spec.Template.Spec.Containers[0].Image = randomImageName + return hubClient.Update(ctx, &dep) + }, eventuallyDuration, eventuallyInterval).Should(Succeed(), "Failed to change the image name in deployment") + }) + + It("should update CRP status on deployment failed as expected", func() { + failedDeploymentResourceIdentifier := placementv1beta1.ResourceIdentifier{ + Group: appv1.SchemeGroupVersion.Group, + Version: appv1.SchemeGroupVersion.Version, + Kind: utils.DeploymentKind, + Name: testDeployment.Name, + Namespace: testDeployment.Namespace, + } + crpStatusActual := safeRolloutWorkloadCRPStatusUpdatedActual(wantSelectedResources, failedDeploymentResourceIdentifier, allMemberClusterNames, "1", 2) + Eventually(crpStatusActual, eventuallyDuration, eventuallyInterval).Should(Succeed(), "Failed to update CRP status as expected") + }) + + It("update work to trigger a work generator reconcile", func() { + for idx := range allMemberClusters { + memberCluster := allMemberClusters[idx].ClusterName + namespaceName := fmt.Sprintf(utils.NamespaceNameFormat, memberCluster) + workName := fmt.Sprintf(placementv1beta1.FirstWorkNameFmt, crpName) + work := placementv1beta1.Work{} + Expect(hubClient.Get(ctx, types.NamespacedName{Name: workName, Namespace: namespaceName}, &work)).Should(Succeed(), "Failed to get the work") + if work.Status.ManifestConditions != nil { + work.Status.ManifestConditions = nil + } else { + meta.SetStatusCondition(&work.Status.Conditions, metav1.Condition{ + Type: placementv1beta1.WorkConditionTypeAvailable, + Status: metav1.ConditionFalse, + Reason: "WorkNotAvailable", + }) + } + Expect(hubClient.Status().Update(ctx, &work)).Should(Succeed(), "Failed to update the work") + } + }) + + It("change the image name in deployment, to roll over the resourcesnapshot", func() { + crsList := &placementv1beta1.ClusterResourceSnapshotList{} + Expect(hubClient.List(ctx, crsList, client.MatchingLabels{placementv1beta1.CRPTrackingLabel: crpName})).Should(Succeed(), "Failed to list the resourcesnapshot") + Expect(len(crsList.Items) == 1).Should(BeTrue()) + oldCRS := crsList.Items[0].Name + Expect(hubClient.Get(ctx, types.NamespacedName{Name: testDeployment.Name, Namespace: testDeployment.Namespace}, &testDeployment)).Should(Succeed(), "Failed to get deployment") + testDeployment.Spec.Template.Spec.Containers[0].Image = "extra-snapshot" + Expect(hubClient.Update(ctx, &testDeployment)).Should(Succeed(), "Failed to change the image name in deployment") + // wait for the new resourcesnapshot to be created + Eventually(func() bool { + Expect(hubClient.List(ctx, crsList, client.MatchingLabels{placementv1beta1.CRPTrackingLabel: crpName})).Should(Succeed(), "Failed to list the resourcesnapshot") + Expect(len(crsList.Items) == 1).Should(BeTrue()) + return crsList.Items[0].Name != oldCRS + }, eventuallyDuration, eventuallyInterval).Should(BeTrue(), "Failed to remove the old resourcensnapshot") + }) + + It("update work to trigger a work generator reconcile", func() { + for idx := range allMemberClusters { + memberCluster := allMemberClusters[idx].ClusterName + namespaceName := fmt.Sprintf(utils.NamespaceNameFormat, memberCluster) + workName := fmt.Sprintf(placementv1beta1.FirstWorkNameFmt, crpName) + work := placementv1beta1.Work{} + Expect(hubClient.Get(ctx, types.NamespacedName{Name: workName, Namespace: namespaceName}, &work)).Should(Succeed(), "Failed to get the work") + if work.Status.ManifestConditions != nil { + work.Status.ManifestConditions = nil + } else { + meta.SetStatusCondition(&work.Status.Conditions, metav1.Condition{ + Type: placementv1beta1.WorkConditionTypeAvailable, + Status: metav1.ConditionFalse, + Reason: "WorkNotAvailable", + }) + } + Expect(hubClient.Status().Update(ctx, &work)).Should(Succeed(), "Failed to update the work") + } + }) + + It("change the image name in deployment, to make it available again", func() { + Eventually(func() error { + err := hubClient.Get(ctx, types.NamespacedName{Name: testDeployment.Name, Namespace: testDeployment.Namespace}, &testDeployment) + if err != nil { + return err + } + testDeployment.Spec.Template.Spec.Containers[0].Image = "1.26.2" + return hubClient.Update(ctx, &testDeployment) + }, eventuallyDuration, eventuallyInterval).Should(Succeed(), "Failed to change the image name in deployment") + }) + + It("should place the resources on all member clusters", func() { + for idx := range allMemberClusters { + memberCluster := allMemberClusters[idx] + workResourcesPlacedActual := waitForDeploymentPlacementToReady(memberCluster, &testDeployment) + Eventually(workResourcesPlacedActual, eventuallyDuration, eventuallyInterval).Should(Succeed(), "Failed to place work resources on member cluster %s", memberCluster.ClusterName) + } + }) + + AfterAll(func() { + // Remove the custom deletion blocker finalizer from the CRP. + ensureCRPAndRelatedResourcesDeletion(crpName, allMemberClusters) + }) + }) + Context("Test a CRP place workload objects successfully, don't block rollout based on job availability", Ordered, func() { crpName := fmt.Sprintf(crpNameTemplate, GinkgoParallelProcess()) workNamespace := appNamespace() @@ -821,9 +978,12 @@ func waitForDeploymentPlacementToReady(memberCluster *framework.Cluster, testDep } } if placedDeployment.Status.ObservedGeneration == placedDeployment.Generation && depCond != nil && depCond.Status == corev1.ConditionTrue { + if placedDeployment.Spec.Template.Spec.Containers[0].Image != testDeployment.Spec.Template.Spec.Containers[0].Image { + return fmt.Errorf("deployment spec`%s` is not updated, placedDeployment = %+v, testDeployment = %+v", testDeployment.Name, placedDeployment.Spec, testDeployment.Spec) + } return nil } - return nil + return fmt.Errorf("deployment `%s` is not updated", testDeployment.Name) } } @@ -841,6 +1001,9 @@ func waitForDaemonSetPlacementToReady(memberCluster *framework.Cluster, testDaem if placedDaemonSet.Status.ObservedGeneration == placedDaemonSet.Generation && placedDaemonSet.Status.NumberAvailable == placedDaemonSet.Status.DesiredNumberScheduled && placedDaemonSet.Status.CurrentNumberScheduled == placedDaemonSet.Status.UpdatedNumberScheduled { + if placedDaemonSet.Spec.Template.Spec.Containers[0].Image != testDaemonSet.Spec.Template.Spec.Containers[0].Image { + return fmt.Errorf("daemonSet spec`%s` is not updated", testDaemonSet.Name) + } return nil } return errors.New("daemonset is not ready") @@ -861,6 +1024,9 @@ func waitForStatefulSetPlacementToReady(memberCluster *framework.Cluster, testSt if placedStatefulSet.Status.ObservedGeneration == placedStatefulSet.Generation && placedStatefulSet.Status.CurrentReplicas == *placedStatefulSet.Spec.Replicas && placedStatefulSet.Status.CurrentReplicas == placedStatefulSet.Status.UpdatedReplicas { + if placedStatefulSet.Spec.Template.Spec.Containers[0].Image != testStatefulSet.Spec.Template.Spec.Containers[0].Image { + return fmt.Errorf("statefulSet spec`%s` is not updated", placedStatefulSet.Name) + } return nil } return errors.New("statefulset is not ready")