Merge pull request #5009 from jas-nik/vpc-cni-helm

🐛 Remove Helm condition for AWS VPC CNI deletion

Author: k8s-ci-robot

docs/book/src/topics/eks/pod-networking.md

@@ -97,6 +97,26 @@ spec:
   disableVPCCNI: true
 ```
 
+If you are replacing Amazon VPC CNI with your own helm managed instance, you will need to set `AWSManagedControlPlane.spec.disableVPCCNI` to `true` and add `"prevent-deletion": "true"` label on the Daemonset. This label is needed so `aws-node` daemonset is not reaped during CNI reconciliation.
+
+The following example shows how to label your aws-node Daemonset.
+
+```yaml
+apiVersion: apps/v1
+kind: DaemonSet
+metadata:
+  annotations:
+    ...
+  generation: 1
+  labels:
+    app.kubernetes.io/instance: aws-vpc-cni
+    app.kubernetes.io/managed-by: Helm
+    app.kubernetes.io/name: aws-node
+    app.kubernetes.io/version: v1.15.1
+    helm.sh/chart: aws-vpc-cni-1.15.1
+    prevent-deletion: true
+```
+
 > You cannot set **disableVPCCNI** to true if you are using the VPC CNI addon.
 
 Some alternative CNIs provide for the replacement of kube-proxy, such as in [Calico](https://projectcalico.docs.tigera.io/maintenance/ebpf/enabling-ebpf#configure-kube-proxy) and [Cilium](https://docs.cilium.io/en/stable/gettingstarted/kubeproxy-free/). When enabling the kube-proxy alternative, the kube-proxy installed by EKS must be deleted. This can be done via the **disable** property of **kubeProxy** in **AWSManagedControlPlane**:

go.mod

@@ -57,7 +57,6 @@ require (
 	sigs.k8s.io/cluster-api v1.7.1
 	sigs.k8s.io/cluster-api/test v1.7.1
 	sigs.k8s.io/controller-runtime v0.17.3
-	sigs.k8s.io/kustomize/api v0.13.5-0.20230601165947-6ce0bf390ce3
 	sigs.k8s.io/yaml v1.4.0
 )
 
@@ -226,6 +225,7 @@ require (
 	sigs.k8s.io/apiserver-network-proxy/konnectivity-client v0.29.0 // indirect
 	sigs.k8s.io/json v0.0.0-20221116044647-bc3834ca7abd // indirect
 	sigs.k8s.io/kind v0.22.0 // indirect
+	sigs.k8s.io/kustomize/api v0.13.5-0.20230601165947-6ce0bf390ce3 // indirect
 	sigs.k8s.io/kustomize/kustomize/v5 v5.0.4-0.20230601165947-6ce0bf390ce3 // indirect
 	sigs.k8s.io/kustomize/kyaml v0.14.3-0.20230601165947-6ce0bf390ce3 // indirect
 	sigs.k8s.io/structured-merge-diff/v4 v4.4.1 // indirect

pkg/cloud/services/awsnode/cni.go

@@ -31,7 +31,6 @@ import (
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/klog/v2"
 	"sigs.k8s.io/controller-runtime/pkg/client"
-	"sigs.k8s.io/kustomize/api/konfig"
 
 	infrav1 "sigs.k8s.io/cluster-api-provider-aws/v2/api/v1beta2"
 	"sigs.k8s.io/cluster-api-provider-aws/v2/pkg/cloud/awserrors"
@@ -272,22 +271,25 @@ func (s *Service) deleteResource(ctx context.Context, remoteClient client.Client
 			return fmt.Errorf("deleting resource %s: %w", key, err)
 		}
 		s.scope.Debug(fmt.Sprintf("resource %s was not found, no action", key))
-	} else {
-		// resource found, delete if no label or not managed by helm
-		if val, ok := obj.GetLabels()[konfig.ManagedbyLabelKey]; !ok || val != "Helm" {
-			if err := remoteClient.Delete(ctx, obj, &client.DeleteOptions{}); err != nil {
-				if !apierrors.IsNotFound(err) {
-					return fmt.Errorf("deleting %s: %w", key, err)
-				}
-				s.scope.Debug(fmt.Sprintf(
-					"resource %s was not found, not deleted", key))
-			} else {
-				s.scope.Debug(fmt.Sprintf("resource %s was deleted", key))
-			}
-		} else {
-			s.scope.Debug(fmt.Sprintf("resource %s is managed by helm, not deleted", key))
+		return nil
+	}
+	// Don't delete if the `prevent-deletion` label exists. It could be there because CAPA added it (see below),
+	// or because it was added externally, for example if a custom version of AWS CNI was already installed.
+	// Either way, CAPA should not delete such a labelled CNI installation.
+	labels := obj.GetLabels()
+	if _, exists := labels["prevent-deletion"]; exists {
+		s.scope.Debug(fmt.Sprintf("resource %s has 'prevent-deletion' label, skipping deletion", key))
+		return nil
+	}
+	// Delete the resource
+	if err := remoteClient.Delete(ctx, obj, &client.DeleteOptions{}); err != nil {
+		if !apierrors.IsNotFound(err) {
+			return fmt.Errorf("failed to delete %s: %w", key, err)
 		}
+		s.scope.Debug(fmt.Sprintf(
+			"resource %s was not found, not deleted", key))
+	} else {
+		s.scope.Debug(fmt.Sprintf("resource %s was deleted", key))
 	}
-
 	return nil
 }