Merge branch 'main' into no-emojis

addons/config/expected/cni.tanzu.vmware.com/v1alpha1/antreaconfig/default.yaml

@@ -13,13 +13,20 @@ spec:
       tlsCipherSuites: TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256,TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256,TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384,TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384,TLS_RSA_WITH_AES_256_GCM_SHA384
       disableUdpTunnelOffload: false
       featureGates:
-        AntreaProxy: true
-        EndpointSlice: true
+        AntreaIPAM: false
         AntreaPolicy: true
-        NodePortLocal: false
+        AntreaProxy: true
         AntreaTraceflow: true
-        Egress: false
+        Egress: true
+        EndpointSlice: true
         FlowExporter: false
+        Multicast: false
+        Multicluster: false
+        NetworkPolicyStats: false
+        NodePortLocal: true
+        SecondaryNetwork: false
+        ServiceExternalIP: false
+        TrafficControl: false
 ---
 apiVersion: cni.tanzu.vmware.com/v1alpha1
 kind: AntreaConfig
@@ -36,10 +43,17 @@ spec:
       tlsCipherSuites: TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256,TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256,TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384,TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384,TLS_RSA_WITH_AES_256_GCM_SHA384
       disableUdpTunnelOffload: false
       featureGates:
-        AntreaProxy: true
-        EndpointSlice: true
+        AntreaIPAM: false
         AntreaPolicy: true
-        NodePortLocal: false
+        AntreaProxy: true
         AntreaTraceflow: true
-        Egress: false
+        Egress: true
+        EndpointSlice: true
         FlowExporter: false
+        Multicast: false
+        Multicluster: false
+        NetworkPolicyStats: false
+        NodePortLocal: true
+        SecondaryNetwork: false
+        ServiceExternalIP: false
+        TrafficControl: false

addons/config/templates/cni.tanzu.vmware.com/v1alpha1/antreaconfig.yaml

@@ -15,13 +15,20 @@ spec:
       tlsCipherSuites: TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256,TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256,TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384,TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384,TLS_RSA_WITH_AES_256_GCM_SHA384
       disableUdpTunnelOffload: false
       featureGates:
-        AntreaProxy: true
-        EndpointSlice: true
+        AntreaIPAM: false
         AntreaPolicy: true
-        NodePortLocal: false
+        AntreaProxy: true
         AntreaTraceflow: true
-        Egress: false
+        Egress: true
+        EndpointSlice: true
         FlowExporter: false
+        Multicast: false
+        Multicluster: false
+        NetworkPolicyStats: false
+        NodePortLocal: true
+        SecondaryNetwork: false
+        ServiceExternalIP: false
+        TrafficControl: false
 ---
 apiVersion: cni.tanzu.vmware.com/v1alpha1
 kind: AntreaConfig
@@ -38,10 +45,17 @@ spec:
       tlsCipherSuites: TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256,TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256,TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384,TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384,TLS_RSA_WITH_AES_256_GCM_SHA384
       disableUdpTunnelOffload: false
       featureGates:
-        AntreaProxy: true
-        EndpointSlice: true
+        AntreaIPAM: false
         AntreaPolicy: true
-        NodePortLocal: false
+        AntreaProxy: true
         AntreaTraceflow: true
-        Egress: false
+        Egress: true
+        EndpointSlice: true
         FlowExporter: false
+        Multicast: false
+        Multicluster: false
+        NetworkPolicyStats: false
+        NodePortLocal: true
+        SecondaryNetwork: false
+        ServiceExternalIP: false
+        TrafficControl: false

addons/controllers/addon_controller.go

@@ -234,6 +234,15 @@ func (r *AddonReconciler) reconcileDelete(
 		return ctrl.Result{}, kerrors.NewAggregate(errors)
 	}
 
+	_, isManagmentCluster := cluster.Labels[constants.ManagementClusterRoleLabel]
+	if !isManagmentCluster {
+		err = r.removeFinalizer(ctx, cluster, cluster.DeepCopy())
+		if err != nil {
+			log.Error(err, "failed to remove finalizer from cluster")
+			return ctrl.Result{}, err
+		}
+	}
+
 	return ctrl.Result{}, nil
 }
 
@@ -243,6 +252,15 @@ func (r *AddonReconciler) reconcileNormal(
 	log logr.Logger,
 	cluster *clusterapiv1beta1.Cluster) (ctrl.Result, error) {
 
+	_, isManagmentCluster := cluster.Labels[constants.ManagementClusterRoleLabel]
+	if !isManagmentCluster {
+		err := r.addFinalizer(ctx, cluster, cluster.DeepCopy())
+		if err != nil {
+			log.Error(err, "failed to add finalizer to cluster")
+			return ctrl.Result{}, err
+		}
+	}
+
 	// Get addon secrets for the cluster
 	addonSecrets, err := util.GetAddonSecretsForCluster(ctx, r.Client, cluster)
 	if err != nil {
@@ -314,6 +332,22 @@ func (r *AddonReconciler) reconcileNormal(
 	return result, nil
 }
 
+func (r *AddonReconciler) addFinalizer(ctx context.Context, o, deepCopy client.Object) error {
+	if !controllerutil.ContainsFinalizer(deepCopy, addontypes.AddonFinalizer) {
+		controllerutil.AddFinalizer(deepCopy, addontypes.AddonFinalizer)
+		return r.Client.Patch(ctx, deepCopy, client.MergeFrom(o))
+	}
+	return nil
+}
+
+func (r *AddonReconciler) removeFinalizer(ctx context.Context, o, deepCopy client.Object) error {
+	if controllerutil.ContainsFinalizer(deepCopy, addontypes.AddonFinalizer) {
+		controllerutil.RemoveFinalizer(deepCopy, addontypes.AddonFinalizer)
+		return r.Client.Patch(ctx, deepCopy, client.MergeFrom(o))
+	}
+	return nil
+}
+
 // reconcileNormal reconciles the addons belonging to the cluster
 func (r *AddonReconciler) reconcileAddonSecret(
 	ctx context.Context,

addons/controllers/addon_controller_test.go

@@ -11,9 +11,12 @@ import (
 	. "github.com/onsi/gomega"
 	v1 "k8s.io/api/core/v1"
 	rbacv1 "k8s.io/api/rbac/v1"
+	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	clusterapiv1beta1 "sigs.k8s.io/cluster-api/api/v1beta1"
 	"sigs.k8s.io/cluster-api/util/secret"
 	"sigs.k8s.io/controller-runtime/pkg/client"
+	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
 
 	kappctrl "github.com/vmware-tanzu/carvel-kapp-controller/pkg/apis/kappctrl/v1alpha1"
 	pkgiv1alpha1 "github.com/vmware-tanzu/carvel-kapp-controller/pkg/apis/packaging/v1alpha1"
@@ -392,4 +395,108 @@ var _ = Describe("Addon Reconciler", func() {
 		})
 
 	})
+
+})
+
+var _ = Describe("Addon cluster finalizer", func() {
+	var (
+		clusterName             string
+		clusterNamespace        string
+		clusterResourceFilePath string
+		finalizerPresent        bool
+	)
+
+	JustBeforeEach(func() {
+		// create namespace
+		ns := &v1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: clusterNamespace}}
+		Expect(k8sClient.Create(ctx, ns)).To(Succeed())
+		clusterName = clusterNamespace + "-cluster"
+		clusterResourceFilePath = "testdata/test-" + clusterNamespace + ".yaml"
+		// create cluster resources
+		By("Creating cluster")
+		f, err := os.Open(clusterResourceFilePath)
+		Expect(err).ToNot(HaveOccurred())
+		defer f.Close()
+		Expect(testutil.CreateResources(f, cfg, dynamicClient)).To(Succeed())
+		By("Creating kubeconfig for cluster")
+		Expect(testutil.CreateKubeconfigSecret(cfg, clusterName, clusterNamespace, k8sClient)).To(Succeed())
+
+		By("Check if finalizer is set correctly")
+		key := client.ObjectKey{
+			Namespace: clusterNamespace,
+			Name:      clusterName,
+		}
+		cluster := &clusterapiv1beta1.Cluster{}
+		Eventually(func() bool {
+			if err := k8sClient.Get(ctx, key, cluster); err != nil {
+				return false
+			}
+			return controllerutil.ContainsFinalizer(cluster, addontypes.AddonFinalizer) == finalizerPresent
+		}, 20, pollingInterval).Should(BeTrue())
+
+		By("Check that finalizers is remove if necessary")
+		Expect(k8sClient.Delete(ctx, cluster)).To(Succeed())
+		Eventually(func() bool {
+			if err := k8sClient.Get(ctx, key, cluster); apierrors.IsNotFound(err) {
+				return false // if the cluster was deleted it means finalizer was removed.
+			}
+			Expect(err).To(BeNil()) // If error is not nil at this point lets fail test altogether
+			return controllerutil.ContainsFinalizer(cluster, addontypes.AddonFinalizer)
+
+		}, 20, pollingInterval).Should(BeFalse())
+	})
+
+	AfterEach(func() {
+		// some resources should have already been deleted by test, so  we don't error out if this deletion attempt fails.
+		// we attempt to delete them in case something failed, so other tests don't get blocked by orphaned resources
+		By("Deleting cluster, tkr, BOM config map and addon secret")
+		f, err := os.Open(clusterResourceFilePath)
+		Expect(err).ToNot(HaveOccurred())
+		defer f.Close()
+
+		testutil.DeleteResources(f, cfg, dynamicClient, true) // nolint:errcheck
+
+		By("Deleting Addon data-values secrets")
+		addonSecretKey := client.ObjectKey{
+			Namespace: addonNamespace,
+			Name:      "antrea-data-values",
+		}
+		dataValuesSecret := &v1.Secret{}
+		k8sClient.Get(ctx, addonSecretKey, dataValuesSecret) // nolint:errcheck
+		k8sClient.Delete(ctx, dataValuesSecret)              // nolint:errcheck
+
+		By("Deleting Addon app CR")
+		appKey := client.ObjectKey{
+			Namespace: addonNamespace,
+			Name:      "antrea",
+		}
+		antreaApp := &kappctrl.App{}
+		// some testcases don't create App CR
+		k8sClient.Get(ctx, appKey, antreaApp) // nolint:errcheck
+		k8sClient.Delete(ctx, antreaApp)      // nolint:errcheck
+
+		By("Deleting kubeconfig for cluster")
+		key := client.ObjectKey{
+			Namespace: clusterNamespace,
+			Name:      secret.Name(clusterName, secret.Kubeconfig),
+		}
+		s := &v1.Secret{}
+		k8sClient.Get(ctx, key, s) // nolint:errcheck
+		k8sClient.Delete(ctx, s)   // nolint:errcheck
+	})
+	Context("workload cluster finalizer", func() {
+		BeforeEach(func() {
+			clusterNamespace = "workload-finalizer"
+			finalizerPresent = true
+		})
+		It("Should add and remove  finalizer to/from workload cluster", func() {})
+	})
+
+	Context("management cluster finalizer", func() {
+		BeforeEach(func() {
+			clusterNamespace = "management-finalizer"
+			finalizerPresent = false
+		})
+		It("Should add and remove  finalizer to/from management cluster", func() {})
+	})
 })

addons/controllers/antreaconfig_controller_test.go

@@ -136,6 +136,7 @@ var _ = Describe("AntreaConfig Reconciler and Webhooks", func() {
 				Expect(config.Spec.Antrea.AntreaConfigDataValue.FeatureGates.MultiCluster).Should(Equal(false))
 				Expect(config.Spec.Antrea.AntreaConfigDataValue.FeatureGates.SecondaryNetwork).Should(Equal(false))
 				Expect(config.Spec.Antrea.AntreaConfigDataValue.FeatureGates.TrafficControl).Should(Equal(false))
+				Expect(config.Spec.Antrea.AntreaConfigDataValue.FeatureGates.NodePortLocal).Should(Equal(true))
 
 				return true
 			}, waitTimeout, pollingInterval).Should(BeTrue())

addons/controllers/clusterbootstrap_controller.go

@@ -500,19 +500,29 @@ func (r *ClusterBootstrapReconciler) mergeClusterBootstrapPackagesWithTemplate(
 	//    4. All packages, including additional packages, can't be deleted (meaning the package refName can't be changed, only allow version bump)
 	//    5. We will keep users' customization on valuesFrom of each package, users are responsible for the correctness of the content they put in will work with the next version.
 	packages := make([]*runtanzuv1alpha3.ClusterBootstrapPackage, 0)
-	if updatedClusterBootstrap.Spec.CNI == nil {
-		log.Info("no CNI package specified in ClusterBootstrap, should not happen. Continue with CNI in ClusterBootstrapTemplate of new TKR")
-		updatedClusterBootstrap.Spec.CNI = clusterBootstrapTemplate.Spec.CNI.DeepCopy()
+
+	_, unmanagedCNI := updatedClusterBootstrap.Annotations[constants.UnmanagedCNI]
+	if unmanagedCNI && updatedClusterBootstrap.Spec.CNI != nil {
+		return nil, errors.New("Spec.CNI should be empty if the clusterbootstrap is annotated to use unmanaged CNI.")
+	}
+
+	if unmanagedCNI {
+		log.Info("Unmanaged CNI")
 	} else {
-		// We don't allow change to the CNI selection once it starts running, however we allow version bump
-		// TODO: check correctness of the following statement, as we still allow version bump
-		// ClusterBootstrap webhook will make sure the package RefName always match the original CNI
-		updatedCNI, cniNamePrefix, err := util.GetBootstrapPackageNameFromTKR(r.context, r.Client, updatedClusterBootstrap.Spec.CNI.RefName, cluster)
-		if err != nil {
-			errorMsg := fmt.Sprintf("unable to find any CNI bootstrap package prefixed with '%s' for ClusterBootstrap %s/%s in TKR", cniNamePrefix, cluster.Namespace, cluster.Name)
-			return nil, errors.Wrap(err, errorMsg)
+		if updatedClusterBootstrap.Spec.CNI == nil {
+			log.Info("no CNI package specified in ClusterBootstrap, and it is not annotated to use a unmanaged CNI. Continue with CNI in ClusterBootstrapTemplate of new TKR")
+			updatedClusterBootstrap.Spec.CNI = clusterBootstrapTemplate.Spec.CNI.DeepCopy()
+		} else {
+			// We don't allow change to the CNI selection once it starts running, however we allow version bump
+			// TODO: check correctness of the following statement, as we still allow version bump
+			// ClusterBootstrap webhook will make sure the package RefName always match the original CNI
+			updatedCNI, cniNamePrefix, err := util.GetBootstrapPackageNameFromTKR(r.context, r.Client, updatedClusterBootstrap.Spec.CNI.RefName, cluster)
+			if err != nil {
+				errorMsg := fmt.Sprintf("unable to find any CNI bootstrap package prefixed with '%s' for ClusterBootstrap %s/%s in TKR", cniNamePrefix, cluster.Namespace, cluster.Name)
+				return nil, errors.Wrap(err, errorMsg)
+			}
+			updatedClusterBootstrap.Spec.CNI.RefName = updatedCNI
 		}
-		updatedClusterBootstrap.Spec.CNI.RefName = updatedCNI
 	}
 
 	if updatedClusterBootstrap.Spec.Kapp == nil {

addons/controllers/testdata/test-cluster-metadata.yaml

@@ -83,11 +83,11 @@ data:
             imagePath: azure-cloud-node-manager
             tag: v0.7.4_vmware.1
       cluster-api-provider-azure:
-      - version: v1.5.3+vmware.1
+      - version: v1.6.1+vmware.1
         images:
           capzControllerImage:
             imagePath: cluster-api/cluster-api-azure-controller
-            tag: v1.5.3_vmware.1
+            tag: v1.6.1_vmware.1
       cluster-api-provider-bringyourownhost:
       - version: v0.2.0+vmware.4
         images: