-
Notifications
You must be signed in to change notification settings - Fork 165
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
kola: add gce support for --find-parent-image #1468
Conversation
(Didn't actually test this against GCP; need to dust off credentials -- or @dustymabe are you able to sanity check it?) |
LGTM. The CI failures seem to be a flake. |
@@ -568,8 +568,13 @@ func syncFindParentImageOptions() error { | |||
if err != nil { | |||
return err | |||
} | |||
case "gce": |
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.
should this be gcp
?
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.
I think kola only knows gce
right now. We should probably deprecate gce
and move it to gcp
.
ok I rebased this on top of #1335 over in dustymabe@aebe946 was going to try to run a test today but I think we have some chicken/egg problems here. Will discuss with @jlebon tomorrow more about how to properly test this. |
#1335 merged so we should be able to rebase this now |
OK, updated this now and added a fallback for |
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.
The CI bot is asking for my review.
LGTM, and based on the conversation this seems ready.
I don't see a new upload from you (maybe my browswer window is acting up)? |
We want to run upgrade tests in the pipeline now.
6ba4f94
to
beccce3
Compare
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: darkmuggle, jlebon 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 |
Yeah, GitHub is acting up. I just restamped the commit and re-pushed! |
it works!
|
CI failed with:
|
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 sanity |
@dustymabe: The
Use 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. |
/test sanity |
tests pass for me when I run them locally:
|
Yeah, let's just do |
@cgwalters: Overrode contexts on behalf of cgwalters: continuous-integration/jenkins/pr-merge 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. |
We want to run upgrade tests in the pipeline now.