Add 'MissingSecret' reason for pending builds #8792
Conversation
|
@bparees @smarterclayton PTAL |
| // When build is in pending state due to missing pull secrets for the image | ||
| // indicate this condition in reason. | ||
| if secret := build.Spec.Output.PushSecret; secret != nil { | ||
| if bc.SecretClient.Secrets(build.Namespace).Get(secret.Name); err != nil && errors.IsNotFound(err) { |
There was a problem hiding this comment.
how often is this going to get called? you should probably not fetch the secrets if the reason is already set to status missing secret.
| // StatusReasonMissingSecret indicates that the build is missing required | ||
| // secret for pushing the output image. The build will stay in the pending | ||
| // state until the secret is not created or the build timeout. | ||
| StatusReasonMissingSecret = "MissingSecret" |
There was a problem hiding this comment.
MissingPushSecret unless we're going to reuse this for missing source secrets and missing pull secrets.
There was a problem hiding this comment.
@bparees it would be nice to re-use it, do you want to create special reason for each secret or have one generic?
There was a problem hiding this comment.
it's more informative to the user if it's specific to which secret is missing.
|
@smarterclayton @bparees fixed && rebased PTAL [test] |
|
@bparees @smarterclayton updated. |
|
|
||
| // StatusReasonMissingPushSecret indicates that the build is missing required | ||
| // secret for pushing the output image. The build will stay in the pending | ||
| // state until the secret is not created or the build timeout. |
There was a problem hiding this comment.
"The build will stay in the pending state until the secret is created, or the build times out."
mfojtik
force-pushed
the
pending-secrets
branch
4 times, most recently
from
42cb57b
to
e9670ec
Compare
|
@bparees ptal |
| nextStatus = buildapi.BuildPhaseRunning | ||
| case kapi.PodPending: | ||
| nextStatus = buildapi.BuildPhasePending |
There was a problem hiding this comment.
needs to set build.Status.Reason="" first, to ensure the old reason (if any) is cleared, if we change the state to pending and do not have a pushsecret issue.
There was a problem hiding this comment.
yeah, but it will also interfere with the check of the reason is StatusReasonMissingPushSecret to skip the secret fetching... I cached the original reason, so it should be fine now.
|
Evaluated for origin test up to ba73984 |
|
continuous-integration/openshift-jenkins/test FAILURE (https://ci.openshift.redhat.com/jenkins/job/test_pr_origin/5539/) |
|
lgtm but you appear to have some test issues. |
|
[merge] |
|
continuous-integration/openshift-jenkins/merge SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pr_origin/5649/) (Image: devenv-rhel7_4495) |
|
[merge] |
|
Evaluated for origin merge up to ba73984 |
Fixes: #6781
When you have a build configuration and you pushing to a private repository in external registry and you set the push secret but the push secret does not exists, we should set the reason for the build to be in the pending state.
This PR will add 'MissingSecret' reason for such build. This will be displayed next to 'Pending'.