test: add additional integration tests for waved work applier manifest processing

[michaelawyu]

Description of your changes

This PR adds additional integration tests for waved work applier manifest processing.

I have:

• Run make reviewable to ensure this PR is ready for review.

How has this code been tested

N/A

Special notes for your reviewer

N/A

[michaelawyu]

Note: this PR is a follow-up to #234.

[michaelawyu]

Note: this PR includes commits from changes not yet checked in to avoid conflicts -> will rebase after #191 is merged.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

[weng271190436]

there are 3 "if it still exists" object deletions, which are Role, PriorityClass, ConfigMap. Are those supposed to be deleted by the controller after deleteWorkObject but in practice the controller failed to delete them? If so why?

[michaelawyu]

Hi Wei! This is a standard Ginkgo practice where we try to clean up the objects anyhow in the AfterAll block -> this make sures that even if the current test spec fails in-between, other tests can still run with no worries for alreadyExists types of errors (note that our Ginkgo setup allows parallelization and specs assigned to the same Ginkgo process may use the same namespace/resource names).

[michaelawyu]

Hi Wei! This is a standard Ginkgo practice where we try to clean up the objects anyhow in the AfterAll block -> this make sures that even if the current test spec fails in-between, other tests can still run with no worries for alreadyExists types of errors or any other side-effects (though admittedly the chances for this to occur in this specific integration test setup are quite low with the random suffix + the fact that envtest does not allow namespace GC).

[weng271190436]

Thanks for explaining why cleanup is necessary!

I still have a question on why only need to clean up the role but not the namespace? From what I can see you applied both role and namespace to the member cluster but in the cleanup you explicitly delete the role only

I was thinking maybe deleteWorkObject handles the namespace cleanup. But then shouldn't deleteWorkObject also handle the role cleanup?

[michaelawyu]

Hi Wei! The reason why we do not clean up the namespace was that the envtest package we use for running integration tests do not support namespace deletion -> if the Work object includes a namespace manifest, after it is applied, it cannot be deleted (and the deletion won't trigger any errors, it's just stuck there) -> so there's no point in deleting the NS. In the environment we do not have GC controllers either so deleting the NS will not delete objects within in cascade.