diff --git a/controllers/resourcesummary_collection.go b/controllers/resourcesummary_collection.go index 72f4b7c7..168ae548 100644 --- a/controllers/resourcesummary_collection.go +++ b/controllers/resourcesummary_collection.go @@ -749,18 +749,14 @@ func containsDriftedResource(resources []configv1beta1.DriftedResourceRef, resou return false } +// resetResourceSummaryStatus clears the drift recorded in rs, the ResourceSummary that has been processed. +// The update carries the resourceVersion of rs, so if drift-detection-manager reported a drift after rs was +// read, the update fails with a Conflict instead of clearing a drift that was never processed. The next +// collection reads the ResourceSummary again and processes it. func resetResourceSummaryStatus(ctx context.Context, remoteClient client.Client, rs *libsveltosv1beta1.ResourceSummary, logger logr.Logger) error { - resourceSummary := &libsveltosv1beta1.ResourceSummary{} - - err := remoteClient.Get(ctx, types.NamespacedName{Namespace: rs.Namespace, Name: rs.Name}, resourceSummary) - if err != nil { - if apierrors.IsNotFound(err) { - return nil - } - return err - } + resourceSummary := rs.DeepCopy() logger.V(logs.LogDebug).Info("reset resourceSummary status") resourceSummary.Status.ResourcesChanged = false @@ -771,7 +767,12 @@ func resetResourceSummaryStatus(ctx context.Context, remoteClient client.Client, // a resource already in it, so keeping the entries would pin their detection time to the first drift. resourceSummary.Status.DriftedResources = nil resourceSummary.Status.DriftedResourcesTruncated = false - return remoteClient.Status().Update(ctx, resourceSummary) + + err := remoteClient.Status().Update(ctx, resourceSummary) + if err != nil && apierrors.IsNotFound(err) { + return nil + } + return err } // markDriftedHelmCharts sets NeedsRedeploy on the HelmReleaseSummaries entries matching diff --git a/controllers/resourcesummary_collection_test.go b/controllers/resourcesummary_collection_test.go index a20e1ac7..8e832311 100644 --- a/controllers/resourcesummary_collection_test.go +++ b/controllers/resourcesummary_collection_test.go @@ -268,6 +268,86 @@ var _ = Describe("ResourceSummary Collection", func() { }, timeout, pollingInterval).Should(BeTrue()) }) + It("processResourceSummary does not clear a drift reported after the ResourceSummary was listed", func() { + cluster := prepareCluster() + + clusterSummary := &configv1beta1.ClusterSummary{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: cluster.Namespace, + Name: clusterProfileNamePrefix + randomString(), + Labels: map[string]string{clusterops.ClusterProfileLabelName: randomString()}, + }, + Spec: configv1beta1.ClusterSummarySpec{ + ClusterType: libsveltosv1beta1.ClusterTypeCapi, + }, + } + Expect(testEnv.Create(context.TODO(), clusterSummary)).To(Succeed()) + Expect(waitForObject(context.TODO(), testEnv.Client, clusterSummary)).To(Succeed()) + + currentClusterSummary := &configv1beta1.ClusterSummary{} + Expect(testEnv.Get(context.TODO(), + types.NamespacedName{Namespace: clusterSummary.Namespace, Name: clusterSummary.Name}, + currentClusterSummary)).To(Succeed()) + currentClusterSummary.Status.FeatureSummaries = []configv1beta1.FeatureSummary{ + { + FeatureID: libsveltosv1beta1.FeatureResources, + Status: libsveltosv1beta1.FeatureStatusProvisioned, + Hash: []byte(randomString()), + }, + { + FeatureID: libsveltosv1beta1.FeatureHelm, + Status: libsveltosv1beta1.FeatureStatusProvisioned, + Hash: []byte(randomString()), + }, + } + Expect(testEnv.Status().Update(context.TODO(), currentClusterSummary)).To(Succeed()) + + resourceSummary := getResourceSummary() + resourceSummary.Annotations = map[string]string{ + libsveltosv1beta1.ClusterSummaryNameAnnotation: clusterSummary.Name, + libsveltosv1beta1.ClusterSummaryNamespaceAnnotation: clusterSummary.Namespace, + } + Expect(testEnv.Create(context.TODO(), resourceSummary)).To(Succeed()) + Expect(waitForObject(context.TODO(), testEnv.Client, resourceSummary)).To(Succeed()) + + resourceSummaryKey := types.NamespacedName{Namespace: resourceSummary.Namespace, Name: resourceSummary.Name} + + // The copy the collector listed: only resourcesChanged is set + listedResourceSummary := &libsveltosv1beta1.ResourceSummary{} + Expect(testEnv.Get(context.TODO(), resourceSummaryKey, listedResourceSummary)).To(Succeed()) + listedResourceSummary.Status.ResourcesChanged = true + Expect(testEnv.Status().Update(context.TODO(), listedResourceSummary)).To(Succeed()) + + // drift-detection-manager reports a Helm drift after the ResourceSummary was listed + // Eventual loop so testEnv Cache is synced and the update below does not use a stale resourceVersion + latestResourceSummary := &libsveltosv1beta1.ResourceSummary{} + Eventually(func() bool { + err := testEnv.Get(context.TODO(), resourceSummaryKey, latestResourceSummary) + return err == nil && latestResourceSummary.Status.ResourcesChanged + }, timeout, pollingInterval).Should(BeTrue()) + latestResourceSummary.Status.HelmResourcesChanged = true + Expect(testEnv.Status().Update(context.TODO(), latestResourceSummary)).To(Succeed()) + + // Processing the stale copy must not reset the ResourceSummary: the Helm drift was never processed + err := controllers.ProcessResourceSummary(context.TODO(), testEnv.Client, listedResourceSummary, + textlogger.NewLogger(textlogger.NewConfig())) + Expect(err).ToNot(BeNil()) + Expect(apierrors.IsConflict(err)).To(BeTrue()) + + // Eventual loop so testEnv Cache is synced + Eventually(func() bool { + currentResourceSummary := &libsveltosv1beta1.ResourceSummary{} + err := testEnv.Get(context.TODO(), resourceSummaryKey, currentResourceSummary) + return err == nil && currentResourceSummary.Status.HelmResourcesChanged + }, timeout, pollingInterval).Should(BeTrue()) + + Consistently(func() bool { + currentResourceSummary := &libsveltosv1beta1.ResourceSummary{} + err := testEnv.Get(context.TODO(), resourceSummaryKey, currentResourceSummary) + return err == nil && currentResourceSummary.Status.HelmResourcesChanged + }, 2*time.Second, pollingInterval).Should(BeTrue()) + }) + It("processResourceSummary records the drifted resources of each feature in DriftHistory", func() { cluster := prepareCluster()