Automatically trigger the first BuildRun about build

[sbose78]

Same annotation.

1. If I create a Build with no annotation, then the controller creates the first BuildRun, and sets an annotation pending=false ( default behaviour )

2. If I create a Build with an annotation pending=false, then do nothing.

3. If I create a Build with an annotation pending=true, do same as (1)

What do you think?

[sbose78]

Sounds good, @otaviof .

How would the controller, in subsequent reconciliations understand that the first build has already been triggered? Would that be by looking at status information?

[qu1queee]

@otaviof hi, for:

I would like to add that when creating a BuildRun returns errors, what would the operator do?

If the creation of the BuildRun happens in the Build reconciler, I think it should be manage as yet another validation in the Build reconciliation. If a validation fails in the Build, we reconcile again and again and again ... :)

[zhangtbj]

Hi @sbose78 ,

Good idea, some quick questions about the feature:

On creation of a Build, automatically trigger the creation of a BuildRun.

I am thinking of the name for this new created BuildRun? Should we hardcode a name such as the same name as the Build?
I prefer to use the GenerateName to create this new BuildRun and also modify to use GenreateName in our samples so that the new created BuildRun won't report error like duplicated and the BuildRun is already exist

Use annotation to disable this behavior.

Like the annotation name build.build.dev/newBuildRun = true or false ?

Have the controller add/update an annotation to record the creation of a BuildRun.

I didn't understand this one very well. Do you mean also have another annotation to record the new created BuildRun for this Build? If yes, why we don't set it in the Build Status? Update an annotation will reconcile the build controller again.

One more requirement here:
We also need to set OwnerRefernce for the new created BuildRun

[sbose78]

I am thinking of the name for this new created BuildRun? Should we hardcode a name such as the same name as the Build?
I prefer to use the GenerateName to create this new BuildRun and also modify to use GenreateName in our samples so that the new created BuildRun won't report error like duplicated and the BuildRun is already exist

Sounds good!

Like the annotation name build.build.dev/newBuildRun = true or false ?

Yeah or something like buildun.build.dev/pending=true. If pending, create a BuildRun and update this to false. Else, do nothing about it.

I didn't understand this one very well. Do you mean also have another annotation to record the new created BuildRun for this Build? If yes, why we don't set it in the Build Status?

I wanted to avoid recording this user experience enhancement information in the API spec or status :) Not too picky, but the BuildRun may not necessarily be of much value to the Build for us to record it in the status.

Update an annotation will reconcile the build controller again.

The meta generation doesn't change if spec isn't updated. That helps us filter out events generated from annotations.

	pred := predicate.Funcs{
		UpdateFunc: func(e event.UpdateEvent) bool {
		         // Ignore updates to CR status in which case metadata.Generation does not change
                          // If annotations are updated, then "old generation" = "new generation" 
		DeleteFunc: func(e event.DeleteEvent) bool {
			// Evaluates to false if the object has been confirmed deleted.
			return !e.DeleteStateUnknown
		},
	}

	// Watch for changes to primary resource Build
	err = c.Watch(&source.Kind{Type: &buildv1alpha1.Build{}}, &handler.EnqueueRequestForObject{}, pred)
	

[zhangtbj]

em... sounds good.

Just would like to confirm:

Are item 2 (Use an annotation to disable this behaviour.) and item 3 (Have the controller add/update an annotation to record the creation of a BuildRun.) talking about the same annotationbuild.build.dev/newBuildRun?

Or the item 3 is a total different enhancement thant item 2?

[zhangtbj]

Yes, agree.

I am thinking, if we want to support re-run function by using it.

For example, if the end user want to re-config the build and re-run a new buildRun.

I think we can set the pending=true by edit or by cli/console and the build controller can help create a new buildRun.

How about this one above ^^?

[otaviof]

Overall, I really like the idea, good way to improve the user experience! 👍

What I would like to suggest, is to review the need for changing the annotation (buildrun.build.dev/pending) value between true and false, and the annotation name itself.

For this use-case, I think we are interested in the behavior of automatically creating a first BuildRun object when a build is created, and therefore, we then should name the annotation like the following snippet:

[...]
annotations:
  buildrun.build.dev/auto-create-buildrun: true
  # or set to false, in order to disable this behavior
  # buildrun.build.dev/auto-create-buildrun: false

During reconciliation of Build resource, we can issue the creation of the initial BuildRun object, and this interaction with the Kubernetes API will immediately respond if the creation succeeded. Therefore, Build status can be updated accordingly:

[...]
status:
  [...]
  status: InitialBuildRunCreated
  message: "BuildRun X has been created on namespace Y"

To inform users, we can introduce new status.status entries, like InitialBuildRunCreated and InitialBuildRunFailed. And, add a status.message attribute to show where to find that new object, or why the creation didn't succeed.

[otaviof]

Sounds good, @otaviof .

How would the controller, in subsequent reconciliations understand that the first build has already been triggered? Would that be by looking at status information?

Indeed, making sure the creation of BuildRun happens when Build is created, needs consideration here. Good point!

To make sure this feature is working as expected, when creating BuildRun object we will make sure the errors are captured, and if any errors, the reconciliation of the Build object will also be re-triggered. And following this approach, the operator effectively creates the BuildRun when build is created.

A corner case can happen when BuildRun creation returns errors on every attempt. And in this situation the user can edit the annotation buildrun.build.dev/auto-create-buildrun to false, stopping the attempt to create BuildRun object.

[otaviof]

To add, another interesting side effect of buildrun.build.dev/auto-create-buildrun is, the users can generate a BuildRun whenever they are ready, which could happen on the creation of Build or maybe later on.

[zhangtbj]

Hi all,

two quick question,

1. Is InitialBuildRunXXX good for us just keep the first run status?

status:
  [...]
  status: InitialBuildRunCreated
  message: "BuildRun X has been created on namespace Y"

How about extend to show the last buildRun status, and the InitialBuildRun is the first and special case of this new status, so that we can use it wider?
And use it as a standard status api: corev1alpha1.Status json:",inline", so that the result shows as:

status:
  Conditions or status
    Last Transition Time: 2019-11-01T04:22:41Z
    Status:                True
    Type:                  Ready
    Last Transition Time: 2019-11-01T04:22:41Z
    Status:                True
    Type:                  BuilderReady
    Last Transition Time: 2019-11-01T04:22:41Z
    Status:                True
    Type:                  LastBuildReady
....

To make sure this feature is working as expected, when creating BuildRun object we will make sure the errors are captured, and if any errors, the reconciliation of the Build object will also be re-triggered. And following this approach, the operator effectively creates the BuildRun when build is created.

Do we need to reconcile the Build if the first BuildRun fail?
I think buildrun controller should reconcile and handle it.

• build controller just need to show the fail result.
• The end user can click the button or from cli to trigger a new buildrun
• then the LastBuildRun status can be updated.

Think about if the initialBuildRun fail, then end user find there is something wrong in the config (wrong git or crds), then modify the build setting, then trigger a new buildRun with correct settings. Do they also care about the result/status of the initialBuildRun?

[qu1queee]

My 2 cents on this, the flow of generating a BuildRun automatically should only rely on the BuildRun controller, by reconciling if a Build was created with X label(buildrun.build.dev/auto-create-buildrun and with true) and by checking if the Build instance have an Status.Reason ==Succeeded(by using Predicates this can be achieved, similar to what @sbose78 mentioned above) . In this case, we can delegate the whole BuildRun management to its controller and avoid adding "noise" to the Build status and other Reconciliations in Build from a BuildRun.

I would like to be pragmatic and open a PR with a very basic setup:

1. Achieve what I mentioned above, where BuildRun generates a new BuildRun under certain Build circumstances
2. If buildrun.build.dev/auto-create-buildrun: true annotations exists, 1. takes place, and the set the same annotation to false
3. If buildrun.build.dev/auto-create-buildrun: false, nothing happen.

@otaviof @zhangtbj opinions?

[otaviof]

My 2 cents on this, the flow of generating a BuildRun automatically should only rely on the BuildRun controller, by reconciling if a Build was created with X label(buildrun.build.dev/auto-create-buildrun and with true) and by checking if the Build instance have an Status.Reason ==Succeeded(by using Predicates this can be achieved, similar to what @sbose78 mentioned above) . In this case, we can delegate the whole BuildRun management to its controller and avoid adding "noise" to the Build status and other Reconciliations in Build from a BuildRun.

Indeed, that's a good plan to have a clean cut. Agreed.

I would like to be pragmatic and open a PR with a very basic setup:

1. Achieve what I mentioned above, where `BuildRun` generates a new BuildRun under certain Build circumstances
2. If `buildrun.build.dev/auto-create-buildrun: true` annotations exists, 1. takes place, and the set the same annotation to `false`
3. If `buildrun.build.dev/auto-create-buildrun: false`, nothing happen.

I like this approach, that's a good way to start. I would like to add that when creating a BuildRun returns errors, what would the operator do?

[zhangtbj]

Hi @qu1queee ,

Can I understand it more clearly, about the new annotation buildrun.build.dev/auto-create-buildrun :

• Is it just for the first build? If create a new build, auto create a new buildrun
• If there is any update in build, will we also trigger a new buildrun?
• If it is just a build config change, such as increase the resourceLimit, do we need to trigger a new buildrun?
• Do we still need a status in Build to show the first or last buildrun status?

[qu1queee]

@zhangtbj is only related to your first point -> Is it just for the first build? If create a new build, auto create a new buildrun

Do we still need a status in Build to show the first or last buildrun status?

which Status do you mean?

[zhangtbj]

The status discussion is here:
#82 (comment)

Something like that:

[...]
status:
  [...]
  status: InitialBuildRunCreated
  message: "BuildRun X has been created on namespace Y"

[qu1queee]

yes, we need this, so it should remain there for now.