-
Notifications
You must be signed in to change notification settings - Fork 111
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
Stage sources via k/release API (krel) #61
Stage sources via k/release API (krel) #61
Conversation
b80647e
to
780502a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Really love that we are moving away from the push-build script!
FYI if you want to test this locally this flow gets triggered by
kubetest2 gke --build --strategy=make --stage=gs://your-bucket
b8bfb4c
to
f0810f5
Compare
f0810f5
to
936f2d1
Compare
This is ready for review. PTAL Unfortunately I was not able to test it locally:
|
|
0092e16
to
c6cb39e
Compare
c6cb39e
to
e112dcc
Compare
Thanks! I think the path for the bazel release tars was wrong there: Lines 82 to 86 in 237f174
Fixed it in this PR. Now it seems to work:
Artifacts are located there: https://console.cloud.google.com/storage/browser/kubernetes-release-gcb/ci?pageState=(%22StorageObjectListTable%22:(%22f%22:%22%255B%255D%22))&prefix=&forceOnObjectsSortingFiltering=false Depends on some fixes to be merged in https://github.com/kubernetes/release/pull/1629/files |
d01e87b
to
a756981
Compare
a756981
to
b8173bb
Compare
Alrigtht, gave it another try with the changed build type:
The build results can be found here: https://console.cloud.google.com/storage/browser/kubernetes-release-gcb/ci?pageState=(%22StorageObjectListTable%22:(%22f%22:%22%255B%255D%22))&prefix=&forceOnObjectsSortingFiltering=false Another small fix for noupdatelatest is in flight there: kubernetes/release#1638 |
b8173bb
to
68edbee
Compare
68edbee
to
0fc5c0c
Compare
0fc5c0c
to
83e9037
Compare
PTAL @amwat |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's give this a shot!
nothing is currently using the make strategy. But I'll follow up this with changes to the gce build to start using this.
/lgtm
/approve
/hold
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: amwat, saschagrunert 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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@saschagrunert -- Once kubernetes/release#1731 merges, can you pull it in here?
The logic for handling GCS suffixes gets fixed in that one.
/hold
83e9037
to
8a1a63c
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@saschagrunert -- Once kubernetes/release#1731 merges, can you pull it in here?
The logic for handling GCS suffixes gets fixed in that one.
/hold
Done 👍
/lgtm |
We now stage the sources directly via the API provided by k/release rather than the deprecated push-build.sh script (lives in k/release, too). Signed-off-by: Sascha Grunert <[email protected]>
8a1a63c
to
5ae8312
Compare
Had to rebase once again to include the latest krel fixes |
/lgtm |
We now stage the sources directly via the API provided by k/release rather than the deprecated push-build.sh script (lives in k/release, too).
Side note: The API has not been e2e tested yet. I hope that we can do this with an additional job in test-infra which uses kubetest2 to periodically stage a built release.