Gracefully handle unreachable k8s cluster #946
Conversation
|
Here's the approach I was starting in on until the holiday break: https://gist.github.com/pgavlin/35c55c320d9904d04aa4674ff3c9a615 I stalled out on the error classification bit. FWIW, I would rather use this sort of dynamic approach than the static approach--I would expect us to fail as we do today if the kubeconfig is invalid. |
If I understand correctly, you want:
Is that right? |
|
To follow up on that, I think my current approach on |
Yes.
Essentially, yes. As long as |
lblackstone
force-pushed
the
lblackstone/provider-no-cluster
branch
2 times, most recently
from
512a8fe to
0667766
Compare
lblackstone
changed the title
|
@pgavlin This should be RFR now. I tested these changes locally with the invoke changes, and it seems to be working. |
| { | ||
| Dir: "step2", | ||
| Additive: false, | ||
| ExpectFailure: true, |
There was a problem hiding this comment.
Is this test really testing anything? I assume it passed previously as well - just with a different error?
Is it worth having a test that doesn't fail after these changes?
There was a problem hiding this comment.
Yeah, good point. I think what I really want to test is that a preview succeeds even when a cluster isn't reachable, but I don't think our test framework currently handles that case (preview only, no update).
There was a problem hiding this comment.
Justin informed me that this is possible using RunCommand("pulumi", "preview"), so I'll update the test accordingly.
There was a problem hiding this comment.
This turned out to be more difficult than I expected, so I'm opening an issue to update the test framework and will follow up on this later.
Previously, the provider erroneously expected that the default provider pointed to a functioning Kubernetes cluster. This led to unexpected failures in cases where this wasn't true, such as the user manually setting the kubeconfig value for the stack to an invalid value. This change explicitly checks for a valid configuration, and returns with a descriptive error if this check fails.
lblackstone
force-pushed
the
lblackstone/provider-no-cluster
branch
from
166be7a to
cffa7a6
Compare
Reintroduce the reverted changed (#941) from #925 and #934 with a few additional fixes related to the changes in #946. The major changes include the following: - Use a runtime invoke to call a common decodeYaml method in the provider rather than using YAML libraries specific to each language. - Use the namespace parameter of helm.v2.Chart as a default, and set it on known namespace-scoped resources.
Reintroduce the reverted changed (#941) from #925 and #934 with a few additional fixes related to the changes in #946. The major changes include the following: - Use a runtime invoke to call a common decodeYaml method in the provider rather than using YAML libraries specific to each language. - Use the namespace parameter of helm.v2.Chart as a default, and set it on known namespace-scoped resources.
…ces (#952) Reintroduce the reverted changed (#941) from #925 and #934 with a few additional fixes related to the changes in #946. The major changes include the following: - Use a runtime invoke to call a common decodeYaml method in the provider rather than using YAML libraries specific to each language. - Use the namespace parameter of helm.v2.Chart as a default, and set it on known namespace-scoped resources.
Proposed changes
Previously, the provider erroneously expected that the default provider pointed to a functioning Kubernetes cluster. This led to unexpected failures in cases where this wasn't true, such as the user manually setting the kubeconfig value for the stack to an invalid value. This change explicitly checks for a valid configuration, and falls back to a degraded state if this check fails. This still
allows invoke logic to run during previews without requiring an active k8s cluster.
Related issues (optional)
Fixes #950