Use a predelete hook to cleanly uninstall the config-policy-controller

This change makes it so that a finalizer is added to the
config-policy-controller ManagedClusterAddOn object. Now when it is deleted,
it will trigger an additional ManifestWork to create a Pod which
signals to the config-policy-controller that it's being uninstalled and
it waits for all ConfigurationPolicy objects to have their finalizers
removed before exiting. Once the Pod exits successfully, the finalizer
on the ManagedClusterAddOn object is removed and the
config-policy-controller uninstall proceeds.

If there are a lot of policies, it may take longer than 30 seconds, so
the grace period was bumped up to 120 seconds to exit cleanly.

Note that now with KIND_VERSION set to minimum, it uses a newer version
for the Hub to match the lowest supported version for the Hub. This is
to work around this:
open-cluster-management-io/api#206

Signed-off-by: mprahl <[email protected]>

Author: mprahl

build/manage-clusters.sh

@@ -13,6 +13,12 @@ if [[ -n "${MANAGED_CLUSTER_COUNT//[0-9]}" ]] || [[ "${MANAGED_CLUSTER_COUNT}" =
   exit 1
 fi
 
+HUB_KIND_VERSION=$KIND_VERSION
+if [[ "${KIND_VERSION}" == "minimum" ]]; then
+  # The hub supports less Kubernetes versions than the managed cluster.
+  HUB_KIND_VERSION=v1.23.13
+fi
+
 KIND_PREFIX=${KIND_PREFIX:-"policy-addon-ctrl"}
 CLUSTER_PREFIX=${CLUSTER_PREFIX:-"cluster"}
 
@@ -27,10 +33,10 @@ case ${RUN_MODE} in
     make e2e-debug
     ;;
   create)
-    make kind-deploy-controller
+    KIND_VERSION=$HUB_KIND_VERSION make kind-deploy-controller
     ;;
   create-dev)
-    make kind-prep-ocm
+    KIND_VERSION=$HUB_KIND_VERSION make kind-prep-ocm
     ;;
   deploy-addons)
     make kind-deploy-addons-hub

go.mod

@@ -12,8 +12,8 @@ require (
 	k8s.io/apimachinery v0.25.0
 	k8s.io/client-go v0.25.0
 	k8s.io/component-base v0.25.0
-	open-cluster-management.io/addon-framework v0.5.1-0.20221206033210-55b40b1bc8c6
-	open-cluster-management.io/api v0.8.1-0.20220919023232-a2688935edf3
+	open-cluster-management.io/addon-framework v0.6.0
+	open-cluster-management.io/api v0.9.1-0.20221222015712-61cf30907d02
 	sigs.k8s.io/controller-runtime v0.11.2
 )
 
@@ -130,4 +130,6 @@ require (
 replace (
 	golang.org/x/text => golang.org/x/text v0.3.8 // CVE-2022-32149
 	helm.sh/helm/v3 => helm.sh/helm/v3 v3.9.4 // sonatype-2022-5277; see helm v3.9.4 release notes
+	// Pending https://github.com/open-cluster-management-io/addon-framework/pull/149
+	open-cluster-management.io/addon-framework => github.com/mprahl/addon-framework v0.0.0-20230213164346-e05945686992
 )

go.sum

@@ -496,6 +496,8 @@ github.com/modern-go/reflect2 v0.0.0-20180701023420-4b7aa43c6742/go.mod h1:bx2lN
 github.com/modern-go/reflect2 v1.0.1/go.mod h1:bx2lNnkwVCuqBIxFjflWJWanXIb3RllmbCylyMrvgv0=
 github.com/modern-go/reflect2 v1.0.2 h1:xBagoLtFs94CBntxluKeaWgTMpvLxC4ur3nMaC9Gz0M=
 github.com/modern-go/reflect2 v1.0.2/go.mod h1:yWuevngMOJpCy52FWWMvUC8ws7m/LJsjYzDa0/r8luk=
+github.com/mprahl/addon-framework v0.0.0-20230213164346-e05945686992 h1:40OmIBncAOCO/IDAlRdI6zXmx/Yh3+2nMovd1kyhSvE=
+github.com/mprahl/addon-framework v0.0.0-20230213164346-e05945686992/go.mod h1:efcOXMAzJQR8jNtz9bS8YCbZ/Jr2fmgKJFOXeiAc+gk=
 github.com/munnerz/goautoneg v0.0.0-20120707110453-a547fc61f48d/go.mod h1:+n7T8mK8HuQTcFwEeznm/DIxMOiR9yIdICNftLE1DvQ=
 github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822 h1:C3w9PqII01/Oq1c1nUAm88MOHcQC9l5mIlSMApZMrHA=
 github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822/go.mod h1:+n7T8mK8HuQTcFwEeznm/DIxMOiR9yIdICNftLE1DvQ=
@@ -1267,10 +1269,8 @@ modernc.org/golex v1.0.0/go.mod h1:b/QX9oBD/LhixY6NDh+IdGv17hgB+51fET1i2kPSmvk=
 modernc.org/mathutil v1.0.0/go.mod h1:wU0vUrJsVWBZ4P6e7xtFJEhFSNsfRLJ8H458uRjg03k=
 modernc.org/strutil v1.0.0/go.mod h1:lstksw84oURvj9y3tn8lGvRxyRC1S2+g5uuIzNfIOBs=
 modernc.org/xc v1.0.0/go.mod h1:mRNCo0bvLjGhHO9WsyuKVU4q0ceiDDDoEeWDJHrNx8I=
-open-cluster-management.io/addon-framework v0.5.1-0.20221206033210-55b40b1bc8c6 h1:TnQDy3V7wkiblgR5tz1J+ASswcfnRB9gpVbmwCCecVo=
-open-cluster-management.io/addon-framework v0.5.1-0.20221206033210-55b40b1bc8c6/go.mod h1:Fymctw1tnmCTXnmAMgc0zdHetAw6UaiAsj1S6w5VW6s=
-open-cluster-management.io/api v0.8.1-0.20220919023232-a2688935edf3 h1:x1KFpqwnVbt4YWnpZzZeip+zWRUIMnsj0Bx7Y2Lrtk8=
-open-cluster-management.io/api v0.8.1-0.20220919023232-a2688935edf3/go.mod h1:+OEARSAl2jIhuLItUcS30UgLA3khmA9ihygLVxzEn+U=
+open-cluster-management.io/api v0.9.1-0.20221222015712-61cf30907d02 h1:QUO3HHm38/99Zr7+ZB3j6PoDpzRzFKOmBBXNMAgaKxs=
+open-cluster-management.io/api v0.9.1-0.20221222015712-61cf30907d02/go.mod h1:6BB/Y6r3hXlPjpJgDwIs6Ubxyx/kXXOg6D9Cntg1I9E=
 rsc.io/binaryregexp v0.2.0/go.mod h1:qTv7/COck+e2FymRvadv62gMdZztPaShugOCi3I+8D8=
 rsc.io/quote/v3 v3.1.0/go.mod h1:yEA65RcK8LyAZtP9Kv3t0HmxON59tX3rD+tICJqUlj0=
 rsc.io/sampler v1.3.0/go.mod h1:T1hPZKmBbMNahiBKFy5HrXp6adAjACjK9JXDnKaTXpA=

pkg/addon/common.go

@@ -156,15 +156,17 @@ type PolicyAgentAddon struct {
 	agent.AgentAddon
 }
 
-func (pa *PolicyAgentAddon) Manifests(cluster *clusterv1.ManagedCluster,
+func (pa *PolicyAgentAddon) Manifests(
+	cluster *clusterv1.ManagedCluster,
 	addon *addonapiv1alpha1.ManagedClusterAddOn,
+	hostingCluster *clusterv1.ManagedCluster,
 ) ([]runtime.Object, error) {
 	pauseAnnotation := addon.GetAnnotations()[PolicyAddonPauseAnnotation]
 	if pauseAnnotation == "true" {
 		return nil, errors.New("the Policy Addon controller is paused due to the policy-addon-pause annotation")
 	}
 
-	return pa.AgentAddon.Manifests(cluster, addon)
+	return pa.AgentAddon.Manifests(cluster, addon, hostingCluster)
 }
 
 // getLogLevel verifies the user-provided log level against Zap, returning 0 if the check fails.

pkg/addon/configpolicy/manifests/managedclusterchart/templates/cleanup_pod.yaml

@@ -0,0 +1,68 @@
+# Copyright Contributors to the Open Cluster Management project
+
+apiVersion: v1
+kind: Pod
+metadata:
+  name: {{ include "controller.fullname" . }}-uninstall
+  namespace: {{ .Release.Namespace }}
+  labels:
+    app: {{ include "controller.fullname" . }}
+    chart: {{ include "controller.chart" . }}
+    release: {{ .Release.Name }}
+    heritage: {{ .Release.Service }}
+    addon.open-cluster-management.io/hosted-manifest-location: hosting
+  annotations:
+    addon.open-cluster-management.io/addon-pre-delete: ""
+spec:
+  restartPolicy: OnFailure
+  containers:
+    - name: {{ .Chart.Name }}-uninstall
+      image: "{{ .Values.global.imageOverrides.config_policy_controller }}"
+      imagePullPolicy: "{{ .Values.global.imagePullPolicy }}"
+      command: ["config-policy-controller"]
+      args:
+        - trigger-uninstall
+        - --deployment-name={{ include "controller.fullname" . }}
+        - --deployment-namespace={{ .Release.Namespace }}
+        {{- if eq .Values.installMode "Hosted" }}
+        - --policy-namespace={{ .Release.Namespace }}
+        {{- else }}
+        - --policy-namespace={{ .Values.clusterName }}
+        {{- end }}
+        - --log-encoder={{ .Values.args.logEncoder }}
+        - --log-level={{ .Values.args.logLevel }}
+        - --v={{ .Values.args.pkgLogLevel }}
+      env:
+        {{- if .Values.global.proxyConfig }}
+        - name: HTTP_PROXY
+          value: {{ .Values.global.proxyConfig.HTTP_PROXY }}
+        - name: HTTPS_PROXY
+          value: {{ .Values.global.proxyConfig.HTTPS_PROXY }}
+        - name: NO_PROXY
+          value: {{ .Values.global.proxyConfig.NO_PROXY }}
+        {{- end }}
+      resources: {{- toYaml .Values.resources | nindent 10 }}
+      securityContext:
+      allowPrivilegeEscalation: false
+      capabilities:
+        drop:
+        - ALL
+      privileged: false
+      readOnlyRootFilesystem: true
+  {{- if .Values.global.imagePullSecret }}
+  imagePullSecrets:
+  - name: "{{ .Values.global.imagePullSecret }}"
+  {{- end }}
+  affinity: {{ toYaml .Values.affinity | nindent 8 }}
+  {{- if hasKey .Values "tolerations" }}
+  tolerations: {{ toYaml .Values.tolerations | nindent 8 }}
+  {{- end }}
+  {{- if hasKey .Values.global "nodeSelector" }}
+  nodeSelector: {{ toYaml .Values.global.nodeSelector | nindent 8 }}
+  {{- end }}
+  hostNetwork: false
+  hostPID: false
+  hostIPC: false
+  serviceAccount: {{ include "controller.serviceAccountName" . }}
+  securityContext:
+    runAsNonRoot: true

pkg/addon/configpolicy/manifests/managedclusterchart/templates/deployment.yaml

@@ -57,6 +57,7 @@ spec:
         imagePullPolicy: "{{ .Values.global.imagePullPolicy }}"
         command: ["config-policy-controller"]
         args:
+          - controller
           - "--enable-lease=true"
           - "--cluster-name={{ .Values.clusterName }}"
           {{- if eq (.Values.replicas | int) 1 }}
@@ -177,3 +178,4 @@ spec:
       serviceAccount: {{ include "controller.serviceAccountName" . }}
       securityContext:
         runAsNonRoot: true
+      terminationGracePeriodSeconds: 120

test/e2e/case1_framework_deployment_test.go

@@ -457,7 +457,8 @@ var _ = Describe("Test framework deployment", func() {
 			container, ok := containers[0].(map[string]interface{})
 			Expect(ok).To(BeTrue())
 
-			if startupProbeInCluster(i) {
+			// Use i+1 since the for loop ranges over a slice skipping first index
+			if startupProbeInCluster(i + 1) {
 				By(logPrefix + "checking for startupProbe on kubernetes 1.20 or higher")
 				_, found, err = unstructured.NestedMap(container, "startupProbe")
 				Expect(err).To(BeNil())

test/e2e/case2_config_deployment_test.go

@@ -98,7 +98,7 @@ var _ = Describe("Test config-policy-controller deployment", func() {
 				"removing the config-policy-controller deployment when the ManagedClusterAddOn CR is removed")
 			Kubectl("delete", "-n", cluster.clusterName, "-f", case2ManagedClusterAddOnCR)
 			deploy := GetWithTimeout(
-				cluster.clusterClient, gvrDeployment, case2DeploymentName, addonNamespace, false, 30,
+				cluster.clusterClient, gvrDeployment, case2DeploymentName, addonNamespace, false, 180,
 			)
 			Expect(deploy).To(BeNil())
 		}
@@ -142,7 +142,7 @@ var _ = Describe("Test config-policy-controller deployment", func() {
 				"removing the config-policy-controller deployment when the ManagedClusterAddOn CR is removed")
 			Kubectl("delete", "-n", cluster.clusterName, "-f", case2ManagedClusterAddOnCR)
 			deploy = GetWithTimeout(
-				cluster.clusterClient, gvrDeployment, case2DeploymentName, addonNamespace, false, 30,
+				cluster.clusterClient, gvrDeployment, case2DeploymentName, addonNamespace, false, 180,
 			)
 			Expect(deploy).To(BeNil())
 		}
@@ -154,7 +154,7 @@ var _ = Describe("Test config-policy-controller deployment", func() {
 	})
 
 	It("should create the default config-policy-controller deployment in hosted mode", Label("hosted-mode"), func() {
-		for i, cluster := range managedClusterList[1:] {
+		for _, cluster := range managedClusterList[1:] {
 			Expect(cluster.clusterType).To(Equal("managed"))
 
 			cluster = managedClusterConfig{
@@ -176,22 +176,17 @@ var _ = Describe("Test config-policy-controller deployment", func() {
 				logPrefix, hubClient, case2ManagedClusterAddOnName,
 				cluster.clusterName, managedClusterList[0].clusterName, installNamespace)
 
-			verifyConfigPolicyDeployment(logPrefix, hubClient, cluster.clusterName, installNamespace, i)
+			verifyConfigPolicyDeployment(logPrefix, hubClient, cluster.clusterName, installNamespace, 0)
 
 			By(logPrefix +
 				"removing the config-policy-controller deployment when the ManagedClusterAddOn CR is removed")
-			err := hubClient.Resource(gvrSecret).Namespace(installNamespace).Delete(
-				context.TODO(), "config-policy-controller-managed-kubeconfig", metav1.DeleteOptions{},
-			)
-			Expect(err).To(BeNil())
-
-			err = clientDynamic.Resource(gvrManagedClusterAddOn).Namespace(cluster.clusterName).Delete(
+			err := clientDynamic.Resource(gvrManagedClusterAddOn).Namespace(cluster.clusterName).Delete(
 				context.TODO(), case2ManagedClusterAddOnName, metav1.DeleteOptions{},
 			)
 			Expect(err).To(BeNil())
 
 			deploy := GetWithTimeout(
-				hubClient, gvrDeployment, case2DeploymentName, installNamespace, false, 30,
+				hubClient, gvrDeployment, case2DeploymentName, installNamespace, false, 180,
 			)
 			Expect(deploy).To(BeNil())
 
@@ -207,7 +202,7 @@ var _ = Describe("Test config-policy-controller deployment", func() {
 			By("Creating the config-policy-controller ClusterManagementAddOn to use the AddOnDeploymentConfig")
 			Kubectl("apply", "-f", case2ClusterManagementAddOnCR)
 
-			for i, cluster := range managedClusterList[1:] {
+			for _, cluster := range managedClusterList[1:] {
 				Expect(cluster.clusterType).To(Equal("managed"))
 
 				cluster = managedClusterConfig{
@@ -229,7 +224,7 @@ var _ = Describe("Test config-policy-controller deployment", func() {
 					logPrefix, hubClient, case2ManagedClusterAddOnName,
 					cluster.clusterName, managedClusterList[0].clusterName, installNamespace)
 
-				verifyConfigPolicyDeployment(logPrefix, hubClient, cluster.clusterName, installNamespace, i)
+				verifyConfigPolicyDeployment(logPrefix, hubClient, cluster.clusterName, installNamespace, 0)
 
 				By(logPrefix + "Removing the ManagedClusterAddOn CR")
 				err := clientDynamic.Resource(gvrManagedClusterAddOn).Namespace(cluster.clusterName).Delete(
@@ -241,7 +236,7 @@ var _ = Describe("Test config-policy-controller deployment", func() {
 					"Verifying controller deployment is removed when the ManagedClusterAddOn CR is removed")
 
 				deploy := GetWithTimeout(
-					hubClient, gvrDeployment, case2DeploymentName, installNamespace, false, 30,
+					hubClient, gvrDeployment, case2DeploymentName, installNamespace, false, 180,
 				)
 				Expect(deploy).To(BeNil())
 
@@ -377,7 +372,7 @@ var _ = Describe("Test config-policy-controller deployment", func() {
 				"removing the config-policy-controller deployment when the ManagedClusterAddOn CR is removed")
 			Kubectl("delete", "-n", cluster.clusterName, "-f", case2ManagedClusterAddOnCR)
 			deploy = GetWithTimeout(
-				cluster.clusterClient, gvrDeployment, case2DeploymentName, addonNamespace, false, 30,
+				cluster.clusterClient, gvrDeployment, case2DeploymentName, addonNamespace, false, 180,
 			)
 			Expect(deploy).To(BeNil())
 		}