Allow suppressing of build trigger in new-app

[marekjelen]

When creating new deployment using oc new-app it should be possible not to trigger the build automatically.

Version

oc v1.5.1+7b451fc
kubernetes v1.5.2+43a9be4
features: Basic-Auth

openshift v1.5.1+7b451fc
kubernetes v1.5.2+43a9be4

Steps To Reproduce

1. oc new-app --strategy=docker app/

Current Result

Build is automatically triggered as part of the oc command.

Expected Result

There should be a switch to postpone the first build to be trigger by manual intervention.

[php-coder]

@openshift/devex

[csrwng]

This is in our Trello backlog
https://trello.com/c/3BxvvENz/1200-3-allow-new-app-to-create-things-in-an-inert-state-appcreate

[mhausenblas]

@csrwng what if I'd be willing to send in a PR for this? Would that change the status? In my experience this is a real issue and we would make people's life easier with it.

[csrwng]

👍 @mhausenblas PRs are always welcome :)

[mhausenblas]

How about naming it --suppress-trigger or --skip-config-trigger or --dontbuild?

[csrwng]

@mhausenblas ideally it would work not just for buildconfigs but also for deploymentconfigs so that they don't deploy automatically, so it shouldn't have 'build' or 'config-trigger' in the name. So either --inert or --suppress-triggers seems fine to me.

@bparees thoughts?

[bparees]

@csrwng as much as i'm personally biased towards my --inert suggestion, --suppress-triggers is probably more obvious as to what it does, let's go w/ that. We can always send the PR through cli-review.

[GrahamDumpleton]

I can see a bit of ambiguity here around use of --suppress-triggers.

The intent as I understand it is that want to create the build configuration and deployment configuration, but simply don't run the initial build. The initial build would have to be started using oc start-build, or perhaps as web hook trigger from a git push against a repo. After that, everything would then run as normal, as image and config change triggers would still be in place.

The use of --suppress-triggers could though be interpreted as meaning don't even add triggers to the build and deployment configuration, thus requiring oc set triggers later on to add them back.

I would suggest a better name is needed for the option which makes it clear you are deferring the running of the initial build only. It should also be an option name which can also be added to oc new-build for the non binary build case as well, as it would be useful to have to there as well.

So I would suggest --defer-build or --defer-initial-build, and whatever the option, it be added to both oc new-app and oc new-build, with it only having meaning in the latter for a non binary build.

[bparees]

@GrahamDumpleton what you're asking for isn't possible. If you have an imagechangetrigger, it is going to fire when then the buildconfig is created because we're doing to see that we haven't yet run a build for the current level of the imagestream in question.

There is no option for "run a build whenever the imagestream is newer than the most recent build, but only if we've run at least one build" and I'm not in favor of adding such logic just to serve this use case.

So suppress-triggers really does describe what we'd implement here, which is that we'd create buildconfigs that do not have imagechange and configchangetriggers.

Now, that does leave open questions for:

1. should it also suppress webhook triggers (which we also add automatically): probably not, in which case suppress-triggers is not a sufficiently specific name since it is not suppressing all triggers (not even all triggers on buildconfigs)

2. should it create the deploymentconfig with the imagechangetrigger set to "automatic=false" which gives you equivalent behavior to creating a buildconfig w/ no imagechangetrigger (again it won't trigger the deployment automatically, ever). Debatable but i'm certainly amenable to keeping this feature scoped to only affect buildconfigs, not deployments.

[csrwng]

should it create the deploymentconfig with the imagechangetrigger set to "automatic=false" which gives you equivalent behavior to creating a buildconfig w/ no imagechangetrigger

For new-app we have the case where you are only creating a new deployment from an existing image. In that case, I think it would be useful to suppress the deployment config trigger as well so you have a chance to add secrets, change env vars, etc.

[bparees]

For new-app we have the case where you are only creating a new deployment from an existing image. In that case, I think it would be useful to suppress the deployment config trigger as well so you have a chance to add secrets, change env vars, etc.

I considered that and don't disagree, but mostly likely people will be in the model of "i don't want the buildconfig to run, but once it does, i do want my DC to be triggered"

so if we're going to allow suppressing the DC triggers also, i think that has to be separate/explicilit. ie "supress-bc-triggers" and "suppress-dc-triggers" as separate flags. (sigh).

[marekjelen]

@bparees agree, this request came from real world problem ... @mhausenblas and me were using OpenShift to build project --from-dir ... however, as new-app does a lot of magic and there was .git in the directory, new-app was setting the source to the Github repo, which did not have the latest source code, as we were doing local testing. As the first build is always triggered, it gets the source from Github and fails. We agreed that asking to change the behaviour of new-app is probably impossible, we came to a workaround that being able to disable the first automatic build solves our problem and as we discussed it in the office, it was clear that such a feature would make sense for other use-cases as well.

So, back to your comment, keeping DC's normal behaviour is what we would like to have, we just want to be able to skip the first initial build.

[GrahamDumpleton]

@bparees I well suspected that it couldn't be done such that once first build was then manually triggered everything ran as normal. The point of me going to the extent of adding my description was to show to @mhausenblas that I felt the direction things were headed wasn't what they thought they perhaps really wanted as he was describing it to me before I posted my message.

Anyway, if the desire is not to use the remote repo, but what was in the current directory, the option exists to use a binary build.

oc new-build --binary --image-stream python --name blog
oc start-build blog --from-dir=.

This doesn't result in a deployment configuration though and it can only be created once first build has completed:

oc new-app --image-stream blog

When you use oc new-app, it has some short cut where it is able to work out the deployment config even though a first build hasn't run, by looking at the builder image to work out ports exposed etc.

So if you end up having a way to not set triggers on the build configuration only when using oc new-app, so that the first build doesn't run, you would still need to go back and use oc set triggers to add them when you want to revert back to everything being automatic.

oc new-app --image-stream python:latest --code https://github.com/openshift-katacoda/blog-django-py --name blog --suppress-bc-triggers
oc start-build blog
oc set triggers bc/blog --from-config
oc set triggers bc/blog --from-image=python:latest

I am left wondering whether the binary build shouldn't have been used in the first place if they were attempting to test with local code only.

I would have reservations though of having an option on oc new-app to suppress build triggers as it already does too much. Various things are already only possible with oc new-build when needing to do non typical things around a build so maybe an ability to defer the build should only be available by not adding the triggers when using oc new-build. In fact the --binary option already gets close to what is wanted in that triggers aren't added.

In some respects the --binary option could have been named differently to better note that it doesn't add build triggers, and then also allow the following to work where it doesn't now.

$ oc new-build --binary --code https://github.com/openshift-katacoda/blog-django-py --image-stream python --name blog
error: specifying binary builds and source repositories at the same time is not allowed

Where the --binary option was instead named differently to mean manual builds required.

Does --binary option do anything else besides disable adding of triggers and complaining if code source is supplied?

[bparees]

Does --binary option do anything else besides disable adding of triggers and complaining if code source is supplied?

technically it creates a buildconfig that explicitly indicates it expects binary inputs (which helps provide nice error messages if you try to start the build by hand w/o providing a binary input), but in hindsight the "binary" model for buildconfigs is maybe not the greatest design, since any buildconfig can be run as a binary build, even if it doesn't explicitly declare itself to be a binary build.

and there's no particular reason that new-app could not also have a --binary option, at least i can't think of one, i think we only added it to new-build because again we were trying to limit the explosion of new-app flags and --binary was more closely associated with new-build.

[marekjelen]

This is the use-case https://github.com/mhausenblas/reshifter/blob/master/Makefile

oc new-app --strategy=docker --name='$(app_name)' --context-dir='./app/' .
oc set env bc/$(app_name) rversion=$(reshifter_version)
oc start-build $(app_name) --from-dir .

a) it does not need to use remote git repo, but as . is cloned from Github it does set the source to the github repo automagically
b) the variable needs to be setup before the build can be started without failing

[GrahamDumpleton]

In that case you would use oc new-build. The issue is whether --context-dir works for binary build. Likely need to do:

oc new-build --binary --strategy=docker --name='$(app_name)'
oc set env bc/$(app_name) rversion=$(reshifter_version)
oc start-build $(app_name) --from-dir ./app/

Or if it copies up the files into an app subdirectory, just cd to the subdirectory before running the oc start-build command and use --from-dir=..

As noted above, you afterwards still have to do:

oc new-app $(app_name)

when build finished.

[jorgemoralespou]

I would also consider interesting to be able to deploy skip application deployment, so additional configuration can be added to the deploymentConfig, like probes before doing a deployment.

[mjudeikis]

I think we covered all possible use-cases and even more :D I liked the idea of suppress-bc-triggers and suppress-dc-triggers. But does this mean we 'exploding' new-app flags even more?

Could we do something like --suppress-triggers - default all, but we can do --suppress-triggers=DeploymentConfig,DC,BC,BuildConfig,SomeOtherRandomObject

And if I understood consensus is if flag is provided we disable ImageChange, and ConfigChange triggers. But we can give hits how to re-enable them so "copy-paste" could be done easily.