-
Notifications
You must be signed in to change notification settings - Fork 168
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
mantle/ore: gcloud: add promote-image command #1456
mantle/ore: gcloud: add promote-image command #1456
Conversation
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.
Only nits. This looks good to me 👍.
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.
One question otherwise LGTM
} | ||
|
||
// First undeprecate the image we want to promote | ||
deprecateImage(promoteImageName, gcloud.DeprecationStateActive, "") |
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.
Question: does the api.DeprecateImage
call no-op without an error if the image is already in the desired deprecation state?
If not we might need to build some annoying checks that skips an action if it's already in the correct state so that this command can be re-ran if it errors the first time through.
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.
my experience is that it just returns successfully it it's already in the desired state, though as @bgilbert mentions in #1456 (comment) we should try to minimize the number of API calls we do by only requesting to change the deprecation state for images that need it.
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.
ok updated the code so that it only calls the deprecate for images that need it
name, string(state)) | ||
pending, err := api.DeprecateImage(name, state, replacement) | ||
if err == nil { | ||
err = pending.Wait() |
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.
In principle we should collect up all the pendings and wait on them at the end. I don't know how much it matters here, though, since we're probably only deprecating one image.
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.
right this should be one API call IIUC
This can be used to convert various string representations of an image name into a full API endpoint for an image.
…eImage For the replacement in the images.deprecate API call we need the string representation of the replacement to be the full API endpoint for the image. Let's use the new getImageAPIEndpoint function to calculate the image API endpoint given a short name representation.
…mily This will allow us to just retrieve images in a given image family.
In our build/release workflow for GCP we build and upload/create a GCP image in the build pipeline. We deprecate the image (because we haven't decided to release it yet) and then add it to the corresponding image family for the stream it was built against. The promote-image command will be used in the release pipeline to undeprecate the image now that we have decided to release it. Since we only want one image visible in an image family at any given time, the promote-image command will then deprecate all other images in the specified image family.
c368714
to
dc920d6
Compare
just rebased and pushed a new update to address code review comments! |
/lgtm |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: ashcrow, bgilbert, dustymabe 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 |
In our build/release workflow for GCP we build and upload/create
a GCP image in the build pipeline. We deprecate the image (because
we haven't decided to release it yet) and then add it to the
corresponding image family for the stream it was built against.
The promote-image command will be used in the release pipeline to
undeprecate the image now that we have decided to release it. Since
we only want one image visible in an image family at any given time,
the promote-image command will then deprecate all other images in
the specified image family.