exec mixin should allow certain commands to have non-zero exit codes

[carolynvs]

Sometimes a command is allowed to have a non-zero exit code. For example when deleting a resource that is already gone, our mixins check for the "not found" text on stdout and then continue gracefully.

• Mixin authors should be able to define in the delete action text that means that the resource was already deleted, so that the action succeeds.
• Bundle authors should be able to define text for mixins extending exec (and for exec itself) where if that text is in stdout/err, then the exit code can be ignored and the command considered successful.

Ignore a non-zero exit code when stderr/stdout contains a message

- exec:
    command: helm
    arguments:
    - delete
    - myRelease
    ignoreError:
      output:
        contains: ["not found", "not exist"]

Ignore a non-zero exit code when stderr/stdout matches a regular expression

- exec:
    command: helm
    arguments:
    - delete
    - myRelease
    ignoreError:
      output:
        regex: ["release: .* not found"]

Ignore some exit codes

- exec:
    command: helm
    arguments:
    - delete
    - myrelease
    ignoreError:
      exitCodes: [1, 127]

Ignore all non-zero exit codes

- exec:
    command: rm
    arguments:
    - foo
    ignoreError:
      all: true

If we make the change to the our mixin framework directly (which a lot of mixins use), then other mixins besides the exec mixin can update and get this feature for free too

porter/pkg/exec/builder/execute.go

Lines 145 to 148 in 5efddb8

	err = cmd.Wait()
	if err != nil {
	return "", errors.Wrap(err, fmt.Sprintf("error running command %s", prettyCmd))
	}

This should include test cases for gracefully handling errors, either in execute_test.go or in the exec mixins tests, whichever is easier to add a new test for. I don't believe we have an existing test that you can modify.

It's totally fine to just implement a single use case listed here. The best one to start with is ignoring all errors (last use case).

[karolz-ms]

Yes! Please! And not just for exec mixin, but the bundle author should be allowed to handle a failure of any step. For example, I am creating an Azure resource group using az mixin and I am fine to proceed if the rg already exists.

[carolynvs]

Thanks for letting us know that this would be helpful! Most of the mixins use exec as a base, so they can pick up the feature once we release it in the exec mixin. I'll make sure when we do this to update every mixin that can support it.

[jaudiger]

I also would like this convenient feature. Since it can be useful in some cases. I encountered the problem recently, right now I can fix it by using a script that does the trick for me (aka always sending back a zero exit code). But it could be much more convenient to directly have access to this feature from porter.yaml file.

[carolynvs]

If anyone is interested in implementing this, the change can be made against the main branch (it doesn't have to wait for v1). The relevant code is in the get.porter.sh/pkg/exec/builder package, mainly in action.go, which is shared with all the other mixins.

We should define an interface that any mixin can implement that lets the mixin decide if it wants to ignore the error or not from a command.

type HasErrorHandling interface {
  IsFatalError(exitCode int, output string) bool
}

Mixins can use whatever logic they like then to evaluate the exit code, output and the command executed to decide if the error should cause the bundle to stop execution.

We may want to provide reusable structs and functions for common error handling, similar to what we do for outputs, so that mixins don't have to implement the interface from scratch and have some consistency between mixins.

[carolynvs]

Implemented in #1846