[WIP] resolve buildconfig output references when constructing the build

[bparees]

fixes #4518

[bparees]

Needs test case updates still.

@smarterclayton need to know how you feel about this.. with this change you'll get an immediate error if your buildconfig's outputstream cannot be resolved, rather than us creating a build and then retrying the imagestream resolution for 30 minutes before failing the build (if you don't create the imagestream in time).

I know this sort of goes against the "eventually consistent" model, but it also gives much more immediate feedback in what i suspect is the much more typical case of "oh, i forgot to create my output target".

the risk might be the race condition cases where you're creating the BuildConfig and the ImageStream at the same time, and then the BuildConfig is going to immediately create a Build due to a config change trigger. In that scenario if the ImageStream is being created last, the Build creation could fail because it couldn't resolve the imagestream.

Even assuming that is a concern, i'd like to think about whether there's another approach we could take to solve that(such as delaying the creation of the first build only, by a couple seconds), because i think this behavior is much more user friendly, and also simpler to reason about from a code perspective.

[bparees]

sample output:

$ oc start-build ruby-sample-build
Error from server: the referenced output image stream test/origin-rby-sample does not exist

[bparees]

[test][extended:core]

[smarterclayton]

I know this sort of goes against the "eventually consistent" model, but it also gives much more immediate feedback in what i suspect is the much more typical case of "oh, i forgot to create my output target".

the risk might be the race condition cases where you're creating the BuildConfig and the ImageStream at the same time, and then the BuildConfig is going to immediately create a Build due to a config change trigger. In that scenario if the ImageStream is being created last, the Build creation could fail because it couldn't resolve the imagestream.

That's a significant problem. We do not require ordering in our create
flows, so this would break that.

Even assuming that is a concern, i'd like to think about whether there's another approach we could take to solve that(such as delaying the creation of the first build only, by a couple seconds), because i think this behavior is much more user friendly, and also simpler to reason about from a code perspective.

Technically we support or we're going to support "create image stream repo on push" (although I haven't tried it in a while @ncdc) which means we might be fixing the wrong problem. I'd rather be able to create IStags completely on the fly, than get an error here.

[bparees]

Needs test case updates still.

@smarterclayton i was looking for feedback from you on whether you consider this to be acceptable behavior before i invested the effort in creating the TCs.

[bparees]

@smarterclayton so is this acceptable behavior? (fail immediately instead of retrying for 30mins)? keeping in mind the race condition caveat that isn't yet solved.

[openshift-bot]

Evaluated for origin test up to b4e7703

[bparees]

I'd rather be able to create IStags completely on the fly, than get an error here.

i'd be ok with doing that, if that's the approach we want to take.

[openshift-bot]

continuous-integration/openshift-jenkins/test FAILURE (https://ci.openshift.redhat.com/jenkins/job/test_pull_requests_origin/7169/) (Extended Tests: core)

[openshift-bot]

Origin Action Required: Pull request cannot be automatically merged, please rebase your branch from latest HEAD and push again