Skip content resource creation for HelmOps bundles (#3327)

integrationtests/controller/bundle/bundle_helm_test.go

@@ -1,6 +1,7 @@
 package bundle
 
 import (
+	"context"
 	"crypto/rand"
 	"os"
 
@@ -11,7 +12,6 @@ import (
 	"github.com/rancher/fleet/pkg/apis/fleet.cattle.io/v1alpha1"
 
 	corev1 "k8s.io/api/core/v1"
-	v1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/labels"
 	"k8s.io/apimachinery/pkg/types"
@@ -42,7 +42,7 @@ var _ = Describe("Bundle with helm options", Ordered, func() {
 	)
 
 	JustBeforeEach(func() {
-		bundle, err := utils.CreateHelmBundle(ctx, k8sClient, bundleName, namespace, targets, targetRestrictions, helmOptions)
+		bundle, err := createHelmBundle(ctx, k8sClient, bundleName, namespace, targets, targetRestrictions, helmOptions)
 		Expect(err).NotTo(HaveOccurred())
 		Expect(bundle).To(Not(BeNil()))
 
@@ -218,13 +218,13 @@ func createHelmSecret(c client.Client, helmOptions *v1alpha1.BundleHelmOptions,
 	if err != nil {
 		return err
 	}
-	secret := &v1.Secret{
+	secret := &corev1.Secret{
 		ObjectMeta: metav1.ObjectMeta{
 			Name:      helmOptions.SecretName,
 			Namespace: ns,
 		},
-		Data: map[string][]byte{v1.BasicAuthUsernameKey: username, v1.BasicAuthPasswordKey: password, "cacerts": certs},
-		Type: v1.SecretTypeBasicAuth,
+		Data: map[string][]byte{corev1.BasicAuthUsernameKey: username, corev1.BasicAuthPasswordKey: password, "cacerts": certs},
+		Type: corev1.SecretTypeBasicAuth,
 	}
 
 	return c.Create(ctx, secret)
@@ -235,7 +235,7 @@ func deleteHelmSecret(c client.Client, helmOptions *v1alpha1.BundleHelmOptions,
 		return nil
 	}
 	nsName := types.NamespacedName{Namespace: ns, Name: helmOptions.SecretName}
-	secret := &v1.Secret{}
+	secret := &corev1.Secret{}
 	err := c.Get(ctx, nsName, secret)
 	if err != nil {
 		return err
@@ -252,13 +252,13 @@ func checkBundleDeploymentSecret(c client.Client, helmOptions *v1alpha1.BundleHe
 
 	// get the secret for the bundle
 	nsName := types.NamespacedName{Namespace: bNamespace, Name: helmOptions.SecretName}
-	bundleSecret := &v1.Secret{}
+	bundleSecret := &corev1.Secret{}
 	err := c.Get(ctx, nsName, bundleSecret)
 	Expect(err).NotTo(HaveOccurred())
 
 	// get the secret for the bundle deployment
 	bdNsName := types.NamespacedName{Namespace: bdNamespace, Name: helmOptions.SecretName}
-	bdSecret := &v1.Secret{}
+	bdSecret := &corev1.Secret{}
 	err = c.Get(ctx, bdNsName, bdSecret)
 	Expect(err).NotTo(HaveOccurred())
 
@@ -273,3 +273,30 @@ func checkBundleDeploymentSecret(c client.Client, helmOptions *v1alpha1.BundleHe
 	Expect(controller.Kind).To(Equal("BundleDeployment"))
 	Expect(controller.APIVersion).To(Equal("fleet.cattle.io/v1alpha1"))
 }
+
+func createHelmBundle(ctx context.Context, k8sClient client.Client, name, namespace string, targets []v1alpha1.BundleTarget, targetRestrictions []v1alpha1.BundleTarget, helmOptions *v1alpha1.BundleHelmOptions) (*v1alpha1.Bundle, error) {
+	restrictions := []v1alpha1.BundleTargetRestriction{}
+	for _, r := range targetRestrictions {
+		restrictions = append(restrictions, v1alpha1.BundleTargetRestriction{
+			Name:                 r.Name,
+			ClusterName:          r.ClusterName,
+			ClusterSelector:      r.ClusterSelector,
+			ClusterGroup:         r.ClusterGroup,
+			ClusterGroupSelector: r.ClusterGroupSelector,
+		})
+	}
+	bundle := v1alpha1.Bundle{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      name,
+			Namespace: namespace,
+			Labels:    map[string]string{"foo": "bar"},
+		},
+		Spec: v1alpha1.BundleSpec{
+			Targets:            targets,
+			TargetRestrictions: restrictions,
+			HelmAppOptions:     helmOptions,
+		},
+	}
+
+	return &bundle, k8sClient.Create(ctx, &bundle)
+}

integrationtests/controller/bundle/suite_test.go

@@ -3,6 +3,7 @@ package bundle
 import (
 	"context"
 	"testing"
+	"time"
 
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
@@ -38,6 +39,9 @@ func TestFleet(t *testing.T) {
 }
 
 var _ = BeforeSuite(func() {
+	SetDefaultEventuallyTimeout(60 * time.Second)
+	SetDefaultEventuallyPollingInterval(1 * time.Second)
+
 	ctx, cancel = context.WithCancel(context.TODO())
 	testenv = utils.NewEnvTest("../../..")
 

integrationtests/utils/helpers.go

@@ -4,6 +4,7 @@ import (
 	"context"
 
 	"github.com/rancher/fleet/pkg/apis/fleet.cattle.io/v1alpha1"
+
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 )
@@ -34,33 +35,6 @@ func CreateBundle(ctx context.Context, k8sClient client.Client, name, namespace
 	return &bundle, k8sClient.Create(ctx, &bundle)
 }
 
-func CreateHelmBundle(ctx context.Context, k8sClient client.Client, name, namespace string, targets []v1alpha1.BundleTarget, targetRestrictions []v1alpha1.BundleTarget, helmOptions *v1alpha1.BundleHelmOptions) (*v1alpha1.Bundle, error) {
-	restrictions := []v1alpha1.BundleTargetRestriction{}
-	for _, r := range targetRestrictions {
-		restrictions = append(restrictions, v1alpha1.BundleTargetRestriction{
-			Name:                 r.Name,
-			ClusterName:          r.ClusterName,
-			ClusterSelector:      r.ClusterSelector,
-			ClusterGroup:         r.ClusterGroup,
-			ClusterGroupSelector: r.ClusterGroupSelector,
-		})
-	}
-	bundle := v1alpha1.Bundle{
-		ObjectMeta: metav1.ObjectMeta{
-			Name:      name,
-			Namespace: namespace,
-			Labels:    map[string]string{"foo": "bar"},
-		},
-		Spec: v1alpha1.BundleSpec{
-			Targets:            targets,
-			TargetRestrictions: restrictions,
-			HelmAppOptions:     helmOptions,
-		},
-	}
-
-	return &bundle, k8sClient.Create(ctx, &bundle)
-}
-
 func CreateCluster(ctx context.Context, k8sClient client.Client, name, controllerNs string, labels map[string]string, clusterNs string) (*v1alpha1.Cluster, error) {
 	cluster := &v1alpha1.Cluster{
 		ObjectMeta: metav1.ObjectMeta{

internal/cmd/controller/reconciler/bundle_controller.go

@@ -350,9 +350,11 @@ func (r *BundleReconciler) createBundleDeployment(
 
 	bd.Spec.OCIContents = contentsInOCI
 	bd.Spec.HelmChartOptions = helmAppOptions
+	logger = logger.WithValues("bundledeployment", bd, "deploymentID", bd.Spec.DeploymentID)
+	contentsInHelmChart := helmAppOptions != nil
 
-	// contents resources stored in etcd, finalizers to add here.
-	if !contentsInOCI {
+	// When content resources are stored in etcd, we need to add finalizers.
+	if !contentsInOCI && !contentsInHelmChart {
 		content := &fleet.Content{}
 		err := r.Get(ctx, types.NamespacedName{Name: manifestID}, content)
 		if client.IgnoreNotFound(err) != nil {
@@ -367,7 +369,6 @@ func (r *BundleReconciler) createBundleDeployment(
 		}
 	}
 
-	contentsInHelmChart := helmAppOptions != nil
 	updated := bd.DeepCopy()
 	op, err := controllerutil.CreateOrUpdate(ctx, r.Client, bd, func() error {
 		// When this mutation function is called by CreateOrUpdate, bd contains the
@@ -380,14 +381,7 @@ func (r *BundleReconciler) createBundleDeployment(
 			bd.Spec.DeploymentID != "" &&
 			bd.Spec.DeploymentID != updated.Spec.DeploymentID {
 			if err := finalize.PurgeContent(ctx, r.Client, bd.Name, bd.Spec.DeploymentID); err != nil {
-				logger.Error(
-					err,
-					"Reconcile failed to purge old content resource",
-					"bundledeployment",
-					bd,
-					"deploymentID",
-					bd.Spec.DeploymentID,
-				)
+				logger.Error(err, "Reconcile failed to purge old content resource")
 			}
 		}
 
@@ -397,17 +391,10 @@ func (r *BundleReconciler) createBundleDeployment(
 		return nil
 	})
 	if err != nil && !apierrors.IsAlreadyExists(err) {
-		logger.Error(
-			err,
-			"Reconcile failed to create or update bundledeployment",
-			"bundledeployment",
-			bd,
-			"operation",
-			op,
-		)
+		logger.Error(err, "Reconcile failed to create or update bundledeployment", "operation", op)
 		return nil, err
 	}
-	logger.V(1).Info(upper(op)+" bundledeployment", "bundledeployment", bd, "operation", op)
+	logger.Info(upper(op)+" bundledeployment", "operation", op)
 
 	return bd, nil
 }
@@ -444,7 +431,7 @@ func (r *BundleReconciler) createDeploymentSecret(ctx context.Context, secretNam
 
 func (r *BundleReconciler) getOCIReference(ctx context.Context, bundle *fleet.Bundle) (string, error) {
 	if bundle.Spec.ContentsID == "" {
-		return "", fmt.Errorf("cannot get OCI reference. Bundles's ContentsID is not set")
+		return "", fmt.Errorf("cannot get OCI reference. Bundle's ContentsID is not set")
 	}
 	namespacedName := types.NamespacedName{
 		Namespace: bundle.Namespace,