Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Elide creating Build when the Build portion of the Configuration doesn't change. #439

Closed
mattmoor opened this issue Mar 20, 2018 · 6 comments
Assignees
Labels
area/API API objects and controllers area/build Build topics specifically related to Knative kind/feature Well-understood/specified features, ready for coding.
Milestone

Comments

@mattmoor
Copy link
Member

This TODO.

@mattmoor mattmoor added kind/feature Well-understood/specified features, ready for coding. area/API API objects and controllers area/build Build topics specifically related to Knative labels Mar 20, 2018
@mattmoor mattmoor added this to the M4 milestone Mar 20, 2018
@tcnghia tcnghia self-assigned this Apr 6, 2018
@mattmoor
Copy link
Member Author

cc @imjasonh

@mattmoor mattmoor modified the milestones: M4, M5 Apr 29, 2018
@mattmoor mattmoor modified the milestones: M5, M6 May 24, 2018
@mattmoor
Copy link
Member Author

Just found out that riff is currently relying on this, so we should be sensitive to not breaking them.

cc @trisberg

@trisberg
Copy link
Member

trisberg commented Sep 14, 2018

If we could add the annotation to the build part instead of the revisionTemplate then that would work fine for our purposes

@mattmoor mattmoor removed this from the M6 milestone Oct 31, 2018
@mattmoor
Copy link
Member Author

@trisberg With decoupled build in 0.2 you can start doing this.

Before:

build:
  <spec>

After:

build:
  apiVersion: build.knative.dev/v1alpha1
  kind: Build
  metadata:
    annotations:
      # Since we will treat the entire "build" as opaque, any change should
      # trigger a new build, so this would serve a similar purpose to the nonce
      # @seyfer has proposed for forcing redeployments via the CLI.
      whatever/you: want
  spec:
    <spec>

@mattmoor
Copy link
Member Author

(Separate comment for the original issue)

We should be able to achieve build elision by changing how we Create / Get builds for Revision to rely on a selector in place of the deterministic naming we have today.

As a strawman, we could serialize[1] the build and take its hash as hex, H then augment the build with:

metadata:
  labels:
    serving.knative.dev/buildVersion: H

We should then be able to efficiently query for an existing Build, and if not create it.

If multiple Builds exist with this label selector (user error), then the selection is undefined (we should just pick the first).

[1] - We actually already have this here, so we don't even have to serialize the build. We could consider adding a method to this helper to produce the hash.

mattmoor added a commit to mattmoor/serving that referenced this issue Nov 4, 2018
We want to be able to elide build creation when that portion of the `ConfigurationSpec` remains unchanged.  To support those, we need to remove assumptions that `MakeRevision` can directly synthesize an `ObjectReference` for the `Build` directly from the `Configuration`.

The idea is that in a subsequent change, we will start to label builds using the schema outlined in knative#439 and potentially pass in an `ObjectReference` we have looked up instead.

Progress towards: knative#439
mattmoor added a commit to mattmoor/serving that referenced this issue Nov 4, 2018
mattmoor added a commit to mattmoor/serving that referenced this issue Nov 4, 2018
mattmoor added a commit to mattmoor/serving that referenced this issue Nov 4, 2018
Things like EnvVar updates should not trigger a fresh build, when a Build is specified in the ConfigurationSpec.  This builds on prior changes to lookup a pre-existing build with the same spec before creating its own.  This modifies the Build e2e tests to each perform an env-var only update to the configuration, and check that there are no differences in the resulting BuildRef.

Fixes: knative#439
mattmoor added a commit to mattmoor/serving that referenced this issue Nov 5, 2018
mattmoor added a commit to mattmoor/serving that referenced this issue Nov 5, 2018
Things like EnvVar updates should not trigger a fresh build, when a Build is specified in the ConfigurationSpec.  This builds on prior changes to lookup a pre-existing build with the same spec before creating its own.  This modifies the Build e2e tests to each perform an env-var only update to the configuration, and check that there are no differences in the resulting BuildRef.

Fixes: knative#439
mattmoor added a commit to mattmoor/serving that referenced this issue Nov 5, 2018
Things like EnvVar updates should not trigger a fresh build, when a Build is specified in the ConfigurationSpec.  This builds on prior changes to lookup a pre-existing build with the same spec before creating its own.  This modifies the Build e2e tests to each perform an env-var only update to the configuration, and check that there are no differences in the resulting BuildRef.

Fixes: knative#439
knative-prow-robot pushed a commit that referenced this issue Nov 5, 2018
We want to be able to elide build creation when that portion of the `ConfigurationSpec` remains unchanged.  To support those, we need to remove assumptions that `MakeRevision` can directly synthesize an `ObjectReference` for the `Build` directly from the `Configuration`.

The idea is that in a subsequent change, we will start to label builds using the schema outlined in #439 and potentially pass in an `ObjectReference` we have looked up instead.

Progress towards: #439
mattmoor added a commit to mattmoor/serving that referenced this issue Nov 5, 2018
mattmoor added a commit to mattmoor/serving that referenced this issue Nov 5, 2018
Things like EnvVar updates should not trigger a fresh build, when a Build is specified in the ConfigurationSpec.  This builds on prior changes to lookup a pre-existing build with the same spec before creating its own.  This modifies the Build e2e tests to each perform an env-var only update to the configuration, and check that there are no differences in the resulting BuildRef.

Fixes: knative#439
mattmoor added a commit to mattmoor/serving that referenced this issue Nov 5, 2018
mattmoor added a commit to mattmoor/serving that referenced this issue Nov 5, 2018
Things like EnvVar updates should not trigger a fresh build, when a Build is specified in the ConfigurationSpec.  This builds on prior changes to lookup a pre-existing build with the same spec before creating its own.  This modifies the Build e2e tests to each perform an env-var only update to the configuration, and check that there are no differences in the resulting BuildRef.

Fixes: knative#439
@mattmoor
Copy link
Member Author

mattmoor commented Nov 5, 2018

/assign

mattmoor added a commit to mattmoor/serving that referenced this issue Nov 6, 2018
Things like EnvVar updates should not trigger a fresh build, when a Build is specified in the ConfigurationSpec.  This builds on prior changes to lookup a pre-existing build with the same spec before creating its own.  This modifies the Build e2e tests to each perform an env-var only update to the configuration, and check that there are no differences in the resulting BuildRef.

Fixes: knative#439
mattmoor added a commit to mattmoor/serving that referenced this issue Nov 6, 2018
Things like EnvVar updates should not trigger a fresh build, when a Build is specified in the ConfigurationSpec.  This builds on prior changes to lookup a pre-existing build with the same spec before creating its own.  This modifies the Build e2e tests to each perform an env-var only update to the configuration, and check that there are no differences in the resulting BuildRef.

Fixes: knative#439
knative-prow-robot pushed a commit that referenced this issue Nov 6, 2018
Things like EnvVar updates should not trigger a fresh build, when a Build is specified in the ConfigurationSpec.  This builds on prior changes to lookup a pre-existing build with the same spec before creating its own.  This modifies the Build e2e tests to each perform an env-var only update to the configuration, and check that there are no differences in the resulting BuildRef.

Fixes: #439
@mattmoor mattmoor modified the milestone: Serving 0.3 Nov 20, 2018
nak3 added a commit to nak3/serving that referenced this issue Apr 4, 2020
* Update catalog source

* Update channel

* Update to 0.13.1 images

* Update configmap patch
skonto pushed a commit to skonto/serving that referenced this issue Sep 7, 2023
…#439)

Co-authored-by: Kenjiro Nakayama <nakayamakenjiro@gmail.com>
mgencur pushed a commit to mgencur/serving-1 that referenced this issue Nov 16, 2023
…#439)

Co-authored-by: Kenjiro Nakayama <nakayamakenjiro@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/API API objects and controllers area/build Build topics specifically related to Knative kind/feature Well-understood/specified features, ready for coding.
Projects
None yet
Development

No branches or pull requests

3 participants