Trigger uninstalls through a new annotation

.github/workflows/kind.yml

@@ -87,17 +87,22 @@ jobs:
  export GOPATH=$(go env GOPATH)
  KUBECONFIG=${PWD}/kubeconfig_managed make e2e-test-hosted-mode-coverage
 
+ - name: Verify Deployment Configuration
+ run: |
+ make build-images
+ KUBECONFIG=${PWD}/kubeconfig_managed_e2e make kind-deploy-controller-dev
+
+ - name: E2E tests that require the controller running in a cluster
+ run: |
+ export GOPATH=$(go env GOPATH)
+ KUBECONFIG=${PWD}/kubeconfig_managed make e2e-test-running-in-cluster
+
  - name: Test Coverage Verification
  if: ${{ github.event_name == 'pull_request' }}
  run: |
  make test-coverage
  make coverage-verify
 
- - name: Verify Deployment Configuration
- run: |
- make build-images
- KUBECONFIG=${PWD}/kubeconfig_managed_e2e make kind-deploy-controller-dev
-
  - name: Debug
  if: ${{ failure() }}
  run: |

Makefile

@@ -334,15 +334,19 @@ e2e-test: e2e-dependencies
  $(GINKGO) -v --fail-fast --slow-spec-threshold=10s $(E2E_TEST_ARGS) test/e2e
 
 .PHONY: e2e-test-coverage
-e2e-test-coverage: E2E_TEST_ARGS = --json-report=report_e2e.json --label-filter='!hosted-mode' --output-dir=.
+e2e-test-coverage: E2E_TEST_ARGS = --json-report=report_e2e.json --label-filter='!hosted-mode && !running-in-cluster' --output-dir=.
 e2e-test-coverage: e2e-run-instrumented e2e-test e2e-stop-instrumented
 
 .PHONY: e2e-test-hosted-mode-coverage
-e2e-test-hosted-mode-coverage: E2E_TEST_ARGS = --json-report=report_e2e_hosted_mode.json --label-filter="hosted-mode" --output-dir=.
+e2e-test-hosted-mode-coverage: E2E_TEST_ARGS = --json-report=report_e2e_hosted_mode.json --label-filter="hosted-mode && !running-in-cluster" --output-dir=.
 e2e-test-hosted-mode-coverage: COVERAGE_E2E_OUT = coverage_e2e_hosted_mode.out
 e2e-test-hosted-mode-coverage: export TARGET_KUBECONFIG_PATH = $(PWD)/kubeconfig_managed2
 e2e-test-hosted-mode-coverage: e2e-run-instrumented e2e-test e2e-stop-instrumented
 
+.PHONY: e2e-test-running-in-cluster
+e2e-test-running-in-cluster: E2E_TEST_ARGS = --label-filter="running-in-cluster" --covermode=atomic --coverprofile=coverage_e2e_uninstall.out --coverpkg=open-cluster-management.io/config-policy-controller/pkg/triggeruninstall
+e2e-test-running-in-cluster: e2e-test
+
 .PHONY: e2e-build-instrumented
 e2e-build-instrumented:
  go test -covermode=atomic -coverpkg=$(shell cat go.mod | head -1 | cut -d ' ' -f 2)/... -c -tags e2e ./ -o build/_output/bin/$(IMG)-instrumented

build/Dockerfile

@@ -24,7 +24,7 @@ COPY --from=builder ${REPO_PATH}/build/_output/bin/${COMPONENT} ${OPERATOR}
 COPY --from=builder ${REPO_PATH}/build/bin /usr/local/bin
 RUN /usr/local/bin/user_setup
 
-ENTRYPOINT ["/usr/local/bin/entrypoint"]
+ENTRYPOINT ["/usr/local/bin/entrypoint", "controller"]
 
 RUN microdnf update && \
  microdnf clean all

controllers/configurationpolicy_controller.go

@@ -163,20 +163,11 @@ func (r *ConfigurationPolicyReconciler) PeriodicallyExecConfigPolicies(
  }
 
  const waiting = 10 * time.Minute
- var exiting bool
- // Loop twice after exit condition is received to account for race conditions and retries.
- loopsAfterExit := 2
 
- for !exiting || (exiting && loopsAfterExit > 0) {
- start := time.Now()
-
- select {
- case <-ctx.Done():
- exiting = true
- loopsAfterExit--
- default:
- }
+ exiting := false
 
+ for !exiting {
+ start := time.Now()
  policiesList := policyv1.ConfigurationPolicyList{}
 
  var skipLoop bool
@@ -206,28 +197,6 @@ func (r *ConfigurationPolicyReconciler) PeriodicallyExecConfigPolicies(
  skipLoop = true
  }
 
- needDeploymentFinalizer := false
-
- for i := range policiesList.Items {
- plc := policiesList.Items[i]
-
- if objHasFinalizer(&plc, pruneObjectFinalizer) {
- needDeploymentFinalizer = true
-
- break
- }
- }
-
- if err := r.manageDeploymentFinalizer(needDeploymentFinalizer); err != nil {
- if errors.Is(err, common.ErrNoNamespace) || errors.Is(err, common.ErrRunLocal) {
- log.Info("Not managing the controller's deployment finalizer because it is running locally")
- } else {
- log.Error(err, "Failed to manage the controller's deployment finalizer, skipping loop")
-
- skipLoop = true
- }
- }
-
  cleanupImmediately, err := r.cleanupImmediately()
  if err != nil {
  log.Error(err, "Failed to determine if it's time to cleanup immediately")
@@ -281,6 +250,12 @@ func (r *ConfigurationPolicyReconciler) PeriodicallyExecConfigPolicies(
  log.V(2).Info("Sleeping before reprocessing the configuration policies", "seconds", sleepTime)
  time.Sleep(sleepTime)
  }
+
+ select {
+ case <-ctx.Done():
+ exiting = true
+ default:
+ }
  }
 }
 
@@ -618,10 +593,10 @@ func (r *ConfigurationPolicyReconciler) cleanUpChildObjects(plc policyv1.Configu
 
 // cleanupImmediately returns true when the cluster is in a state where configurationpolicies should
 // be removed as soon as possible, ignoring the pruneObjectBehavior of the policies. This is the
-// case when the CRD or the controller's deployment are already being deleted.
+// case when the controller is being uninstalled or the CRD is being deleted.
 func (r *ConfigurationPolicyReconciler) cleanupImmediately() (bool, error) {
- deployDeleting, deployErr := r.deploymentIsDeleting()
- if deployErr == nil && deployDeleting {
+ beingUninstalled, beingUninstalledErr := r.isBeingUninstalled()
+ if beingUninstalledErr == nil && beingUninstalled {
  return true, nil
  }
 
@@ -630,36 +605,16 @@ func (r *ConfigurationPolicyReconciler) cleanupImmediately() (bool, error) {
  return true, nil
  }
 
- if deployErr == nil && defErr == nil {
+ if beingUninstalledErr == nil && defErr == nil {
  // if either was deleting, we would've already returned.
  return false, nil
  }
 
  // At least one had an unexpected error, so the decision can't be made right now
  //nolint:errorlint // we can't choose just one of the errors to "correctly" wrap
- return false, fmt.Errorf("deploymentIsDeleting error: '%v', definitionIsDeleting error: '%v'",
- deployErr, defErr)
-}
-
-func (r *ConfigurationPolicyReconciler) deploymentIsDeleting() (bool, error) {
- key, keyErr := common.GetOperatorNamespacedName()
- if keyErr != nil {
- if errors.Is(keyErr, common.ErrNoNamespace) || errors.Is(keyErr, common.ErrRunLocal) {
- // running locally
- return false, nil
- }
-
- return false, keyErr
- }
-
- deployment := appsv1.Deployment{}
-
- err := r.Get(context.TODO(), key, &deployment)
- if err != nil {
- return false, err
- }
-
- return deployment.DeletionTimestamp != nil, nil
+ return false, fmt.Errorf(
+ "isBeingUninstalled error: '%v', definitionIsDeleting error: '%v'", beingUninstalledErr, defErr,
+ )
 }
 
 func (r *ConfigurationPolicyReconciler) definitionIsDeleting() (bool, error) {
@@ -2808,32 +2763,23 @@ func convertPolicyStatusToString(plc *policyv1.ConfigurationPolicy) (results str
  return result
 }
 
-func (r *ConfigurationPolicyReconciler) manageDeploymentFinalizer(shouldBeSet bool) error {
+func (r *ConfigurationPolicyReconciler) isBeingUninstalled() (bool, error) {
  key, err := common.GetOperatorNamespacedName()
  if err != nil {
- return err
+ // Running locally
+ if errors.Is(err, common.ErrNoNamespace) || errors.Is(err, common.ErrRunLocal) {
+ return false, nil
+ }
+
+ return false, err
  }
 
  deployment := appsv1.Deployment{}
  if err := r.Client.Get(context.TODO(), key, &deployment); err != nil {
- return err
- }
-
- if objHasFinalizer(&deployment, pruneObjectFinalizer) {
- if shouldBeSet {
- return nil
- }
-
- deployment.SetFinalizers(removeObjFinalizer(&deployment, pruneObjectFinalizer))
- } else {
- if !shouldBeSet {
- return nil
- }
-
- deployment.SetFinalizers(addObjFinalizer(&deployment, pruneObjectFinalizer))
+ return false, err
  }
 
- return r.Update(context.TODO(), &deployment)
+ return deployment.Annotations[common.UninstallingAnnotation] == "true", nil
 }
 
 func recoverFlow() {

controllers/configurationpolicy_utils.go

@@ -549,7 +549,6 @@ func addObjFinalizer(obj metav1.Object, finalizer string) []string {
  return append(obj.GetFinalizers(), finalizer)
 }
 
-// nolint: unparam
 func removeObjFinalizer(obj metav1.Object, finalizer string) []string {
  result := []string{}
 

deploy/manager/manager.yaml

@@ -20,6 +20,7 @@ spec:
  command:
  - config-policy-controller
  args:
+ - "controller"
  - "--enable-lease=true"
  - "--log-level=2"
  - "--v=0"

deploy/operator.yaml

@@ -45,6 +45,7 @@ spec:
  spec:
  containers:
  - args:
+ - controller
  - --enable-lease=true
  - --log-level=2
  - --v=0