kubectl diff exit code 1 for both errors _and_ diffs

Created on 23 Aug 2019  ·  8Comments  ·  Source: kubernetes/kubectl

It seems kubectl diff exits with 1 on both errors (eg for RBAC) or the presence of a diff.

Would be useful if we could have two different codes here.

arekubectl kinfeature sicli

Most helpful comment

This makes a lot of sense to me and the actual code implementation would be trivial. The challenge I suppose is what to select as a sentinel error code to indicate an error interacting with the API vs an error with the diff since the diff status code is propagated from an external binary.

The most common case would be GNU diff which will return 0, 1, or 2 giving us a pretty wide window of codes to use. However since users could have anything in their PATH for "diff" or override the default diff tool via the KUBECTL_EXTERNAL_DIFF env var there is an unknown range of possible exit codes that could be returned by an arbitrary diff tool.

I think it would be reasonable to select a value, I propose 255, and document this in the kubectl diff help message. I'll submit a PR implementing this for discussion.

/kind feature
/area kubectl
/sig cli
/assign

All 8 comments

This makes a lot of sense to me and the actual code implementation would be trivial. The challenge I suppose is what to select as a sentinel error code to indicate an error interacting with the API vs an error with the diff since the diff status code is propagated from an external binary.

The most common case would be GNU diff which will return 0, 1, or 2 giving us a pretty wide window of codes to use. However since users could have anything in their PATH for "diff" or override the default diff tool via the KUBECTL_EXTERNAL_DIFF env var there is an unknown range of possible exit codes that could be returned by an arbitrary diff tool.

I think it would be reasonable to select a value, I propose 255, and document this in the kubectl diff help message. I'll submit a PR implementing this for discussion.

/kind feature
/area kubectl
/sig cli
/assign

It could also make sense to add an option to make the non-zero exit code for the non-error “diff present” case optional. One use case is CI for GitOps workflows, I’ve noticed that kubectl diff catches mistakes that kubectl apply --dry-run does not.

An alternative to settling on a sentinel error code would be to add an --exit-status=123 flag that allows users to specify what fits best, but set this to the proposed 255 by default

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle stale

/remove-lifecycle stale

/wg apply

/assign

One use case is CI for GitOps workflows, I’ve noticed that kubectl diff catches mistakes that kubectl apply --dry-run does not.

kubectl apply --server-dry-run would catch these errors and is the recommended way to go. We're promoting server-dry-run and diff to GA this cycle, so we're trying to improve a few things.

Was this page helpful?
0 / 5 - 0 ratings