Remove the BuildStrategyStatus from BuildStrategy struct? about build

[zhangtbj]

Yes, I totally agree. We should provide a trigger policy to allow user to configure.

When we using Tekton Triggers in our PoC, we can allow user to configure the trigger policy like below:

• No trigger (Only rebuild by config change)
• Trigger by Git commit
• Trigger by baseimage change
• Trigger by buildpack version change

But for this Trigger by buildstrategy change, I think we still need a good design for it.

Such as when buildstrategy is changed,

• should we create a new buildrun or just re-run the existing buildrun?
• Is there any different for the cluster scope and namespaced buildstrategy change?

I think we can have more discussion in future when we want to add it.

[otaviof]

@zhangtbj, @sbose78 I also like the idea. Making a extensibility point to trigger builds when an external resource changes is great in terms of usability. So a big +1 on this subject.

Thinking on use-cases, security inspection and re-triggering build of images upon CVEs, could also benefit from this new feature.

[zhangtbj]

@sbose78, any idea?

[sbose78]

@zhangtbj Good point. I was looking to have some kind of validation ( not just the openapi validation ) on the BuildStrategy whenever a new BuildStrategy is 'registered'. I haven't had a chance to define what that could look like. Is there anything you think we should have as a validation process ?

If not, I'm good with having it removed. We can always bring it back later on when we have something more concrete.

[zhangtbj]

Yes, I think that is only a reason to keep the status.

And I think that is also a reason to keep the buildstrategy-controller. :)

I think maybe there are some scenarios:

• validate if the grammer or yaml format is correct by using controller, and report the check result in the status
• keep all the builds which are built by this buildstrategy, and if the buildstratgy is updated, all build can be noticed or rebuilt

But I think they are low properties for Alpha or Beta. So I suggest we can remove it first, and in future, if there is any good scenario, we can bring it back directly. :)

[sbose78]

And I think that is also a reason to keep the buildstrategy-controller. :)

Here's what I have in mind with respect to the [cluster]buildstrategy-controller
#71

[sbose78]

keep all the builds which are built by this buildstrategy, and if the buildstratgy is updated, all build can be noticed or rebuilt

I think this is a very valid scenario. Especially for strategies which have images specified. Imagine a security vulnerability making us update the image specified in a build strategy. That should definitely re-trigger Builds.

Now, should we always trigger builds? I guess it should be a configurable by the user.

After we separate Build & BuildRun, we should also add an API field triggerOnChange.

apiVersion: build.dev/v1alpha1
kind: Build
metadata:
  name: example-build-buildpack 
spec:
  source:
    url: https://github.com/sclorg/nodejs-ex
    credentials:
      name: source-repository-credentials
  strategy:
    name: buildpacks-v3
    Kind: ClusterBuildStrategy <-- triggers should happen for both namespace & cluster-scoped stratgies.
    triggerOnChange: true <--- if strategy changes, a new BuildRun should be instantiated.

[qu1queee]

I think the general consensus here is that there might be room for the Status field in the strategy. Based on the nice feedback in here, I´m closing this issue because the goal of it seems to be accomplished. @zhangtbj please feel free to re-open if I did a mistake.