Skip to content

gcbmgr: Add flag/support for attended GCB build submissions #1052

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Jan 31, 2020
Merged

gcbmgr: Add flag/support for attended GCB build submissions #1052

merged 1 commit into from
Jan 31, 2020

Conversation

justaugustus
Copy link
Member

Here we add a flag (--attended) to gcbmgr which can turn off the --async
flag in the gcloud builds submit invocation, allowing GCB runs to
happen attended.

This will enable us to:

  • run gcbmgr within Prow
  • report the success/failure of the GCB run instead of the success of
    the GCB build submission

An example command run would be:

RELEASE_TOOL_REPO=https://github.com/justaugustus/release.git \
RELEASE_TOOL_BRANCH=gcbmgr-attended \
./gcbmgr stage master --attended

Required for kubernetes/test-infra#16074.

Signed-off-by: Stephen Augustus [email protected]

/assign @tpepper @hoegaarden @saschagrunert @cpanato

@justaugustus
gcbmgr: Add flag/support for attended GCB build submissions
dac803c 
Here we add a flag (--attended) to gcbmgr which can turn off the --async
flag in the gcloud builds submit invocation, allowing GCB runs to
happen attended.

This will enable us to:
- run gcbmgr within Prow
- report the success/failure of the GCB run instead of the success of
the GCB build submission

Signed-off-by: Stephen Augustus <[email protected]>
@k8s-ci-robot k8s-ci-robot added the do-not-merge/blocked-paths Indicates that a PR should not merge because it touches files in blocked paths. label Jan 30, 2020
@k8s-ci-robot
Copy link
Contributor

@justaugustus: Adding label: do-not-merge/blocked-paths because PR changes a protected file.

Reasons for blocking this PR:

[Changes to certain release tools can affect our ability to test, build, and release Kubernetes. This PR must be explicitly approved by SIG Release repo admins.]

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.

@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Jan 30, 2020
@k8s-ci-robot k8s-ci-robot added approved Indicates a PR has been approved by an approver from all required OWNERS files. area/release-eng Issues or PRs related to the Release Engineering subproject sig/release Categorizes an issue or PR as relevant to SIG Release. labels Jan 30, 2020
@justaugustus justaugustus mentioned this pull request Jan 30, 2020
Merged
hasheddan
hasheddan approved these changes Jan 30, 2020
View reviewed changes
Copy link
Contributor

@hasheddan hasheddan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm
/hold

@k8s-ci-robot k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jan 30, 2020
@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jan 30, 2020
saschagrunert
saschagrunert approved these changes Jan 31, 2020
View reviewed changes
Copy link
Member

@saschagrunert saschagrunert left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm
/hold cancel

@k8s-ci-robot k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jan 31, 2020
cpanato
cpanato approved these changes Jan 31, 2020
View reviewed changes
Copy link
Member

@cpanato cpanato left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: cpanato, hasheddan, justaugustus, 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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

hoegaarden
hoegaarden reviewed Jan 31, 2020
View reviewed changes
gcbmgr
@@ -404,6 +404,12 @@ GCP_USER_TAG=${GCP_USER_TAG/@/-at-}
# in the bucket name so convert . to -
GCP_USER_TAG=${GCP_USER_TAG/\./-}

if ((FLAGS_attended)); then
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ca we call the flag tail or follow? I think those names are more commonly used elsewhere and are clearer to me, a non-native speaker.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@hoegaarden -- Okay, will think of a better flag name! Let me get this in since the test-infra PR merged, so I can get an idea of the failures and then we'll rename in the next iteration.

@justaugustus justaugustus removed the do-not-merge/blocked-paths Indicates that a PR should not merge because it touches files in blocked paths. label Jan 31, 2020
@k8s-ci-robot k8s-ci-robot merged commit 459c7f2 into kubernetes:master Jan 31, 2020
@k8s-ci-robot k8s-ci-robot added this to the v1.18 milestone Jan 31, 2020
This was referenced Feb 11, 2020
Merged
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@saschagrunert saschagrunert saschagrunert approved these changes

@cpanato cpanato cpanato approved these changes

+2 more reviewers

@hoegaarden hoegaarden hoegaarden left review comments

@hasheddan hasheddan hasheddan approved these changes

Reviewers whose approvals may not affect merge requirements
Assignees

@saschagrunert saschagrunert

@hoegaarden hoegaarden

@tpepper tpepper

@cpanato cpanato

@hasheddan hasheddan

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. area/release-eng Issues or PRs related to the Release Engineering subproject cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. sig/release Categorizes an issue or PR as relevant to SIG Release. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files.
Projects
None yet
Milestone
v1.18
Development

Successfully merging this pull request may close these issues.
7 participants
@justaugustus @k8s-ci-robot @saschagrunert @hoegaarden @cpanato @hasheddan @tpepper