Repository: rancher/fleet
Pull request #2535 (2024-06-18 12:22:18)

Populate bundle status from bundleDeployment status resources
Refers to #2115
- [x] status derived from new `bundledeployment.status.resources` in bundle reconciler



Patch:
diff --git a/internal/cmd/agent/deployer/normalizers/norm.go b/internal/cmd/agent/deployer/normalizers/norm.go
index a47164c8..64f826ac 100644
--- a/internal/cmd/agent/deployer/normalizers/norm.go
+++ b/internal/cmd/agent/deployer/normalizers/norm.go
@@ -22,6 +22,8 @@ func (n Norm) Normalize(un *unstructured.Unstructured) error {
 func New(lives objectset.ObjectByGVK, additions ...diff.Normalizer) Norm {
 	n := Norm{
 		normalizers: []diff.Normalizer{
+			// Status fields are normally subresources which can't be influenced by resource updates
+			&StatusNormalizer{},
 			&MutatingWebhookNormalizer{
 				Live: lives,
 			},
diff --git a/internal/cmd/agent/deployer/normalizers/status.go b/internal/cmd/agent/deployer/normalizers/status.go
new file mode 100644
index 00000000..26f4605b
--- /dev/null
+++ b/internal/cmd/agent/deployer/normalizers/status.go
@@ -0,0 +1,13 @@
+package normalizers
+
+import (
+	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
+)
+
+// StatusNormalizer removes a top-level "status" fields from the object, if present
+type StatusNormalizer struct{}
+
+func (_ StatusNormalizer) Normalize(un *unstructured.Unstructured) error {
+	unstructured.RemoveNestedField(un.Object, "status")
+	return nil
+}
diff --git a/internal/cmd/controller/reconciler/bundle_controller.go b/internal/cmd/controller/reconciler/bundle_controller.go
index 0633df1d..2305626b 100644
--- a/internal/cmd/controller/reconciler/bundle_controller.go
+++ b/internal/cmd/controller/reconciler/bundle_controller.go
@@ -13,9 +13,7 @@ import (
 	"github.com/rancher/fleet/pkg/sharding"
 
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
-	"k8s.io/apimachinery/pkg/api/meta"
 	"k8s.io/apimachinery/pkg/runtime"
-	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/client-go/util/retry"
 	ctrl "sigs.k8s.io/controller-runtime"
@@ -165,11 +163,7 @@ func (r *BundleReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr
 		return ctrl.Result{}, err
 	}
 
-	if bundle.Status.ObservedGeneration != bundle.Generation {
-		if err := setResourceKey(context.Background(), &bundle.Status, bundle, manifest, r.isNamespaced); err != nil {
-			return ctrl.Result{}, err
-		}
-	}
+	setResourceKey(&bundle.Status, matchedTargets)
 
 	summary.SetReadyConditions(&bundle.Status, "Cluster", bundle.Status.Summary)
 	bundle.Status.ObservedGeneration = bundle.Generation
@@ -227,14 +221,6 @@ func (r *BundleReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr
 	return ctrl.Result{}, err
 }
 
-func (r *BundleReconciler) isNamespaced(gvk schema.GroupVersionKind) bool {
-	mapping, err := r.RESTMapper().RESTMapping(gvk.GroupKind(), gvk.Version)
-	if err != nil {
-		return true
-	}
-	return mapping.Scope.Name() == meta.RESTScopeNameNamespace
-}
-
 func upper(op controllerutil.OperationResult) string {
 	switch op {
 	case controllerutil.OperationResultNone:
diff --git a/internal/cmd/controller/reconciler/bundle_status.go b/internal/cmd/controller/reconciler/bundle_status.go
index 55f43a83..80f90007 100644
--- a/internal/cmd/controller/reconciler/bundle_status.go
+++ b/internal/cmd/controller/reconciler/bundle_status.go
@@ -1,21 +1,12 @@
 package reconciler
 
 import (
-	"context"
 	"fmt"
-	"sort"
 
-	"github.com/sirupsen/logrus"
-
-	"github.com/rancher/fleet/internal/cmd/controller/options"
 	"github.com/rancher/fleet/internal/cmd/controller/summary"
 	"github.com/rancher/fleet/internal/cmd/controller/target"
-	"github.com/rancher/fleet/internal/helmdeployer"
-	"github.com/rancher/fleet/internal/manifest"
 	fleet "github.com/rancher/fleet/pkg/apis/fleet.cattle.io/v1alpha1"
-
-	"k8s.io/apimachinery/pkg/api/meta"
-	"k8s.io/apimachinery/pkg/runtime/schema"
+	"github.com/sirupsen/logrus"
 )
 
 const (
@@ -41,64 +32,26 @@ func updateDisplay(status *fleet.BundleStatus) {
 	status.Display.State = string(summary.GetSummaryState(status.Summary))
 }
 
-// setResourceKey updates status.ResourceKey from the bundle, by running helm template (does not mutate bundle)
-func setResourceKey(ctx context.Context, status *fleet.BundleStatus, bundle *fleet.Bundle, manifest *manifest.Manifest, isNSed func(schema.GroupVersionKind) bool) error {
-	seen := map[fleet.ResourceKey]struct{}{}
-
-	// iterate over the defined targets, from "targets.yaml", not the
-	// actually matched targets to avoid duplicates
-	for i := range bundle.Spec.Targets {
-		opts := options.Merge(bundle.Spec.BundleDeploymentOptions, bundle.Spec.Targets[i].BundleDeploymentOptions)
-		objs, err := helmdeployer.Template(ctx, bundle.Name, manifest, opts)
-		if err != nil {
-			logrus.Infof("While calculating status.ResourceKey, error running helm template for bundle %s with target options from %s: %v", bundle.Name, bundle.Spec.Targets[i].Name, err)
+// setResourceKey updates status.ResourceKey from the bundleDeployments
+func setResourceKey(status *fleet.BundleStatus, allTargets []*target.Target) {
+	keys := []fleet.ResourceKey{}
+	for _, target := range allTargets {
+		if target.Deployment == nil {
 			continue
 		}
-
-		for _, obj := range objs {
-			m, err := meta.Accessor(obj)
-			if err != nil {
-				return err
-			}
+		if target.Deployment.Namespace == "" {
+			logrus.Infof("Skipping bundledeployment with empty namespace %q", target.Deployment.Name)
+			continue
+		}
+		for _, resource := range target.Deployment.Status.Resources {
 			key := fleet.ResourceKey{
-				Namespace: m.GetNamespace(),
-				Name:      m.GetName(),
+				Name:       resource.Name,
+				Namespace:  resource.Namespace,
+				APIVersion: resource.APIVersion,
+				Kind:       resource.Kind,
 			}
-			gvk := obj.GetObjectKind().GroupVersionKind()
-			if key.Namespace == "" && isNSed(gvk) {
-				if opts.DefaultNamespace == "" {
-					key.Namespace = "default"
-				} else {
-					key.Namespace = opts.DefaultNamespace
-				}
-			}
-			key.APIVersion, key.Kind = gvk.ToAPIVersionAndKind()
-			seen[key] = struct{}{}
+			keys = append(keys, key)
 		}
 	}
-
-	keys := []fleet.ResourceKey{}
-	for k := range seen {
-		keys = append(keys, k)
-	}
-	sort.Slice(keys, func(i, j int) bool {
-		keyi := keys[i]
-		keyj := keys[j]
-		if keyi.APIVersion != keyj.APIVersion {
-			return keyi.APIVersion < keyj.APIVersion
-		}
-		if keyi.Kind != keyj.Kind {
-			return keyi.Kind < keyj.Kind
-		}
-		if keyi.Namespace != keyj.Namespace {
-			return keyi.Namespace < keyj.Namespace
-		}
-		if keyi.Name != keyj.Name {
-			return keyi.Name < keyj.Name
-		}
-		return false
-	})
 	status.ResourceKey = keys
-
-	return nil
 }


Test patch:
diff --git a/e2e/assets/drift/correction-enabled/gitrepo.yaml b/e2e/assets/drift/correction-enabled/gitrepo.yaml
index 10b527a5..f7cb2a42 100644
--- a/e2e/assets/drift/correction-enabled/gitrepo.yaml
+++ b/e2e/assets/drift/correction-enabled/gitrepo.yaml
@@ -9,3 +9,4 @@ spec:
     enabled: true
   paths:
     - drift
+    - drift-ignore-status
diff --git a/e2e/drift/drift_test.go b/e2e/drift/drift_test.go
index d42b6fd1..0de9030e 100644
--- a/e2e/drift/drift_test.go
+++ b/e2e/drift/drift_test.go
@@ -184,6 +184,17 @@ var _ = Describe("Drift", Ordered, func() {
 					`{"name":"http","port":1234,"protocol":"TCP","targetPort":"http-web-svc"}]}}`))
 			})
 		})
+
+		Context("Resource manifests containing status fields", func() {
+			// Status must be ignored for drift correction, despite being part of the manifests
+			It("Is marked as ready", func() {
+				bundleName := "drift-correction-test-drift-ignore-status"
+				Eventually(func() bool {
+					b := getBundle(bundleName, k)
+					return b.Status.Summary.Ready == 1
+				}).Should(BeTrue())
+			})
+		})
 	})
 
 	When("Drift correction is enabled with force", func() {
diff --git a/integrationtests/controller/bundle/status_test.go b/integrationtests/controller/bundle/status_test.go
index b67ab1cc..480580e0 100644
--- a/integrationtests/controller/bundle/status_test.go
+++ b/integrationtests/controller/bundle/status_test.go
@@ -70,10 +70,19 @@ var _ = Describe("Bundle Status Fields", func() {
 				if err != nil {
 					return err
 				}
+				resources := []v1alpha1.BundleDeploymentResource{}
+				resource := v1alpha1.BundleDeploymentResource{
+					Kind:       "ConfigMap",
+					APIVersion: "v1",
+					Namespace:  namespace,
+					Name:       "app-config",
+				}
+				resources = append(resources, resource)
 				bd.Status.Display.State = "Ready"
 				bd.Status.AppliedDeploymentID = bd.Spec.DeploymentID
 				bd.Status.Ready = true
 				bd.Status.NonModified = true
+				bd.Status.Resources = resources
 				return k8sClient.Status().Update(ctx, bd)
 			}).ShouldNot(HaveOccurred())
 
@@ -81,6 +90,7 @@ var _ = Describe("Bundle Status Fields", func() {
 			Expect(err).NotTo(HaveOccurred())
 			Expect(bd.Status.Display.State).To(Equal("Ready"))
 			Expect(bd.Status.AppliedDeploymentID).To(Equal(bd.Spec.DeploymentID))
+			Expect(len(bd.Status.Resources)).To(Equal(1))
 
 			Eventually(func() bool {
 				err = k8sClient.Get(ctx, types.NamespacedName{Namespace: namespace, Name: "name"}, bundle)
@@ -89,6 +99,13 @@ var _ = Describe("Bundle Status Fields", func() {
 			}).Should(BeTrue())
 			Expect(bundle.Status.Summary.DesiredReady).To(Equal(1))
 			Expect(bundle.Status.Display.ReadyClusters).To(Equal("1/1"))
+			Expect(len(bundle.Status.ResourceKey)).To(Equal(1))
+			bdResource := bd.Status.Resources[0]
+			bundleResourceKey := bundle.Status.ResourceKey[0]
+			Expect(bundleResourceKey.APIVersion).To(Equal(bdResource.APIVersion))
+			Expect(bundleResourceKey.Name).To(Equal(bdResource.Name))
+			Expect(bundleResourceKey.Namespace).To(Equal(bdResource.Namespace))
+			Expect(bundleResourceKey.Kind).To(Equal(bdResource.Kind))
 		})
 	})
 
diff --git a/internal/cmd/agent/deployer/normalizers/status_test.go b/internal/cmd/agent/deployer/normalizers/status_test.go
new file mode 100644
index 00000000..69fedebc
--- /dev/null
+++ b/internal/cmd/agent/deployer/normalizers/status_test.go
@@ -0,0 +1,55 @@
+package normalizers
+
+import (
+	"errors"
+	"testing"
+
+	corev1 "k8s.io/api/core/v1"
+	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
+	"k8s.io/apimachinery/pkg/runtime"
+)
+
+func TestStatusNormalizer_Normalize(t *testing.T) {
+	tests := []struct {
+		name  string
+		obj   runtime.Object
+		check func(object runtime.Object) error
+	}{
+		{
+			name: "object with status",
+			obj: &corev1.Pod{
+				Status: corev1.PodStatus{
+					PodIP: "1.2.3.4",
+				},
+			},
+			check: func(obj runtime.Object) error {
+				if obj.(*corev1.Pod).Status.PodIP != "" {
+					return errors.New("status was not removed")
+				}
+				return nil
+			},
+		},
+		{
+			name:  "object without status",
+			obj:   &corev1.ConfigMap{},
+			check: func(_ runtime.Object) error { return nil },
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			un, err := runtime.DefaultUnstructuredConverter.ToUnstructured(tt.obj)
+			if err != nil {
+				t.Fatal(err)
+			}
+			if err := (StatusNormalizer{}).Normalize(&unstructured.Unstructured{Object: un}); err != nil {
+				t.Fatal(err)
+			}
+			if err := runtime.DefaultUnstructuredConverter.FromUnstructured(un, tt.obj); err != nil {
+				t.Fatal(err)
+			}
+			if err := tt.check(tt.obj); err != nil {
+				t.Error(err)
+			}
+		})
+	}
+}