operator-sdk cleanup: remove packagemanifests, add generic command#3644
operator-sdk cleanup: remove packagemanifests, add generic command#3644joelanford merged 17 commits intooperator-framework:masterfrom
Conversation
86b0fb3 to
48012c2
Compare
| operator "github.com/operator-framework/operator-sdk/internal/olm/operator" | ||
| operator2 "github.com/operator-framework/operator-sdk/internal/operator" |
There was a problem hiding this comment.
There's some related work going on with run bundle in the github.com/operator-framework/operator-sdk/internal/operator package.
The thought is to do the new run design in the new package, and once that's mostly there we can port the existing run packagemanifests code (mainly the registry setup) into that package.
See #3626
2b7d827 to
56b1331
Compare
estroz
left a comment
There was a problem hiding this comment.
Structurally looks fine, mostly nits/fixes.
|
|
||
| By("destroying the deployed package manifests-formatted operator") | ||
| cleanupPkgManCmd := exec.Command(tc.BinaryName, "cleanup", projectName, | ||
| "--timeout", "4m") |
There was a problem hiding this comment.
Follow-up: helm's e2e tests should be using the test namespace, and run/cleanup should have --namespace=<tc.Kubectl.Namespace> in their command args.
e6dfc0c to
0671078
Compare
| package e2e_test | ||
|
|
||
| import ( | ||
| "testing" |
There was a problem hiding this comment.
Replace:
func TestE2E(t *testing.T) {
RegisterFailHandler(Fail)
RunSpecs(t, "Operator SDK e2e suite")
}
With:
// TestE2EGo ensures the Helm projects built with the Go tool by using its binary.
func TestE2EGo(t *testing.T) {
if testing.Short() {
t.Skip("skipping Operator SDK E2E Go Suite testing in short mode")
}
RegisterFailHandler(Fail)
RunSpecs(t, "E2EGo Suite")
}
Otherwise, the test will be called in the target make test-unit
There was a problem hiding this comment.
make test-unit skips the entire test package with go list ./... | grep -v -E 'github.com/operator-framework/operator-sdk/test/'.
There was a problem hiding this comment.
HI @estroz and @joelanford,
IMO we must implement solutions idempotent for the tests which means that they should not rely on the Travis or make targets. So, if a developer run go tests ../... in the project it should call the tests that should be executed and finish with success what will not occur in this case.
However, it is addressed in the PR: https://github.com/operator-framework/operator-sdk/pull/3499/files
There was a problem hiding this comment.
Otherwise, if we do not force as to follow up this kind of path then it contributes to the scenario where is not possible run the tests outside of the Travis (locally).
| // kubectx stores the k8s context from where the tests are running | ||
| kubectx string | ||
| // projectName is the name of the test project | ||
| projectName string |
There was a problem hiding this comment.
we do not need that we can use tc.TestSuffix
Description of the change:
operator-sdk cleanup packagemanifests <packageManifestsDir>operator-sdk cleanup <packageName>operator-sdk run packagemanifeststo include owner reference on catalog source resourcesMotivation for the change:
Simplify
cleanupcommand to work with arbitrary installation methods (e.g.run packagemanifestsand the upcomingrun bundle)Checklist
If the pull request includes user-facing changes, extra documentation is required:
changelog/fragments(seechangelog/fragments/00-template.yaml)website/content/en/docs