Start to label builds with a hash of the spec from which they were created.#2401
Conversation
knative-prow-robot
added
do-not-merge/work-in-progress
knative-prow-robot
left a comment
knative-prow-robot
left a comment
There was a problem hiding this comment.
@mattmoor: 1 warning.
Details
In response to this:
Progress towards: #439
WIP until #2400 merges.
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.
| ) | ||
|
|
||
| func MakeRevision(config *v1alpha1.Configuration) *v1alpha1.Revision { | ||
| func MakeRevision(config *v1alpha1.Configuration, buildRef *corev1.ObjectReference) *v1alpha1.Revision { |
There was a problem hiding this comment.
Golint comments: exported function MakeRevision should have comment or be unexported. More info.
knative-prow-robot
added
the
approved
|
ugh, the full sha256 won't work because label values can only be 63 characters... |
mattmoor
force-pushed
the
label-builds
branch
from
3b932eb to
549ae3d
Compare
|
|
||
| // Compute the hash of the current build's spec. | ||
| hasher := sha256.New() | ||
| hasher.Write(config.Spec.Build.Raw) |
There was a problem hiding this comment.
Should we normalize this so that changes in whitespace don't trigger a new build.
There was a problem hiding this comment.
I'm inclined to leave it as-is. I'd wager the Kubernetes API Server already normalizes things and the raw bytes should come from there.
| // Compute the hash of the current build's spec. | ||
| hasher := sha256.New() | ||
| hasher.Write(config.Spec.Build.Raw) | ||
| h := hex.EncodeToString(hasher.Sum(nil)) |
There was a problem hiding this comment.
There's a shortcut available sha256.Sum256
mattmoor
force-pushed
the
label-builds
branch
2 times, most recently
from
4507de4 to
7cfee41
Compare
knative-prow-robot
added
size/S
mattmoor
changed the title
knative-prow-robot
removed
the
do-not-merge/work-in-progress
| "blockOwnerDeletion": true, | ||
| }}, | ||
| "labels": map[string]interface{}{ | ||
| serving.BuildHashLabelKey: "2ee4528bee48a78637ec374eb58cb1977b9611b85545f8b91884ff80b8d9472", |
There was a problem hiding this comment.
I'd have expanded the key to be the actual string here and below, but it doesn't look like we do that for any of the other labels. /shrug
| // generation of the Configuration that created this revision | ||
| ConfigurationGenerationLabelKey = GroupName + "/configurationGeneration" | ||
|
|
||
| // BuildHashLabelKey is the label key attached to Builds indicating the |
There was a problem hiding this comment.
style nit: other comments are in the form "attached to a Foo" instead of "attached to Foos".
…eated. Progress towards: knative#439
mattmoor
force-pushed
the
label-builds
branch
from
7cfee41 to
5aa5579
Compare
|
The following is the coverage report on pkg/.
|
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: jonjohnsonjr, mattmoor The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/test pull-knative-serving-integration-tests |
1 similar comment
|
/test pull-knative-serving-integration-tests |
5 participants
Progress towards: #439
WIP until #2400 merges.