How to treat builds upon unsuccesfull retries

[soltysh]

Problem

1. I create a BuildConfig object with non-existing ImageStream. When a new build is created from such BC, current build controller implementation will retry to schedule that build for ever, because this error is marked as recoverable.
2. I create a BuildConfig object with non-existing pullSecretName or sourceSecretName'. When a new build is created from such BC the build pod gets created but will hang inPending` state, and you'll see following errors:

E0512 13:56:58.757800   13539 secret.go:119] Couldn't get secret test/mysecret
E0512 13:56:58.757986   13539 kubelet.go:943] Unable to mount volumes for pod "ruby-sample-build-2_test": secrets "mysecret" not found; skipping pod
E0512 13:56:58.782942   13539 pod_workers.go:102] Error syncing pod b6c9609f-f8ae-11e4-a008-080027c5bfa9, skipping: secrets "mysecret" not found

Solution

I see two possible solutions in this situation:

1. Fail the build with message, where the message contains the error log. There's my initial try to address problem 1 in Fail build after several unsuccessfull retries #2220. The idea there is to have a timer based retry, we'd fail the build after 30 mins of retries.
2. Leave the build in endless retry but still providing the user with information about current status, where the status would contain eventual errors described above. Yet with this we should also provide user with simple tool allowing removing previous build and pod at once, although maybe cancel should suffice here. This is current outcome from the issue I opened upstream Missing secret loops endlessly kubernetes/kubernetes#8178

//cc @bparees @mfojtik @smarterclayton

[bparees]

I would vote for eventual build failures in both cases, because build failures are more obvious to users than builds that never complete (eg in the case of jenkins, one sends you an email, the other you just forget about).

But with regards to the secret issue, i'm not sure builds can solve it themselves. they are waiting for a pod to complete/fail that never will. We're dependent on what k8s decides to do with the pod in this situation, it's no different for a build than any other pod with a bad volume reference, i'd assume. So that's up to k8s whether they're going to eventually fail such a pod (if it's a runonce pod, it would seem to me it's senssible to fail it eventually) or not. I don't see us changing anything in builds to deal with that.

In theory we could try to validate the secret before creating the build pod, but i think that puts us in a position of validating more than we should be, plus there's no guarantee the secret doesn't get deleted after we validate it exists but before the pod gets created.

[smarterclayton]

On May 15, 2015, at 2:54 PM, Ben Parees notifications@github.com wrote:

I would vote for eventual build failures in both cases, because build failures are more obvious to users than builds that never complete (eg in the case of jenkins, one sends you an email, the other you just forget about).

But with regards to the secret issue, i'm not sure builds can solve it themselves. they are waiting for a pod to complete/fail that never will. We're dependent on what k8s decides to do with the pod in this situation, it's no different for a build than any other pod with a bad volume reference, i'd assume. So that's up to k8s whether they're going to eventually fail such a pod (if it's a runonce pod, it would seem to me it's senssible to fail it eventually) or not. I don't see us changing anything in builds to deal with that.

In theory we could try to validate the secret before creating the build pod, but i think that puts us in a position of validating more than we should be, plus there's no guarantee the secret doesn't get deleted after we validate it exists but before the pod gets created.

The message from the pod status should probably be reflected back to the build status. Ben I think your position is reasonable.
—
Reply to this email directly or view it on GitHub.

[bparees]

Reflecting pod status messages in the build status is a good idea. Probably related to the story we need to do to put events into the build itself as we track it through it's lifecycle.

[mfojtik]

How is having a PodTemplate with non-existing (yet) Secret different from having a PodTemplate with Image which does not exists (yet)? Can we allow users to set the maximum build retry count for a build or allow them to set the maximum time that the build will be considered as failed (or the Build status will change from Pending to "Staled" (or Failed) ?

Example: I'm leaving from work and I started a Build. I know that Bob (devops) will push the Docker image in next 2 hours, but I don't want to wait for Bob to kick the build of my app. So I go ahead and start it (and I know that the image does not exists yet). In that case the Build will be Pending for next 2 hours and then execute using the image that Bob provided.
I know that Bob told me he will be back in 2 hours, so I can set the maximum timeout to let say 3 hours?

[bparees]

it's different because we (the build controller) have to resolve the ImageStream to an Image before we can create the pod.

We don't have to resolve the secret before creating the pod.

[soltysh]

I like the additional validation @bparees mentioned and I also vote for build failures as this is more natural flow for developers (build-fail over and over again until you get build-success-run).
As for pods we're currently stuck to what k8s decides, on handling them in situations such as the one with missing secrets, so probably putting events into build logs/messages makes sense. In the future when we'll introduce RunOnce pods hopefully, we should try to force that kind of behavior that will fail, eventually simplifying our build flow.
As for @mfojtik example, I think it's too complicated in what I experienced so far.

[bparees]

I was a actually suggesting we not do that additional validation as validating pod configuration shouldn't be the job of the build controller imho, and once we start adding those rules it's not clear where we stop.

We'd also have to look for the secret using the user's credentials. Not even sure that's possible, but at best it seems messy.

[soltysh]

Based on the outcome from kubernetes/kubernetes#8178 I'm suggesting to go with the mixed approach. Where we can control objects and validation makes sense before doing actual build, because without it we cannot continue build we'll fail the build (I'm talking about non-existent ImageStream, for example). In all the other cases where we rely on external mechanism (missing secret) where the Pod will never fail, but emits events, we should make sure to emit exactly the same events in builds, additionally using them to set current status message for the build as well. @bparees @smarterclayton @mfojtik does this make sense?

[mfojtik]

+1
On May 20, 2015 5:40 PM, "Maciej Szulik" notifications@github.com wrote:

Based on the outcome from kubernetes/kubernetes#8178
kubernetes/kubernetes#8178 I'm
suggesting to go with the mixed approach. Where we can control objects and
validation makes sense before doing actual build, because without it we
cannot continue build we'll fail the build (I'm talking about non-existent
ImageStream, for example). In all the other cases where we rely on external
mechanism (missing secret) where the Pod will never fail, but emits events,
we should make sure to emit exactly the same events in builds, additionally
using them to set current status message for the build as well. @bparees
https://github.com/bparees @smarterclayton
https://github.com/smarterclayton @mfojtik https://github.com/mfojtik
does this make sense?

—
Reply to this email directly or view it on GitHub
#2269 (comment).