Make use of record.FinalSHA and overwrite it in started.json #11407
Conversation
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: krzyzacy The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
k8s-ci-robot
added
approved
k8s-ci-robot
added
size/M
| started := gcs.Started{ | ||
| Timestamp: time.Now().Unix(), | ||
| RepoVersion: downwardapi.GetRevisionFromSpec(spec), | ||
| } | ||
|
|
||
| // TODO(krzyzacy): we still need to resolve a ref into a sha | ||
| if mainRefSHA != "" { | ||
| started.RepoVersion = mainRefSHA |
There was a problem hiding this comment.
How is this different from downwardapi.GetRevisionFromSpec?
There was a problem hiding this comment.
So if I have a ref that's {"k8s.io/test-infra": "master"}, downwardapi.GetRevisionFromSpec will gives "master", which results in https://testgrid.k8s.io/sig-testing-maintenance#ci-janitor (see the big-O "master" in commit column header) and breaks testgrid search 😂
mainRefSHA is from a git rev-parse from cloneref which is the final SHA for a given ref, and we can have a valid display SHA in testgrid..
ref #10359
prow/initupload/run.go
Outdated
| started := specToStarted(spec) | ||
| uploadTargets := map[string]gcs.UploadFunc{} | ||
|
|
||
| failed := false |
There was a problem hiding this comment.
humm sure, is that recommended from go code style?
There was a problem hiding this comment.
https://github.com/kubernetes/test-infra/blob/master/prow/initupload/run.go#L117-L123
also that confused me a bit... 😂
There was a problem hiding this comment.
Joe told me that when you want it to default to the default value, use var foo int rather than foo := 0, whereas you'd do foo := 1
|
LGTM label has been added. Git tree hash: 3e41a98d06efa3ab37deda4ceeb5ad142ff31dc7
|
k8s-ci-robot
added
lgtm
krzyzacy
force-pushed
the
fill-in-jobversion
branch
from
a3364aa
to
e9bbdbb
Compare
k8s-ci-robot
added
lgtm
|
LGTM label has been added. Git tree hash: 17d60e83efb3844520535485c5a7c1a74d868bba
|
|
k8s-ci-robot added lgtm and removed lgtm labels 23 minutes ago - lol |
|
/hold cancel |
k8s-ci-robot
removed
the
do-not-merge/hold
Something like this?
We still need to teach our apps to understand RepoVersion 🤷♂️ but at least our metadata has all the info now.
/assign @cjwagner @stevekuznetsov @fejta