Fix ORC managed log collection

[winiciusallan]

When a failure in the CI occurs, the collectlogs script is not properly collecting logs for two reasons:

1. It is not using the FQDN name for the resources, which causes the script to collect the Kubernetes service resource, instead of the Openstack service managed by ORC
2. After the completion of kuttl test, it deletes all of the resources, preventing the script which runs afterwards to collect the managed resources.

This PR add the KUTTL_SKIP_DELETE flag to prevent kuttl of deleting the resources and use FQDN resource names.

Closes #855

[dlaw4608]

LGTM. Both fixes are correct:

1. FQDN resource names in collectlogs properly disambiguates ORC resources from Kubernetes built-ins.
2. --skip-delete ensures test resources survive for log collection on failure.

The --skip-delete is set unconditionally (even on passing tests), but this is harmless since the kind cluster is torn down at the end of the job regardless. Worth revisiting only if make test-examples starts colliding with leftover resources from make test-e2e.

E2E tests failing due to Router and Subnet timeouts, unrelated to the changes in this PR

[mandre]

The first commit is good and fixes a real bug, however I'm less sure about the --skip-delete flag passed to kuttl. If I understand correctly, this applies to all test (and not just failed ones like the comment in hack/e2e.sh suggests) this now means that we're not cleaning any resources from OpenStack, and we could potentially be accumulating lots of resources.

I imagine this is the reason why you had to bump the quota for routers.

Also, we do rely on the cleanup mechanism (the namespace deletion to be exact) in all of our tests to validate we don't leave undeletable resources behind: this ensures that ORC is able to figure out the deletion dependency and applies it correctly. If it can't, we hit the timeout and fail the test, and it leaves the resources there to be collected.

I believe what you're looking for is a new flag in kuttl that would skip the deletion of namespaces for failed tests.

[winiciusallan]

Also, we do rely on the cleanup mechanism (the namespace deletion to be exact) in all of our tests to validate we don't leave undeletable resources behind: this ensures that ORC is able to figure out the deletion dependency and applies it correctly. If it can't, we hit the timeout and fail the test, and it leaves the resources there to be collected.

I believe what you're looking for is a new flag in kuttl that would skip the deletion of namespaces for failed tests.

I agree with you that having a new flag on KUTTL is the cleanest approach for this case. However, I would like to make sure that I'm missing nothing. How do we rely on the namespace cleanup mechanism? For the deletion dependencies, don't we programatically delete the resources and then check that they don't exist anymore (example)? If it was the case, I was expecting at least one of the tests to fail.

[mandre]

Also, we do rely on the cleanup mechanism (the namespace deletion to be exact) in all of our tests to validate we don't leave undeletable resources behind: this ensures that ORC is able to figure out the deletion dependency and applies it correctly. If it can't, we hit the timeout and fail the test, and it leaves the resources there to be collected.
I believe what you're looking for is a new flag in kuttl that would skip the deletion of namespaces for failed tests.

I agree with you that having a new flag on KUTTL is the cleanest approach for this case. However, I would like to make sure that I'm missing nothing. How do we rely on the namespace cleanup mechanism? For the deletion dependencies, don't we programatically delete the resources and then check that they don't exist anymore (example)? If it was the case, I was expecting at least one of the tests to fail.

The code your pointing to checks that the finalizer prevents dependent resources from being deleted, this is different than what I was referring to.

When a test finishes, kuttl deletes the namespace (unless you pass it --skip-delete), and for us this is another implicit test: we verify that ORC can figure out a way to cleanly delete the all the resources without providing explicit ordering. If it doesn't, resources will be stuck and the test will fail due to the timeout.

[winiciusallan]

The code your pointing to checks that the finalizer prevents dependent resources from being deleted, this is different than what I was referring to.

When a test finishes, kuttl deletes the namespace (unless you pass it --skip-delete), and for us this is another implicit test: we verify that ORC can figure out a way to cleanly delete the all the resources without providing explicit ordering. If it doesn't, resources will be stuck and the test will fail due to the timeout.

Gotcha, it makes sense. I opened an issue on kuttl, feel free to give ideas there :)

kudobuilder/kuttl#708