Add skip-cleanup-on-errors option to test local command

[JAORMX]

Description of the change:
This flag (which is false by default) tells the framework whether to
skip the clean up (or do it) if the test encountered errors.

Motivation for the change:

With the current behavior, the framework will always clean up the
resources it created. This is fine in most cases, however, in CI,
whenever there's an error, we might not get the logs of the workloads
and operator after the cleanup. So this option enables those resources
to linger until the logs have been acquired.

[camilamacedo86]

Thank you for your collab 👍. Following some requirements.

• Few changes were requested. Please, could you check?
• Fix lint issues:

Also, the code is not passing in the lint. ( to test/check locally run make lint )
See the errors faced and how to solve them:

cmd/operator-sdk/test/local.go:86: line is 146 characters (lll)
	testCmd.Flags().BoolVar(&tlConfig.cleanupOnErrors, "cleanup-on-errors", true, "Clean up created esources even after the test encountered errors")

It is regards the size of the line.
Devs should not need to give the scroll to check the code implementation
Just broken in 2 lines ( put the msg on bottom )

pkg/test/framework.go:77:20: struct of size 96 bytes could be of size 88 bytes (maligned)
type frameworkOpts struct {

It is regarded the memory allocation optimization. Keep all bools declared at the end.
More info: https://medium.com/@sebassegros/golang-dealing-with-maligned-structs-9b77bacf4b97

• Update the docs by running the command gen-cli-doc

[JAORMX]

@camilamacedo86 thanks for the suggestions and the thorough review! Just addressed the review with another commit.

[JAORMX]

@camilamacedo86 thanks for the suggestions and the thorough review! Just addressed the review with another commit.

[camilamacedo86]

Hi @JAORMX,

Really tks for the prompt reply. See here that is still missing the fix the issues pointed out by the lint.

You can check/test it locally by running make lint before push the changes. Also, check the comment my tips for you to solve them #2512 (review).

[JAORMX]

@camilamacedo86 I ran make lint but got no output :/

[camilamacedo86]

Hi @JAORMX,

I am getting the same output that you can check in the CI for it. To know more about see: https://github.com/golang/lint. We need to have the code fixed, passing in the CI for we are able to approve and merge OK? Feel free to ping me in the slack if you need help.

[jmccormick2001]

/lgtm with some comments I added that would rephrase some of the user facing messages.

[JAORMX]

changed "artefact" for artifact as suggested, and addressed the merge conflict in the changelog.

[camilamacedo86]

/lgtm

[estroz]

@JAORMX why not add a cleanup function that tails logs at the end of your test so it runs before any cleanup function?

[JAORMX]

@estroz cause we can't ensure the order of the cleanup functions AFAIK. This just makes it a lot easier and it hooks natively with what the ci-operator does in OpenShift CI which is basically that... grabbing all containers and pulling the logs into the artifacts directory. That way, no extra code needs to be written per project, and we can just use this flag.

[estroz]

@JAORMX cleanup functions are executed in LIFO order. Nothing documents this behavior though 👎.

Can you elaborate on what you mean by "hooks natively with what the ci-operator does"? Is this hook specific to ci-operator/OpenShift CI or generally used in prow? This is a nice-to-have feature, but OpenShift-specific features shouldn't be added upstream for the sake of OpenShift alone.

[JAORMX]

I'm honestly not sure if this is a general prow feature or not. And this is an optional flag that is not in use by default. I proposed it here because I thought it would be a more general case. Else I would keep the changes in our own repos.

[estroz]

@JAORMX I think this is useful in general as well, just wanted to clarify why this was being proposed. Thanks for contributing!

[estroz]

@JAORMX can you document this flag with the behavior you've described here? Thanks!

[JAORMX]

Mind if I document this in a separate PR?

[camilamacedo86]

Hi @JAORMX

Regards,

@JAORMX can you document this flag with the behavior you've described here? Thanks!

Following the suggestion.

**Skip Cleanup Error**

If you wish to skip the function which will remove all artefacts when an error is faced to perform this operation then, you can use the `--skip-cleanup-error` flag:

$ operator-sdk test local ./test/e2e --image quay.io/example/my-operator:v0.0.2 --skip-cleanup-error

[camilamacedo86]

/lgtm