-
Notifications
You must be signed in to change notification settings - Fork 19
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Trigger uninstalls through a new annotation #104
Trigger uninstalls through a new annotation #104
Conversation
/hold for the addon controller change |
e21129d
to
ab25708
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nothing wrong, just questions/comments. I think this will work.
44acaa4
to
beea38c
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🤞
/unhold |
Makefile
Outdated
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's --covermode
, but we'll find out
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_ARGS = --label-filter="running-in-cluster" --covermode=atomic --coverprofile=coverage_e2e_uninstall.out --coverpkg=open-cluster-management.io/config-policy-controller/pkg/triggeruninstall |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The other way worked locally, but I missed that it wasn't consistent. I just fixed it.
Previous to this, a finalizer on the Deployment was added so that if the Deployment was deleted, it would handle immediate clean up. This doesn't handle the common case where the config-policy-controller ManagedClusterAddOn is deleted, which causes the ManifestWork to be deleted, which triggers all Configuration Policy controller deployment artifacts, including the service account. A new approach is taken so that a new annotation of policy.open-cluster-management.io/uninstalling=true is set on the Deployment to indicate that the Configuration Policy controller should remove all finalizers because it's getting deleted. The Policy Addon controller will be updated so that when the config-policy-controller ManagedClusterAddOn object is deleted, a finalizer will prevent it and a Pod will run on the managed cluster with the new `trigger-uninstall` subcommand. This sets the uninstalling annotation on the Deployment and then waits until all ConfigurationPolicy finalizers have been removed. Once the command ends, the Pod exits, and the ManagedClusterAddOn object's finalizer is removed and the uninstall proceeds. Relates: https://issues.redhat.com/browse/ACM-3233 https://issues.redhat.com/browse/ACM-2923 Signed-off-by: mprahl <mprahl@users.noreply.github.com>
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: JustinKuli, mprahl The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Previous to this, a finalizer on the Deployment was added so that if the Deployment was deleted, it would handle immediate clean up. This doesn't handle the common case where the config-policy-controller ManagedClusterAddOn is deleted, which causes the ManifestWork to be deleted, which triggers all Configuration Policy controller deployment artifacts, including the service account.
A new approach is taken so that a new annotation of policy.open-cluster-management.io/uninstalling=true is set on the Deployment to indicate that the Configuration Policy controller should remove all finalizers because it's getting deleted.
The Policy Addon controller will be updated so that when the config-policy-controller ManagedClusterAddOn object is deleted, a finalizer will prevent it and a Pod will run on the managed cluster with the new
trigger-uninstall
subcommand. This sets the uninstalling annotation on the Deployment and then waits until all ConfigurationPolicy finalizers have been removed. Once the command ends, the Pod exits, and the ManagedClusterAddOn object's finalizer is removed and the uninstall proceeds.Relates:
https://issues.redhat.com/browse/ACM-3233
https://issues.redhat.com/browse/ACM-2923