Skip to content
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

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion mantle/cmd/ore/gcloud/image.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ func runImage(cmd *cobra.Command, args []string) {
os.Exit(2)
}

images, err := api.ListImages(context.Background(), imagePrefix)
images, err := api.ListImages(context.Background(), imagePrefix, "")
if err != nil {
fmt.Fprintf(os.Stderr, "%v\n", err)
os.Exit(1)
Expand Down
113 changes: 113 additions & 0 deletions mantle/cmd/ore/gcloud/promote-image.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,113 @@
// Copyright 2020 Red Hat
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

package gcloud

import (
"github.com/coreos/mantle/platform/api/gcloud"
"github.com/spf13/cobra"
"golang.org/x/net/context"
)

var (
cmdPromoteImage = &cobra.Command{
Use: "promote-image",
Short: "Promote GCP image in image family",
Long: "Promote GCP image in image family and deprecate all others",
Run: runPromoteImage,
}

promoteImageName string
promoteImageFamily string
)

func init() {
cmdPromoteImage.Flags().StringVar(&promoteImageName, "image", "", "GCP image name")
cmdPromoteImage.Flags().StringVar(&promoteImageFamily, "family", "", "GCP image family to promote within")
GCloud.AddCommand(cmdPromoteImage)
}

func deprecateImage(name string, state gcloud.DeprecationState, replacement string) {
plog.Infof("Changing deprecation state of image: %v -> %v", name, state)
pending, err := api.DeprecateImage(name, state, replacement)
if err == nil {
err = pending.Wait()
Copy link
Contributor

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.

Copy link
Member Author

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

}
dustymabe marked this conversation as resolved.
Show resolved Hide resolved
// New if statement to check err on api.DeprecateImage or pending.Wait()
if err != nil {
plog.Fatalf("Changing deprecation state of image failed: %v\n", err)
}
}

func runPromoteImage(cmd *cobra.Command, args []string) {
// Check that the user provided an image
if promoteImageName == "" {
plog.Fatal("Must provide an image name via --image")
}
bgilbert marked this conversation as resolved.
Show resolved Hide resolved
// Check that the user provided an image family
if promoteImageFamily == "" {
plog.Fatal("Must provide an image family via --family")
}

plog.Infof("Attempting to promote %v in family %v",
promoteImageName, promoteImageFamily)

// Get all images in the image family
images, err := api.ListImages(context.Background(), "", promoteImageFamily)
if err != nil {
plog.Fatal(err)
}

// Make sure the specified image exists in the specified image family
found := false
for _, image := range images {
if image.Name == promoteImageName {
found = true
}
}
if !found {
plog.Fatalf("The image (%v) must be in the image family (%v)",
promoteImageName, promoteImageFamily)
}

// First undeprecate the image we want to promote
deprecateImage(promoteImageName, gcloud.DeprecationStateActive, "")
Copy link
Contributor

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.

Copy link
Member Author

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.

Copy link
Member Author

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


// Next deprecate all other images in the image family
// that need to be deprecated.
for _, image := range images {
// don't deprecate the image we just undeprecated
if image.Name == promoteImageName {
continue
}
// Some debug messages which are useful when needed.
if image.Deprecated != nil {
plog.Debugf("Deprecation state for %v is %v",
image.Name, image.Deprecated.State)
} else {
plog.Debugf("Deprecation state is nil for %v", image.Name)
}
// Perform the deprecation if the image is not already deprecated.
// We detect if it is active by checking if it either doesn't
// have any deprecation state or if it is explicitly ACTIVE.
if image.Deprecated == nil ||
image.Deprecated.State == string(gcloud.DeprecationStateActive) {
deprecateImage(
image.Name,
gcloud.DeprecationStateDeprecated,
promoteImageName,
)
}
}
}
2 changes: 1 addition & 1 deletion mantle/cmd/plume/release.go
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,7 @@ func doGCE(ctx context.Context, client *http.Client, src *storage.Bucket, spec *
desc := fmt.Sprintf("%s, %s, %s published on %s", spec.GCE.Description,
specVersion, specBoard, date.Format("2006-01-02"))

images, err := api.ListImages(ctx, spec.GCE.Family+"-")
images, err := api.ListImages(ctx, spec.GCE.Family+"-", "")
if err != nil {
plog.Fatal(err)
}
Expand Down
23 changes: 7 additions & 16 deletions mantle/platform/api/gcloud/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,8 @@
package gcloud

import (
"fmt"
"io/ioutil"
"net/http"
"strings"
"time"

"github.com/coreos/pkg/capnslog"
Expand Down Expand Up @@ -52,26 +50,19 @@ type API struct {
}

func New(opts *Options) (*API, error) {
const endpointPrefix = "https://www.googleapis.com/compute/v1/"

// If the image name isn't a full api endpoint accept a name beginning
// with "projects/" to specify a different project from the instance.
// Also accept a short name and use instance project.
if opts.Image != "" {
if strings.HasPrefix(opts.Image, "projects/") {
opts.Image = endpointPrefix + opts.Image
} else if !strings.Contains(opts.Image, "/") {
opts.Image = fmt.Sprintf("%sprojects/%s/global/images/%s", endpointPrefix, opts.Project, opts.Image)
} else if !strings.HasPrefix(opts.Image, endpointPrefix) {
return nil, fmt.Errorf("GCE Image argument must be the full api endpoint, begin with 'projects/', or use the short name")
}
}

var (
client *http.Client
err error
)

if opts.Image != "" {
opts.Image, err = getImageAPIEndpoint(opts.Image, opts.Project)
if err != nil {
return nil, err
}
}

if opts.ServiceAuth {
client = auth.GoogleServiceClient()
} else if opts.JSONKeyFile != "" {
Expand Down
39 changes: 38 additions & 1 deletion mantle/platform/api/gcloud/image.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,31 @@ type ImageSpec struct {
Licenses []string // short names
}

// Given a string representing an image return the full API
dustymabe marked this conversation as resolved.
Show resolved Hide resolved
// endpoint for the image. For example:
// https://www.googleapis.com/compute/v1/projects/fedora-coreos-cloud/global/images/fedora-coreos-31-20200420-3-0-gcp-x86-64
func getImageAPIEndpoint(image, project string) (string, error) {
const endpointPrefix = "https://www.googleapis.com/compute/v1/"
// If the input is already a full API endpoint then just return it
if strings.HasPrefix(image, endpointPrefix) {
return image, nil
}
// Accept a name beginning with "projects/" to specify a different
// project from the instance.
if strings.HasPrefix(image, "projects/") {
return endpointPrefix + image, nil
}
// Also accept a short name (no '/') build API endpoint using
// instance project (opts.Project).
if !strings.Contains(image, "/") {
return fmt.Sprintf(
"%sprojects/%s/global/images/%s",
endpointPrefix, project, image), nil
}
return "", fmt.Errorf("GCP Image argument must be the full api endpoint," +
" begin with 'projects/', or use the short name")
}

// CreateImage creates an image on GCE and returns operation details and
// a Pending. If overwrite is true, an existing image will be overwritten
// if it exists.
Expand Down Expand Up @@ -102,12 +127,15 @@ func (a *API) CreateImage(spec *ImageSpec, overwrite bool) (*compute.Operation,
return op, a.NewPending(op.Name, doable), nil
}

func (a *API) ListImages(ctx context.Context, prefix string) ([]*compute.Image, error) {
func (a *API) ListImages(ctx context.Context, prefix string, family string) ([]*compute.Image, error) {
dustymabe marked this conversation as resolved.
Show resolved Hide resolved
var images []*compute.Image
listReq := a.compute.Images.List(a.options.Project)
if prefix != "" {
listReq.Filter(fmt.Sprintf("name eq ^%s.*", prefix))
}
if family != "" {
listReq.Filter(fmt.Sprintf("family eq ^%s$", family))
}
err := listReq.Pages(ctx, func(i *compute.ImageList) error {
images = append(images, i.Items...)
return nil
Expand All @@ -134,6 +162,15 @@ func (a *API) GetPendingForImage(image *compute.Image) (*Pending, error) {
}

func (a *API) DeprecateImage(name string, state DeprecationState, replacement string) (*Pending, error) {
var err error

if replacement != "" {
replacement, err = getImageAPIEndpoint(replacement, a.options.Project)
if err != nil {
return nil, err
}
}

req := a.compute.Images.Deprecate(a.options.Project, name, &compute.DeprecationStatus{
State: string(state),
Replacement: replacement,
Expand Down