From 1c33270f9123fd508723b964d6f59d4022e4e8a8 Mon Sep 17 00:00:00 2001 From: John Gardiner Myers Date: Fri, 28 May 2021 20:45:06 -0700 Subject: [PATCH 1/6] Always set canonical location of assets --- pkg/assets/builder.go | 23 ++++++++++------------- upup/pkg/fi/cloudup/loader.go | 4 ++-- upup/pkg/fi/dryrun_target.go | 4 +--- 3 files changed, 13 insertions(+), 18 deletions(-) diff --git a/pkg/assets/builder.go b/pkg/assets/builder.go index cf92b7df03429..032de6a7be4fb 100644 --- a/pkg/assets/builder.go +++ b/pkg/assets/builder.go @@ -120,9 +120,10 @@ func (a *AssetBuilder) RemapManifest(data []byte) ([]byte, error) { // RemapImage normalizes a containers location if a user sets the AssetsLocation ContainerRegistry location. func (a *AssetBuilder) RemapImage(image string) (string, error) { - asset := &ContainerAsset{} - - asset.DockerImage = image + asset := &ContainerAsset{ + DockerImage: image, + CanonicalLocation: image, + } if strings.HasPrefix(image, "k8s.gcr.io/kops/dns-controller:") { // To use user-defined DNS Controller: @@ -167,7 +168,6 @@ func (a *AssetBuilder) RemapImage(image string) (string, error) { } asset.DockerImage = normalized - asset.CanonicalLocation = image // Run the new image image = asset.DockerImage @@ -192,8 +192,6 @@ func (a *AssetBuilder) RemapImage(image string) (string, error) { asset.DockerImage = registryMirror + "/" + normalized } - asset.CanonicalLocation = image - // Run the new image image = asset.DockerImage } @@ -210,11 +208,11 @@ func (a *AssetBuilder) RemapFileAndSHA(fileURL *url.URL) (*url.URL, *hashing.Has } fileAsset := &FileAsset{ - DownloadURL: fileURL, + DownloadURL: fileURL, + CanonicalURL: fileURL, } if a.AssetsLocation != nil && a.AssetsLocation.FileRepository != nil { - fileAsset.CanonicalURL = fileURL normalizedFileURL, err := a.remapURL(fileURL) if err != nil { @@ -245,13 +243,12 @@ func (a *AssetBuilder) RemapFileAndSHAValue(fileURL *url.URL, shaValue string) ( } fileAsset := &FileAsset{ - DownloadURL: fileURL, - SHAValue: shaValue, + DownloadURL: fileURL, + CanonicalURL: fileURL, + SHAValue: shaValue, } if a.AssetsLocation != nil && a.AssetsLocation.FileRepository != nil { - fileAsset.CanonicalURL = fileURL - normalizedFile, err := a.remapURL(fileURL) if err != nil { return nil, err @@ -283,7 +280,7 @@ func (a *AssetBuilder) findHash(file *FileAsset) (*hashing.Hash, error) { // rest of the time. If not we get a chicken and the egg problem where we are reading the sha file // before it exists. u := file.DownloadURL - if a.Phase == "assets" && file.CanonicalURL != nil { + if a.Phase == "assets" { u = file.CanonicalURL } diff --git a/upup/pkg/fi/cloudup/loader.go b/upup/pkg/fi/cloudup/loader.go index 2e1cccbf02dd0..52691ae25ec56 100644 --- a/upup/pkg/fi/cloudup/loader.go +++ b/upup/pkg/fi/cloudup/loader.go @@ -67,7 +67,7 @@ func (l *Loader) BuildTasks(assetBuilder *assets.AssetBuilder, lifecycle *fi.Lif func (l *Loader) addAssetCopyTasks(assets []*assets.ContainerAsset, lifecycle *fi.Lifecycle) error { for _, asset := range assets { - if asset.CanonicalLocation != "" && asset.DockerImage != asset.CanonicalLocation { + if asset.DockerImage != asset.CanonicalLocation { context := &fi.ModelBuilderContext{ Tasks: l.tasks, } @@ -100,7 +100,7 @@ func (l *Loader) addAssetFileCopyTasks(assets []*assets.FileAsset, lifecycle *fi } // test if the asset needs to be copied - if asset.CanonicalURL != nil && asset.DownloadURL.String() != asset.CanonicalURL.String() { + if asset.DownloadURL.String() != asset.CanonicalURL.String() { klog.V(10).Infof("processing asset: %q, %q", asset.DownloadURL.String(), asset.CanonicalURL.String()) context := &fi.ModelBuilderContext{ Tasks: l.tasks, diff --git a/upup/pkg/fi/dryrun_target.go b/upup/pkg/fi/dryrun_target.go index c90b769d15ae9..9e9258caaccca 100644 --- a/upup/pkg/fi/dryrun_target.go +++ b/upup/pkg/fi/dryrun_target.go @@ -262,10 +262,8 @@ func (t *DryRunTarget) PrintReport(taskMap map[string]Task, out io.Writer) error if len(t.assetBuilder.FileAssets) != 0 { klog.V(4).Infof("FileAssets:") for _, a := range t.assetBuilder.FileAssets { - if a.DownloadURL != nil && a.CanonicalURL != nil { + if a.DownloadURL != nil { klog.V(4).Infof(" %s %s", a.DownloadURL.String(), a.CanonicalURL.String()) - } else if a.DownloadURL != nil { - klog.V(4).Infof(" %s", a.DownloadURL.String()) } } } From 4c2508b6ec3b73398ecfe05d3f9fb913118443c3 Mon Sep 17 00:00:00 2001 From: John Gardiner Myers Date: Thu, 27 May 2021 22:34:21 -0700 Subject: [PATCH 2/6] Add "kops get assets" command --- cmd/kops/BUILD.bazel | 1 + cmd/kops/get.go | 1 + cmd/kops/get_assets.go | 191 +++++++++++++++++++++++++++ cmd/kops/update_cluster.go | 12 +- docs/cli/kops_get.md | 1 + docs/cli/kops_get_assets.md | 54 ++++++++ upup/pkg/fi/cloudup/apply_cluster.go | 18 ++- 7 files changed, 276 insertions(+), 2 deletions(-) create mode 100644 cmd/kops/get_assets.go create mode 100644 docs/cli/kops_get_assets.md diff --git a/cmd/kops/BUILD.bazel b/cmd/kops/BUILD.bazel index 263fb7c730da9..5ec7942d7ec7f 100644 --- a/cmd/kops/BUILD.bazel +++ b/cmd/kops/BUILD.bazel @@ -30,6 +30,7 @@ go_library( "export_kubecfg.go", "gen_help_docs.go", "get.go", + "get_assets.go", "get_cluster.go", "get_instancegroups.go", "get_instances.go", diff --git a/cmd/kops/get.go b/cmd/kops/get.go index 1d69cb2dd84f1..30bd98542996c 100644 --- a/cmd/kops/get.go +++ b/cmd/kops/get.go @@ -106,6 +106,7 @@ func NewCmdGet(f *util.Factory, out io.Writer) *cobra.Command { cmd.PersistentFlags().StringVarP(&options.output, "output", "o", options.output, "output format. One of: table, yaml, json") // create subcommands + cmd.AddCommand(NewCmdGetAssets(f, out, options)) cmd.AddCommand(NewCmdGetCluster(f, out, options)) cmd.AddCommand(NewCmdGetInstanceGroups(f, out, options)) cmd.AddCommand(NewCmdGetSecrets(f, out, options)) diff --git a/cmd/kops/get_assets.go b/cmd/kops/get_assets.go new file mode 100644 index 0000000000000..20af75ef4ea06 --- /dev/null +++ b/cmd/kops/get_assets.go @@ -0,0 +1,191 @@ +/* +Copyright 2020 The Kubernetes Authors. + +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 main + +import ( + "context" + "encoding/json" + "fmt" + "io" + + "k8s.io/kubectl/pkg/util/i18n" + "k8s.io/kubectl/pkg/util/templates" + "sigs.k8s.io/yaml" + + "k8s.io/kops/util/pkg/tables" + + "github.com/spf13/cobra" + "k8s.io/kops/cmd/kops/util" + "k8s.io/kops/upup/pkg/fi/cloudup" +) + +type Image struct { + Image string `json:"image"` + Mirror string `json:"mirror"` +} + +type File struct { + File string `json:"file"` + Mirror string `json:"mirror"` + SHA string `json:"sha"` +} + +type AssetResult struct { + // ContainerAssets are the container assets we use (output). + Images []*Image `json:"images,omitempty"` + // FileAssets are the file assets we use (output). + Files []*File `json:"files,omitempty"` +} + +func NewCmdGetAssets(f *util.Factory, out io.Writer, options *GetOptions) *cobra.Command { + getAssetsShort := i18n.T(`Display assets for cluster.`) + + getAssetsLong := templates.LongDesc(i18n.T(` + Display assets for cluster.`)) + + getAssetsExample := templates.Examples(i18n.T(` + # Display all assets. + kops get assets + `)) + + cmd := &cobra.Command{ + Use: "assets", + Short: getAssetsShort, + Long: getAssetsLong, + Example: getAssetsExample, + Run: func(cmd *cobra.Command, args []string) { + ctx := context.TODO() + + if err := rootCommand.ProcessArgs(args); err != nil { + exitWithError(err) + } + + err := RunGetAssets(ctx, f, out, options) + if err != nil { + exitWithError(err) + } + }, + } + + return cmd +} + +func RunGetAssets(ctx context.Context, f *util.Factory, out io.Writer, options *GetOptions) error { + + clusterName := rootCommand.ClusterName() + options.clusterName = clusterName + if clusterName == "" { + return fmt.Errorf("--name is required") + } + + updateClusterResults, err := RunUpdateCluster(ctx, f, clusterName, out, &UpdateClusterOptions{ + Target: cloudup.TargetDryRun, + Phase: string(cloudup.PhaseStageAssets), + Quiet: true, + }) + if err != nil { + return err + } + + result := AssetResult{ + Images: make([]*Image, 0, len(updateClusterResults.ContainerAssets)), + Files: make([]*File, 0, len(updateClusterResults.FileAssets)), + } + + seen := map[string]bool{} + for _, containerAsset := range updateClusterResults.ContainerAssets { + image := Image{ + Image: containerAsset.CanonicalLocation, + Mirror: containerAsset.DockerImage, + } + if !seen[image.Image] { + result.Images = append(result.Images, &image) + seen[image.Image] = true + } + } + seen = map[string]bool{} + for _, fileAsset := range updateClusterResults.FileAssets { + file := File{ + File: fileAsset.CanonicalURL.String(), + Mirror: fileAsset.DownloadURL.String(), + SHA: fileAsset.SHAValue, + } + if !seen[file.File] { + result.Files = append(result.Files, &file) + seen[file.File] = true + } + } + + switch options.output { + case OutputTable: + if err = containerOutputTable(result.Images, out); err != nil { + return err + } + return fileOutputTable(result.Files, out) + case OutputYaml: + y, err := yaml.Marshal(result) + if err != nil { + return fmt.Errorf("unable to marshal YAML: %v", err) + } + if _, err := out.Write(y); err != nil { + return fmt.Errorf("error writing to output: %v", err) + } + case OutputJSON: + j, err := json.Marshal(result) + if err != nil { + return fmt.Errorf("unable to marshal JSON: %v", err) + } + if _, err := out.Write(j); err != nil { + return fmt.Errorf("error writing to output: %v", err) + } + default: + return fmt.Errorf("unsupported output format: %q", options.output) + } + + return nil +} + +func containerOutputTable(images []*Image, out io.Writer) error { + fmt.Println("") + t := &tables.Table{} + t.AddColumn("IMAGE", func(i *Image) string { + return i.Image + }) + t.AddColumn("MIRROR", func(i *Image) string { + return i.Mirror + }) + + columns := []string{"IMAGE", "MIRROR"} + return t.Render(images, out, columns...) +} + +func fileOutputTable(files []*File, out io.Writer) error { + fmt.Println("") + t := &tables.Table{} + t.AddColumn("FILE", func(f *File) string { + return f.File + }) + t.AddColumn("MIRROR", func(f *File) string { + return f.Mirror + }) + t.AddColumn("SHA", func(f *File) string { + return f.SHA + }) + + columns := []string{"FILE", "MIRROR", "SHA"} + return t.Render(files, out, columns...) +} diff --git a/cmd/kops/update_cluster.go b/cmd/kops/update_cluster.go index fe589d2c996b6..c55e07cc980cc 100644 --- a/cmd/kops/update_cluster.go +++ b/cmd/kops/update_cluster.go @@ -32,6 +32,7 @@ import ( "k8s.io/klog/v2" "k8s.io/kops/cmd/kops/util" "k8s.io/kops/pkg/apis/kops" + "k8s.io/kops/pkg/assets" "k8s.io/kops/pkg/kubeconfig" "k8s.io/kops/upup/pkg/fi" "k8s.io/kops/upup/pkg/fi/cloudup" @@ -65,6 +66,7 @@ type UpdateClusterOptions struct { SSHPublicKey string RunTasksOptions fi.RunTasksOptions AllowKopsDowngrade bool + Quiet bool CreateKubecfg bool admin time.Duration @@ -139,6 +141,11 @@ type UpdateClusterResults struct { // TaskMap is the map of tasks that we built (output) TaskMap map[string]fi.Task + + // ContainerAssets are the container assets we use (output). + ContainerAssets []*assets.ContainerAsset + // FileAssets are the file assets we use (output). + FileAssets []*assets.FileAsset } func RunUpdateCluster(ctx context.Context, f *util.Factory, clusterName string, out io.Writer, c *UpdateClusterOptions) (*UpdateClusterResults, error) { @@ -280,6 +287,7 @@ func RunUpdateCluster(ctx context.Context, f *util.Factory, clusterName string, Phase: phase, TargetName: targetName, LifecycleOverrides: lifecycleOverrideMap, + Quiet: c.Quiet, } if err := applyCmd.Run(ctx); err != nil { @@ -288,8 +296,10 @@ func RunUpdateCluster(ctx context.Context, f *util.Factory, clusterName string, results.Target = applyCmd.Target results.TaskMap = applyCmd.TaskMap + results.ContainerAssets = applyCmd.ContainerAssets + results.FileAssets = applyCmd.FileAssets - if isDryrun { + if isDryrun && !c.Quiet { target := applyCmd.Target.(*fi.DryRunTarget) if target.HasChanges() { fmt.Fprintf(out, "Must specify --yes to apply changes\n") diff --git a/docs/cli/kops_get.md b/docs/cli/kops_get.md index 7cbc9aa51d4ce..68cbdbd8329bc 100644 --- a/docs/cli/kops_get.md +++ b/docs/cli/kops_get.md @@ -70,6 +70,7 @@ kops get [flags] ### SEE ALSO * [kops](kops.md) - kOps is Kubernetes Operations. +* [kops get assets](kops_get_assets.md) - Display assets for cluster. * [kops get clusters](kops_get_clusters.md) - Get one or many clusters. * [kops get instancegroups](kops_get_instancegroups.md) - Get one or many instancegroups * [kops get instances](kops_get_instances.md) - Display cluster instances. diff --git a/docs/cli/kops_get_assets.md b/docs/cli/kops_get_assets.md new file mode 100644 index 0000000000000..a4c19588dca28 --- /dev/null +++ b/docs/cli/kops_get_assets.md @@ -0,0 +1,54 @@ + + + +## kops get assets + +Display assets for cluster. + +### Synopsis + +Display assets for cluster. + +``` +kops get assets [flags] +``` + +### Examples + +``` + # Display all assets. + kops get assets +``` + +### Options + +``` + -h, --help help for assets +``` + +### Options inherited from parent commands + +``` + --add_dir_header If true, adds the file directory to the header of the log messages + --alsologtostderr log to standard error as well as files + --config string yaml config file (default is $HOME/.kops.yaml) + --log_backtrace_at traceLocation when logging hits line file:N, emit a stack trace (default :0) + --log_dir string If non-empty, write log files in this directory + --log_file string If non-empty, use this log file + --log_file_max_size uint Defines the maximum size a log file can grow to. Unit is megabytes. If the value is 0, the maximum file size is unlimited. (default 1800) + --logtostderr log to standard error instead of files (default true) + --name string Name of cluster. Overrides KOPS_CLUSTER_NAME environment variable + --one_output If true, only write logs to their native severity level (vs also writing to each lower severity level) + -o, --output string output format. One of: table, yaml, json (default "table") + --skip_headers If true, avoid header prefixes in the log messages + --skip_log_headers If true, avoid headers when opening log files + --state string Location of state storage (kops 'config' file). Overrides KOPS_STATE_STORE environment variable + --stderrthreshold severity logs at or above this threshold go to stderr (default 2) + -v, --v Level number for the log level verbosity + --vmodule moduleSpec comma-separated list of pattern=N settings for file-filtered logging +``` + +### SEE ALSO + +* [kops get](kops_get.md) - Get one or many resources. + diff --git a/upup/pkg/fi/cloudup/apply_cluster.go b/upup/pkg/fi/cloudup/apply_cluster.go index a783369e5a73c..f241367c0ba25 100644 --- a/upup/pkg/fi/cloudup/apply_cluster.go +++ b/upup/pkg/fi/cloudup/apply_cluster.go @@ -20,6 +20,7 @@ import ( "bytes" "context" "fmt" + "io" "net" "net/url" "os" @@ -135,8 +136,16 @@ type ApplyClusterCmd struct { // that is re-mapped. LifecycleOverrides map[string]fi.Lifecycle + // Quiet suppresses most output + Quiet bool + // TaskMap is the map of tasks that we built (output) TaskMap map[string]fi.Task + + // ContainerAssets are the container assets we use (output). + ContainerAssets []*assets.ContainerAsset + // FileAssets are the file assets we use (output). + FileAssets []*assets.FileAsset } func (c *ApplyClusterCmd) Run(ctx context.Context) error { @@ -719,7 +728,11 @@ func (c *ApplyClusterCmd) Run(ctx context.Context) error { shouldPrecreateDNS = false case TargetDryRun: - target = fi.NewDryRunTarget(assetBuilder, os.Stdout) + var out io.Writer = os.Stdout + if c.Quiet { + out = io.Discard + } + target = fi.NewDryRunTarget(assetBuilder, out) dryRun = true // Avoid making changes on a dry-run @@ -801,6 +814,9 @@ func (c *ApplyClusterCmd) Run(ctx context.Context) error { return fmt.Errorf("error closing target: %v", err) } + c.ContainerAssets = assetBuilder.ContainerAssets + c.FileAssets = assetBuilder.FileAssets + return nil } From 34c6f7f2959df846bb53e0b4c034b80d5bc42c20 Mon Sep 17 00:00:00 2001 From: John Gardiner Myers Date: Sat, 29 May 2021 16:35:23 -0700 Subject: [PATCH 3/6] Rename "ContainerAssets" to "ImageAssets" --- cmd/kops/get_assets.go | 6 +++--- cmd/kops/update_cluster.go | 6 +++--- pkg/assets/builder.go | 18 +++++++++--------- pkg/assets/builder_test.go | 4 ++-- upup/pkg/fi/cloudup/apply_cluster.go | 6 +++--- upup/pkg/fi/cloudup/loader.go | 4 ++-- upup/pkg/fi/dryrun_target.go | 6 +++--- 7 files changed, 25 insertions(+), 25 deletions(-) diff --git a/cmd/kops/get_assets.go b/cmd/kops/get_assets.go index 20af75ef4ea06..bbb1934310ef3 100644 --- a/cmd/kops/get_assets.go +++ b/cmd/kops/get_assets.go @@ -45,7 +45,7 @@ type File struct { } type AssetResult struct { - // ContainerAssets are the container assets we use (output). + // Images are the image assets we use (output). Images []*Image `json:"images,omitempty"` // FileAssets are the file assets we use (output). Files []*File `json:"files,omitempty"` @@ -102,12 +102,12 @@ func RunGetAssets(ctx context.Context, f *util.Factory, out io.Writer, options * } result := AssetResult{ - Images: make([]*Image, 0, len(updateClusterResults.ContainerAssets)), + Images: make([]*Image, 0, len(updateClusterResults.ImageAssets)), Files: make([]*File, 0, len(updateClusterResults.FileAssets)), } seen := map[string]bool{} - for _, containerAsset := range updateClusterResults.ContainerAssets { + for _, containerAsset := range updateClusterResults.ImageAssets { image := Image{ Image: containerAsset.CanonicalLocation, Mirror: containerAsset.DockerImage, diff --git a/cmd/kops/update_cluster.go b/cmd/kops/update_cluster.go index c55e07cc980cc..029243f17f7f3 100644 --- a/cmd/kops/update_cluster.go +++ b/cmd/kops/update_cluster.go @@ -142,8 +142,8 @@ type UpdateClusterResults struct { // TaskMap is the map of tasks that we built (output) TaskMap map[string]fi.Task - // ContainerAssets are the container assets we use (output). - ContainerAssets []*assets.ContainerAsset + // ImageAssets are the image assets we use (output). + ImageAssets []*assets.ImageAsset // FileAssets are the file assets we use (output). FileAssets []*assets.FileAsset } @@ -296,7 +296,7 @@ func RunUpdateCluster(ctx context.Context, f *util.Factory, clusterName string, results.Target = applyCmd.Target results.TaskMap = applyCmd.TaskMap - results.ContainerAssets = applyCmd.ContainerAssets + results.ImageAssets = applyCmd.ImageAssets results.FileAssets = applyCmd.FileAssets if isDryrun && !c.Quiet { diff --git a/pkg/assets/builder.go b/pkg/assets/builder.go index 032de6a7be4fb..aa1990b6501ee 100644 --- a/pkg/assets/builder.go +++ b/pkg/assets/builder.go @@ -39,9 +39,9 @@ import ( // AssetBuilder discovers and remaps assets. type AssetBuilder struct { - ContainerAssets []*ContainerAsset - FileAssets []*FileAsset - AssetsLocation *kops.Assets + ImageAssets []*ImageAsset + FileAssets []*FileAsset + AssetsLocation *kops.Assets // TODO we'd like to use cloudup.Phase here, but that introduces a go cyclic dependency Phase string @@ -63,12 +63,12 @@ type StaticManifest struct { Roles []kops.InstanceGroupRole } -// ContainerAsset models a container's location. -type ContainerAsset struct { - // DockerImage will be the name of the container we should run. +// ImageAsset models an image's location. +type ImageAsset struct { + // DockerImage will be the name of the image we should run. // This is used to copy a container to a ContainerRegistry. DockerImage string - // CanonicalLocation will be the source location of the container. + // CanonicalLocation will be the source location of the image. CanonicalLocation string } @@ -120,7 +120,7 @@ func (a *AssetBuilder) RemapManifest(data []byte) ([]byte, error) { // RemapImage normalizes a containers location if a user sets the AssetsLocation ContainerRegistry location. func (a *AssetBuilder) RemapImage(image string) (string, error) { - asset := &ContainerAsset{ + asset := &ImageAsset{ DockerImage: image, CanonicalLocation: image, } @@ -196,7 +196,7 @@ func (a *AssetBuilder) RemapImage(image string) (string, error) { image = asset.DockerImage } - a.ContainerAssets = append(a.ContainerAssets, asset) + a.ImageAssets = append(a.ImageAssets, asset) return image, nil } diff --git a/pkg/assets/builder_test.go b/pkg/assets/builder_test.go index 7ebfdca5bc7d7..f382c3db6fd24 100644 --- a/pkg/assets/builder_test.go +++ b/pkg/assets/builder_test.go @@ -28,8 +28,8 @@ import ( func buildAssetBuilder(t *testing.T) *AssetBuilder { builder := &AssetBuilder{ - AssetsLocation: &kops.Assets{}, - ContainerAssets: []*ContainerAsset{}, + AssetsLocation: &kops.Assets{}, + ImageAssets: []*ImageAsset{}, } return builder } diff --git a/upup/pkg/fi/cloudup/apply_cluster.go b/upup/pkg/fi/cloudup/apply_cluster.go index f241367c0ba25..62cbd508e7085 100644 --- a/upup/pkg/fi/cloudup/apply_cluster.go +++ b/upup/pkg/fi/cloudup/apply_cluster.go @@ -142,8 +142,8 @@ type ApplyClusterCmd struct { // TaskMap is the map of tasks that we built (output) TaskMap map[string]fi.Task - // ContainerAssets are the container assets we use (output). - ContainerAssets []*assets.ContainerAsset + // ImageAssets are the image assets we use (output). + ImageAssets []*assets.ImageAsset // FileAssets are the file assets we use (output). FileAssets []*assets.FileAsset } @@ -814,7 +814,7 @@ func (c *ApplyClusterCmd) Run(ctx context.Context) error { return fmt.Errorf("error closing target: %v", err) } - c.ContainerAssets = assetBuilder.ContainerAssets + c.ImageAssets = assetBuilder.ImageAssets c.FileAssets = assetBuilder.FileAssets return nil diff --git a/upup/pkg/fi/cloudup/loader.go b/upup/pkg/fi/cloudup/loader.go index 52691ae25ec56..b585e22be631a 100644 --- a/upup/pkg/fi/cloudup/loader.go +++ b/upup/pkg/fi/cloudup/loader.go @@ -51,7 +51,7 @@ func (l *Loader) BuildTasks(assetBuilder *assets.AssetBuilder, lifecycle *fi.Lif l.tasks = context.Tasks } - if err := l.addAssetCopyTasks(assetBuilder.ContainerAssets, lifecycle); err != nil { + if err := l.addAssetCopyTasks(assetBuilder.ImageAssets, lifecycle); err != nil { return nil, err } @@ -65,7 +65,7 @@ func (l *Loader) BuildTasks(assetBuilder *assets.AssetBuilder, lifecycle *fi.Lif return l.tasks, nil } -func (l *Loader) addAssetCopyTasks(assets []*assets.ContainerAsset, lifecycle *fi.Lifecycle) error { +func (l *Loader) addAssetCopyTasks(assets []*assets.ImageAsset, lifecycle *fi.Lifecycle) error { for _, asset := range assets { if asset.DockerImage != asset.CanonicalLocation { context := &fi.ModelBuilderContext{ diff --git a/upup/pkg/fi/dryrun_target.go b/upup/pkg/fi/dryrun_target.go index 9e9258caaccca..d449cefe3f0ad 100644 --- a/upup/pkg/fi/dryrun_target.go +++ b/upup/pkg/fi/dryrun_target.go @@ -252,9 +252,9 @@ func (t *DryRunTarget) PrintReport(taskMap map[string]Task, out io.Writer) error } } - if len(t.assetBuilder.ContainerAssets) != 0 { - klog.V(4).Infof("ContainerAssets:") - for _, a := range t.assetBuilder.ContainerAssets { + if len(t.assetBuilder.ImageAssets) != 0 { + klog.V(4).Infof("ImageAssets:") + for _, a := range t.assetBuilder.ImageAssets { klog.V(4).Infof(" %s %s", a.DockerImage, a.CanonicalLocation) } } From 95aa3fd13e80f622202d2e4f9829bbd2dfc3037b Mon Sep 17 00:00:00 2001 From: John Gardiner Myers Date: Sat, 29 May 2021 16:40:56 -0700 Subject: [PATCH 4/6] Rename "DockerImage" to "DownloadLocation" --- cmd/kops/get_assets.go | 2 +- pkg/assets/builder.go | 16 ++++++++-------- upup/pkg/fi/cloudup/loader.go | 6 +++--- upup/pkg/fi/dryrun_target.go | 2 +- 4 files changed, 13 insertions(+), 13 deletions(-) diff --git a/cmd/kops/get_assets.go b/cmd/kops/get_assets.go index bbb1934310ef3..36b93a7956aee 100644 --- a/cmd/kops/get_assets.go +++ b/cmd/kops/get_assets.go @@ -110,7 +110,7 @@ func RunGetAssets(ctx context.Context, f *util.Factory, out io.Writer, options * for _, containerAsset := range updateClusterResults.ImageAssets { image := Image{ Image: containerAsset.CanonicalLocation, - Mirror: containerAsset.DockerImage, + Mirror: containerAsset.DownloadLocation, } if !seen[image.Image] { result.Images = append(result.Images, &image) diff --git a/pkg/assets/builder.go b/pkg/assets/builder.go index aa1990b6501ee..9d1252b8776ae 100644 --- a/pkg/assets/builder.go +++ b/pkg/assets/builder.go @@ -65,9 +65,9 @@ type StaticManifest struct { // ImageAsset models an image's location. type ImageAsset struct { - // DockerImage will be the name of the image we should run. - // This is used to copy a container to a ContainerRegistry. - DockerImage string + // DownloadLocation will be the name of the image we should run. + // This is used to copy an image to a ContainerRegistry. + DownloadLocation string // CanonicalLocation will be the source location of the image. CanonicalLocation string } @@ -121,7 +121,7 @@ func (a *AssetBuilder) RemapManifest(data []byte) ([]byte, error) { // RemapImage normalizes a containers location if a user sets the AssetsLocation ContainerRegistry location. func (a *AssetBuilder) RemapImage(image string) (string, error) { asset := &ImageAsset{ - DockerImage: image, + DownloadLocation: image, CanonicalLocation: image, } @@ -167,10 +167,10 @@ func (a *AssetBuilder) RemapImage(image string) (string, error) { normalized = re.ReplaceAllString(normalized, containerProxy) } - asset.DockerImage = normalized + asset.DownloadLocation = normalized // Run the new image - image = asset.DockerImage + image = asset.DownloadLocation } if a.AssetsLocation != nil && a.AssetsLocation.ContainerRegistry != nil { @@ -189,11 +189,11 @@ func (a *AssetBuilder) RemapImage(image string) (string, error) { // We can't nest arbitrarily // Some risk of collisions, but also -- and __ in the names appear to be blocked by docker hub normalized = strings.Replace(normalized, "/", "-", -1) - asset.DockerImage = registryMirror + "/" + normalized + asset.DownloadLocation = registryMirror + "/" + normalized } // Run the new image - image = asset.DockerImage + image = asset.DownloadLocation } a.ImageAssets = append(a.ImageAssets, asset) diff --git a/upup/pkg/fi/cloudup/loader.go b/upup/pkg/fi/cloudup/loader.go index b585e22be631a..a4693d28746f6 100644 --- a/upup/pkg/fi/cloudup/loader.go +++ b/upup/pkg/fi/cloudup/loader.go @@ -67,15 +67,15 @@ func (l *Loader) BuildTasks(assetBuilder *assets.AssetBuilder, lifecycle *fi.Lif func (l *Loader) addAssetCopyTasks(assets []*assets.ImageAsset, lifecycle *fi.Lifecycle) error { for _, asset := range assets { - if asset.DockerImage != asset.CanonicalLocation { + if asset.DownloadLocation != asset.CanonicalLocation { context := &fi.ModelBuilderContext{ Tasks: l.tasks, } copyImageTask := &assettasks.CopyDockerImage{ - Name: fi.String(asset.DockerImage), + Name: fi.String(asset.DownloadLocation), SourceImage: fi.String(asset.CanonicalLocation), - TargetImage: fi.String(asset.DockerImage), + TargetImage: fi.String(asset.DownloadLocation), Lifecycle: lifecycle, } diff --git a/upup/pkg/fi/dryrun_target.go b/upup/pkg/fi/dryrun_target.go index d449cefe3f0ad..d94f52b865d19 100644 --- a/upup/pkg/fi/dryrun_target.go +++ b/upup/pkg/fi/dryrun_target.go @@ -255,7 +255,7 @@ func (t *DryRunTarget) PrintReport(taskMap map[string]Task, out io.Writer) error if len(t.assetBuilder.ImageAssets) != 0 { klog.V(4).Infof("ImageAssets:") for _, a := range t.assetBuilder.ImageAssets { - klog.V(4).Infof(" %s %s", a.DockerImage, a.CanonicalLocation) + klog.V(4).Infof(" %s %s", a.DownloadLocation, a.CanonicalLocation) } } From e498c33da3669c6f7b5cb92b16d4ed81c7ef315b Mon Sep 17 00:00:00 2001 From: John Gardiner Myers Date: Sat, 29 May 2021 16:44:10 -0700 Subject: [PATCH 5/6] More "container" to "image" renaming --- cmd/kops/get_assets.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/cmd/kops/get_assets.go b/cmd/kops/get_assets.go index 36b93a7956aee..ffb9e6e540ce2 100644 --- a/cmd/kops/get_assets.go +++ b/cmd/kops/get_assets.go @@ -107,10 +107,10 @@ func RunGetAssets(ctx context.Context, f *util.Factory, out io.Writer, options * } seen := map[string]bool{} - for _, containerAsset := range updateClusterResults.ImageAssets { + for _, imageAsset := range updateClusterResults.ImageAssets { image := Image{ - Image: containerAsset.CanonicalLocation, - Mirror: containerAsset.DownloadLocation, + Image: imageAsset.CanonicalLocation, + Mirror: imageAsset.DownloadLocation, } if !seen[image.Image] { result.Images = append(result.Images, &image) @@ -132,7 +132,7 @@ func RunGetAssets(ctx context.Context, f *util.Factory, out io.Writer, options * switch options.output { case OutputTable: - if err = containerOutputTable(result.Images, out); err != nil { + if err = imageOutputTable(result.Images, out); err != nil { return err } return fileOutputTable(result.Files, out) @@ -159,7 +159,7 @@ func RunGetAssets(ctx context.Context, f *util.Factory, out io.Writer, options * return nil } -func containerOutputTable(images []*Image, out io.Writer) error { +func imageOutputTable(images []*Image, out io.Writer) error { fmt.Println("") t := &tables.Table{} t.AddColumn("IMAGE", func(i *Image) string { From 0e775023ac98ae92db8e53cbbcd11aaa36502eed Mon Sep 17 00:00:00 2001 From: John Gardiner Myers Date: Sun, 30 May 2021 10:06:25 -0700 Subject: [PATCH 6/6] Use more consistent terminology --- cmd/kops/get_assets.go | 48 +++++++++++++++++++++--------------------- 1 file changed, 24 insertions(+), 24 deletions(-) diff --git a/cmd/kops/get_assets.go b/cmd/kops/get_assets.go index ffb9e6e540ce2..3aa8b13737984 100644 --- a/cmd/kops/get_assets.go +++ b/cmd/kops/get_assets.go @@ -34,14 +34,14 @@ import ( ) type Image struct { - Image string `json:"image"` - Mirror string `json:"mirror"` + Canonical string `json:"canonical"` + Download string `json:"download"` } type File struct { - File string `json:"file"` - Mirror string `json:"mirror"` - SHA string `json:"sha"` + Canonical string `json:"canonical"` + Download string `json:"download"` + SHA string `json:"sha"` } type AssetResult struct { @@ -109,24 +109,24 @@ func RunGetAssets(ctx context.Context, f *util.Factory, out io.Writer, options * seen := map[string]bool{} for _, imageAsset := range updateClusterResults.ImageAssets { image := Image{ - Image: imageAsset.CanonicalLocation, - Mirror: imageAsset.DownloadLocation, + Canonical: imageAsset.CanonicalLocation, + Download: imageAsset.DownloadLocation, } - if !seen[image.Image] { + if !seen[image.Canonical] { result.Images = append(result.Images, &image) - seen[image.Image] = true + seen[image.Canonical] = true } } seen = map[string]bool{} for _, fileAsset := range updateClusterResults.FileAssets { file := File{ - File: fileAsset.CanonicalURL.String(), - Mirror: fileAsset.DownloadURL.String(), - SHA: fileAsset.SHAValue, + Canonical: fileAsset.CanonicalURL.String(), + Download: fileAsset.DownloadURL.String(), + SHA: fileAsset.SHAValue, } - if !seen[file.File] { + if !seen[file.Canonical] { result.Files = append(result.Files, &file) - seen[file.File] = true + seen[file.Canonical] = true } } @@ -162,30 +162,30 @@ func RunGetAssets(ctx context.Context, f *util.Factory, out io.Writer, options * func imageOutputTable(images []*Image, out io.Writer) error { fmt.Println("") t := &tables.Table{} - t.AddColumn("IMAGE", func(i *Image) string { - return i.Image + t.AddColumn("CANONICAL", func(i *Image) string { + return i.Canonical }) - t.AddColumn("MIRROR", func(i *Image) string { - return i.Mirror + t.AddColumn("DOWNLOAD", func(i *Image) string { + return i.Download }) - columns := []string{"IMAGE", "MIRROR"} + columns := []string{"CANONICAL", "DOWNLOAD"} return t.Render(images, out, columns...) } func fileOutputTable(files []*File, out io.Writer) error { fmt.Println("") t := &tables.Table{} - t.AddColumn("FILE", func(f *File) string { - return f.File + t.AddColumn("CANONICAL", func(f *File) string { + return f.Canonical }) - t.AddColumn("MIRROR", func(f *File) string { - return f.Mirror + t.AddColumn("DOWNLOAD", func(f *File) string { + return f.Download }) t.AddColumn("SHA", func(f *File) string { return f.SHA }) - columns := []string{"FILE", "MIRROR", "SHA"} + columns := []string{"CANONICAL", "DOWNLOAD", "SHA"} return t.Render(files, out, columns...) }