Fix await status for Job and Pod #2299
Conversation
Does the PR have any schema changes?Looking good! No breaking changes found. |
|
I'd like to see (in the commit message, ideally) an explanation of what was wrong with the way it was done previously (i.e., in #1856), and how this fixes it. It is far from self-evident! |
Fix a regression where the intermediate status of Job and Pod resources was no longer printed during an update. The cloud-ready-checks package changed the way that messages are populated in the state checker. Previously, each condition was returned as a StatusMessage when the Update method was called. During the refactor, the Update method was replaced by the ReadyDetails method, which changed the return type. The updated return type includes a Description and an optional message that was only set when a problem was detected. The regression was caused by failing to print the description if the message was empty. Thus, intermediate status was still printed in case of errors, but was erroneously skipped on the happy path. This change updates the two usages of this library to iterate the result objects and log each result as a status message. This approximates the previous behavior, and fixes the regression. The cloud-ready-checks package was intended to make this process easier, but the current interface still needs some work, since it shouldn't be necessary for clients to understand this nuance between the Description and Message field.
lblackstone
force-pushed
the
lblackstone/await-status-fix
branch
from
9ed51c7
to
e06f385
Compare
Does the PR have any schema changes?Looking good! No breaking changes found. |
There was a problem hiding this comment.
The commit message (and PR description) check out -- thanks Levi 💯 So the crucial bit is that checker.Update(...) would construct a status message (https://github.com/pulumi/pulumi-kubernetes/pull/1856/files#diff-cd291f815be7da9f37d3fc13e934cc2781d2316fd473e4e0954f2314a3f6e88eL68) from descriptions; the replacement, checker.ReadyDetails, simply doesn't create those messages. With that in mind, the solution here of printing the descriptions makes sense.
Proposed changes
Fix a regression where the intermediate status of Job and Pod resources was no longer printed during an update.
The cloud-ready-checks package changed the way that messages are populated in the state checker. Previously, each condition was returned as a StatusMessage when the Update method was called. During the refactor, the Update method was replaced by the ReadyDetails method, which changed the return type. The updated return type includes a Description and an optional message that was only set when a problem was detected. The regression was caused by failing to print the description if the message was empty. Thus, intermediate status was still printed in case of errors, but was erroneously skipped on the happy path.
This change updates the two usages of this library to iterate the result objects and log each result as a status message. This approximates the previous behavior, and fixes the regression. The cloud-ready-checks package was intended to make this process easier, but the current interface still needs some work, since it shouldn't be necessary for clients to understand this nuance between the Description and Message field.
Related issues (optional)
Fix #2298