Repository: rancher/fleet
Pull request #2239 (2024-03-19 12:53:26)

Only create Content resource if Bundle matches any target
Refers to https://github.com/rancher/fleet/issues/2214


Patch:
diff --git a/internal/cmd/controller/reconciler/bundle_controller.go b/internal/cmd/controller/reconciler/bundle_controller.go
index 072a6f48..e57a172e 100644
--- a/internal/cmd/controller/reconciler/bundle_controller.go
+++ b/internal/cmd/controller/reconciler/bundle_controller.go
@@ -31,7 +31,7 @@ type BundleQuery interface {
 }
 
 type Store interface {
-	Store(context.Context, *manifest.Manifest) (string, error)
+	Store(context.Context, *manifest.Manifest) error
 }
 
 type TargetBuilder interface {
@@ -81,11 +81,9 @@ func (r *BundleReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr
 	}
 	bundle.Status.ResourcesSHA256Sum = manifestDigest
 
-	// this does not need to happen after merging the
-	// BundleDeploymentOptions, since 'fleet apply' already put the right
-	// resources into bundle.Spec.Resources
-	manifestID, err := r.Store.Store(ctx, manifest)
+	manifestID, err := manifest.ID()
 	if err != nil {
+		// this should never happen, since manifest.SHASum() cached the result and worked above.
 		return ctrl.Result{}, err
 	}
 
@@ -94,6 +92,23 @@ func (r *BundleReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr
 		return ctrl.Result{}, err
 	}
 
+	// do not create a content resource if there are no targets, it will
+	// only create work for `PurgeOrphanedInBackground`.
+	if len(matchedTargets) > 0 {
+		// `fleet apply` puts all resources into `bundle.Spec.Resources`.
+		// `Store` copies all the resources into the content resource.
+		// There is no pruning of unused resources. Therefore we write
+		// the content resource immediately, even though
+		// `BundleDeploymentOptions`, e.g. `targetCustomizations` on
+		// the `helm.Chart` field, change which resources are used. The
+		// agents have access to all resources and use their specific
+		// set of `BundleDeploymentOptions`.
+		err := r.Store.Store(ctx, manifest)
+		if err != nil {
+			return ctrl.Result{}, err
+		}
+	}
+
 	if err := resetStatus(&bundle.Status, matchedTargets); err != nil {
 		updateDisplay(&bundle.Status)
 		return ctrl.Result{}, err
diff --git a/internal/manifest/manifest.go b/internal/manifest/manifest.go
index 41626a11..f7ee1c32 100644
--- a/internal/manifest/manifest.go
+++ b/internal/manifest/manifest.go
@@ -90,7 +90,8 @@ func (m *Manifest) ResetSHASum() {
 	m.shasum = ""
 }
 
-// SHASum returns the SHA256 sum of the JSON serialization
+// SHASum returns the SHA256 sum of the JSON serialization. If necessary it
+// loads the content resource and caches its shasum and raw data.
 func (m *Manifest) SHASum() (string, error) {
 	if m.shasum == "" {
 		if err := m.load(); err != nil {
@@ -100,7 +101,9 @@ func (m *Manifest) SHASum() (string, error) {
 	return m.shasum, nil
 }
 
-// ID returns the name of the Content resource produced from this Manifest
+// ID returns the name of the Content resource produced from this Manifest.
+// If necessary it loads the content resource and caches its shasum and
+// raw data.
 func (m *Manifest) ID() (string, error) {
 	shasum, err := m.SHASum()
 	if err != nil {
diff --git a/internal/manifest/store.go b/internal/manifest/store.go
index 7ed163d9..f5a29996 100644
--- a/internal/manifest/store.go
+++ b/internal/manifest/store.go
@@ -22,21 +22,21 @@ type ContentStore struct {
 	client client.Client
 }
 
-// Store stores the manifest as a content resource and returns the name.
+// Store stores the manifest as a content resource.
 // It copies the resources from the bundle to the content resource.
-func (c *ContentStore) Store(ctx context.Context, manifest *Manifest) (string, error) {
+func (c *ContentStore) Store(ctx context.Context, manifest *Manifest) error {
 	id, err := manifest.ID()
 	if err != nil {
-		return "", err
+		return err
 	}
 
 	if err := c.client.Get(ctx, types.NamespacedName{Name: id}, &fleet.Content{}); err != nil && !apierrors.IsNotFound(err) {
-		return "", err
+		return err
 	} else if err == nil {
-		return id, nil
+		return nil
 	}
 
-	return id, c.createContents(ctx, id, manifest)
+	return c.createContents(ctx, id, manifest)
 }
 
 func (c *ContentStore) createContents(ctx context.Context, id string, manifest *Manifest) error {


Test patch:
diff --git a/internal/manifest/store_test.go b/internal/manifest/store_test.go
index 69aa67b6..d4024b68 100644
--- a/internal/manifest/store_test.go
+++ b/internal/manifest/store_test.go
@@ -81,14 +81,11 @@ func Test_contentStore_Store(t *testing.T) {
 				}).Times(1)
 			}
 
-			got, err := store.Store(ctx, tt.args.manifest)
+			err := store.Store(ctx, tt.args.manifest)
 			if err != nil {
 				t.Errorf("Store() error = %v", err)
 				return
 			}
-			if got != tt.want {
-				t.Errorf("Store() got = %v, want %v", got, tt.want)
-			}
 		})
 	}
 }