Skip to content
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

Add openstack_cleanup_all make target #970

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

weinimo
Copy link
Contributor

@weinimo weinimo commented Dec 11, 2024

Goal of this new make target is to clean up everything that may be necessary in order to re-run make openstack successfully if it failed before.

@openshift-ci openshift-ci bot requested review from abays and fultonj December 11, 2024 10:23
Copy link
Contributor

openshift-ci bot commented Dec 11, 2024

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: weinimo
Once this PR has been reviewed and has the lgtm label, please assign fultonj for approval. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@weinimo weinimo requested a review from stuggi December 11, 2024 10:24
@weinimo weinimo force-pushed the add_os_cleanup_all_target branch from 5aaeffa to efa4d71 Compare December 11, 2024 10:26
Goal of this new make target is to clean up everything that
may be necessary in order to re-run `make openstack` successfully
if it failed before.
@weinimo weinimo force-pushed the add_os_cleanup_all_target branch from efa4d71 to 7b517f3 Compare December 11, 2024 10:33
Copy link
Contributor

@stuggi stuggi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what is the goal for this target? because make openstack only installs the operators and its dependencies. it does not deploy the ctlplane. should it also cleanup the deployment, like make openstack_deploy_cleanup

Because of that the usual workflow is to run

make openstack_deploy_cleanup
<wait for the deployemts gone>
make openstack_cleanup
make crc_storage_cleanup
...

there you could add the nncp_cleanup at the end and the namespace_cleanup. if you do not wait for the ctlplane resources cleanup up before uninstall the operators, you will have remaining objects with a finalizer. as a result you won't be able to delete the namespace

if you uninstall the operators, the CRDs are still on the cluster. if you also want to remove those, you'd have to run openstack_crds_cleanup.

metallb resource are in the metallb-system namespace. metallb_config_cleanup would remove those, and if you also want to uninstall that operator metallb_cleanup.

similar to cert-manager certmanager_cleanup

@@ -747,6 +747,10 @@ openstack_cleanup: operator_namespace## deletes the operator, but does not clean
oc delete catalogsource --all=true
test -d ${OPERATOR_BASE_DIR}/baremetal-operator && make crc_bmo_cleanup || true

.PHONY: openstack_cleanup_all
openstack_cleanup_all: openstack_cleanup nncp_cleanup namespace_cleanup ## deletes the operator and its resources
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this won't work. openstack_cleanup will uninstall the operators. if they are down, you can not cleanup the openstack namespace.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So I just need to switch the order?

Copy link
Contributor

openshift-ci bot commented Dec 11, 2024

@weinimo: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/install-yamls-deploy 7b517f3 link false /test install-yamls-deploy

Full PR test history. Your PR dashboard.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants