google/knative-gcp

Properly tear down knative related installation

cathyzhyi opened this issue · 6 comments

Describe the bug
In e2e-secret-tests.sh knative_teardown overwrites the original definition

function knative_teardown() {
and just removes some temp file. However, the original knative_teardown defined in
function knative_teardown() {
does more complicated job and uninstall things properly.

Similar for test_teardown:
our definition

function test_teardown() {

original definition

Expected behavior
Maybe we should also uninstall resources properly as the original definition does?

To Reproduce
Steps to reproduce the behavior.

Knative-GCP release version

Additional context
Add any other context about the problem here such as proposed priority

The test_teardown in eventing is to uninstall the channel CRD, which is not applicable in knative-gcp. knative-teardown however removes the system namespace, which knative-gcp will have problem due to the dependency of webhook and brokercell in the same namespace. The whole teardown is not very important as the Prow job will remove the whole cluster after the test, but not sure what people will expect in local test.

I thought the knative_teardown in knative-gcp needs to cleanup knative-gcp related resources and then do whatever the original knative_teardown in e2e-common.sh does to cleanup eventing related resources.

Hey @zhongduo do you know more about @cathyzhyi followup Q?

I agree to teardown the resources properly. If you are going to uninstall knative-gcp, just make sure you remove the webhook in the last step rather than removing the namespace directly. Having said that, the teardown process is not tested in Prow jobs as the whole cluster will be deleted.

I just feel surprising when noticing this and want to raise the issue to make sure it's not unintentional. If the testing team believes this is not an issue I can close it.

This issue is stale because it has been open for 90 days with no
activity. It will automatically close after 30 more days of
inactivity. Reopen the issue with /reopen. Mark the issue as
fresh by adding the comment /remove-lifecycle stale.