-
Notifications
You must be signed in to change notification settings - Fork 71
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
cmd/cip: Default to non-production runs with --confirm
#285
cmd/cip: Default to non-production runs with --confirm
#285
Conversation
c1fe326
to
6a5b04a
Compare
(Needed to flip the flag in the e2e test as well. Fixed in 6a5b04a.) |
??? grumbles |
Local run: $ time bazel run --workspace_status_command=$(pwd)/workspace_status.sh //cmd/cip:cip -- run --snapshot=us.gcr.io/k8s-cip-test-prod/golden-bar
INFO: Analyzed target //cmd/cip:cip (0 packages loaded, 0 targets configured).
INFO: Found 1 target...
Target //cmd/cip:cip up-to-date:
bazel-bin/cmd/cip/cip_/cip
INFO: Elapsed time: 2.650s, Critical Path: 2.49s
INFO: 13 processes: 13 linux-sandbox.
INFO: Build completed successfully, 14 total actions
INFO: Build completed successfully, 14 total actions
INFO Request {us.gcr.io/k8s-cip-test-prod/golden-bar true}: OK
INFO Request {us.gcr.io/k8s-cip-test-prod/golden-bar/bar false}: OK
- name: bar
dmap:
"sha256:610b1ef6fec876146dee2b2846c890b566d26f235d7ea8982056a3e84bd35929": ["1.0"]
real 0m4.111s
user 0m0.125s
sys 0m0.064s |
Oh hello, auditor flake? |
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.
/lgtm
/retest |
The CI issue seems reproducible to me. 🤔 |
6a5b04a
to
57dfb5d
Compare
Yeah that seems likely. FTR every deployed instance of CIP in recent memory (including the current version v2.4.1 that's seeing presubmit/postsubmit usage) has always had |
Right, we haven't changed the actual |
Just to capture some things from Slack: This line creates a new set of options: The options themselves: https://github.com/kubernetes-sigs/k8s-container-image-promoter/blob/97e351eb08d2af791f1a62ead66e5291398c4f0b/legacy/cli/root.go#L25-L28 If |
070ca3e
to
2f2c1ff
Compare
2f2c1ff
to
cad0de2
Compare
Signed-off-by: Stephen Augustus <[email protected]>
Signed-off-by: Stephen Augustus <[email protected]>
cad0de2
to
ce2f8bf
Compare
(rebasing for handoff) |
f16452e
to
a0e1a2c
Compare
Flag changed from `dry-run` to `confirm` so we flip the logic in MakeSyncContext(). Prior to this, test registries were not cleared as the delete invocation was running in dry run (or confirm = false). Signed-off-by: Adolfo García Veytia (Puerco) <[email protected]>
a0e1a2c
to
a87e90e
Compare
@puerco -- Nice work! 🎉 |
--confirm
Reviewing @justaugustus commits :) |
Symbolic LGTM for @puerco's fixes (even though the bot will yell at me): |
@justaugustus: you cannot LGTM your own PR. In response to this:
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. |
In response to this:
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. |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: 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 |
What type of PR is this?
/kind bug regression
/priority critical-urgent
What this PR does / why we need it:
test-e2e: Minor cleanup for cmp.Diff() output
cmd/cip: Use
--confirm
instead of--dry-run
Similar to our
--nomock
flags for other tools, we use a--confirm
flag for
cip
, since the default nil value for booleans isfalse
.Meaning: If
--dry-run
is not explicitly set totrue
, our toolingwill initiate an image promotion.
Flip confirm logic in cip/cip-auditor e2e MakeSyncContext() calls
/assign @hasheddan @xmudrii
cc: @kubernetes-sigs/release-engineering
Which issue(s) this PR fixes:
Special notes for your reviewer:
Does this PR introduce a user-facing change?