From d1ee5492bfca30c2996155e6373654fe6115acca Mon Sep 17 00:00:00 2001 From: Bridget McErlean Date: Wed, 18 Dec 2019 14:03:09 -0500 Subject: [PATCH] Refactor image client to not be E2E specific The existing client for handling plugin images assumed that the images were for the e2e plugin. We are trying to add support for other images so we need to make the image client more generic. This change refactors the image client to deal with image names rather than the internal Kubernetes image `Config` type. Signed-off-by: Bridget McErlean --- cmd/sonobuoy/app/images.go | 252 +++++++++++++++---------------------- pkg/image/image.go | 42 +++---- pkg/image/images_test.go | 20 +-- pkg/image/manifest.go | 39 +++--- 4 files changed, 149 insertions(+), 204 deletions(-) diff --git a/cmd/sonobuoy/app/images.go b/cmd/sonobuoy/app/images.go index 06518b761..574c52d23 100644 --- a/cmd/sonobuoy/app/images.go +++ b/cmd/sonobuoy/app/images.go @@ -20,10 +20,10 @@ import ( "fmt" "os" - "github.com/vmware-tanzu/sonobuoy/pkg/errlog" - "github.com/vmware-tanzu/sonobuoy/pkg/image" "github.com/pkg/errors" "github.com/spf13/cobra" + "github.com/vmware-tanzu/sonobuoy/pkg/errlog" + "github.com/vmware-tanzu/sonobuoy/pkg/image" ) var imagesflags imagesFlags @@ -45,8 +45,13 @@ func NewCmdImages() *cobra.Command { cmd := &cobra.Command{ Use: "images", Short: "Manage images used in a plugin. Supported plugins are: 'e2e'", - Run: listImages, - Args: cobra.ExactArgs(0), + Run: func(cmd *cobra.Command, args []string) { + if err := listImages(cmd, args); err != nil { + errlog.LogError(err) + os.Exit(1) + } + }, + Args: cobra.ExactArgs(0), } AddKubeconfigFlag(&imagesflags.kubeconfig, cmd.Flags()) @@ -56,8 +61,15 @@ func NewCmdImages() *cobra.Command { pullCmd := &cobra.Command{ Use: "pull", Short: "Pulls images to local docker client for a specific plugin", - Run: pullImages, - Args: cobra.ExactArgs(0), + Run: func(cmd *cobra.Command, args []string) { + if errs := pullImages(cmd, args); len(errs) > 0 { + for _, err := range errs { + errlog.LogError(err) + } + os.Exit(1) + } + }, + Args: cobra.ExactArgs(0), } AddE2ERegistryConfigFlag(&imagesflags.e2eRegistryConfig, pullCmd.Flags()) AddKubeconfigFlag(&imagesflags.kubeconfig, pullCmd.Flags()) @@ -67,9 +79,15 @@ func NewCmdImages() *cobra.Command { downloadCmd := &cobra.Command{ Use: "download", Short: "Saves downloaded images from local docker client to a tar file", - Run: downloadImages, - Args: cobra.ExactArgs(0), + Run: func(cmd *cobra.Command, args []string) { + if err := downloadImages(cmd, args); err != nil { + errlog.LogError(err) + os.Exit(1) + } + }, + Args: cobra.ExactArgs(0), } + AddE2ERegistryConfigFlag(&imagesflags.e2eRegistryConfig, downloadCmd.Flags()) AddKubeconfigFlag(&imagesflags.kubeconfig, downloadCmd.Flags()) AddPluginFlag(&imagesflags.plugin, downloadCmd.Flags()) @@ -77,8 +95,15 @@ func NewCmdImages() *cobra.Command { pushCmd := &cobra.Command{ Use: "push", Short: "Pushes images to docker registry for a specific plugin", - Run: pushImages, - Args: cobra.ExactArgs(0), + Run: func(cmd *cobra.Command, args []string) { + if errs := pushImages(cmd, args); len(errs) > 0 { + for _, err := range errs { + errlog.LogError(err) + } + os.Exit(1) + } + }, + Args: cobra.ExactArgs(0), } AddE2ERegistryConfigFlag(&imagesflags.e2eRegistryConfig, pushCmd.Flags()) AddKubeconfigFlag(&imagesflags.kubeconfig, pushCmd.Flags()) @@ -89,8 +114,15 @@ func NewCmdImages() *cobra.Command { deleteCmd := &cobra.Command{ Use: "delete", Short: "Deletes all images downloaded to local docker client", - Run: deleteImages, - Args: cobra.ExactArgs(0), + Run: func(cmd *cobra.Command, args []string) { + if errs := deleteImages(cmd, args); len(errs) > 0 { + for _, err := range errs { + errlog.LogError(err) + } + os.Exit(1) + } + }, + Args: cobra.ExactArgs(0), } AddE2ERegistryConfigFlag(&imagesflags.e2eRegistryConfig, deleteCmd.Flags()) AddKubeconfigFlag(&imagesflags.kubeconfig, deleteCmd.Flags()) @@ -104,119 +136,78 @@ func NewCmdImages() *cobra.Command { return cmd } -func listImages(cmd *cobra.Command, args []string) { - - switch imagesflags.plugin { - case "e2e": - - if len(imagesflags.e2eRegistryConfig) > 0 { - // Check if the e2e file exists - if _, err := os.Stat(imagesflags.e2eRegistryConfig); err != nil { - errlog.LogError(errors.Errorf("file does not exist or cannot be opened: %v", imagesflags.e2eRegistryConfig)) - os.Exit(1) - } - } +func getClusterVersion(kubeconfig Kubeconfig) (string, error) { + sbc, err := getSonobuoyClientFromKubecfg(imagesflags.kubeconfig) + if err != nil { + return "", errors.Wrap(err, "couldn't create sonobuoy client") + } - sbc, err := getSonobuoyClientFromKubecfg(imagesflags.kubeconfig) - if err != nil { - errlog.LogError(errors.Wrap(err, "could not create sonobuoy client")) - os.Exit(1) - } + version, err := sbc.Version() + if err != nil { + return "", errors.Wrap(err, "couldn't get Sonobuoy client") + } - version, err := sbc.Version() - if err != nil { - errlog.LogError(errors.Wrap(err, "couldn't get Sonobuoy client")) - os.Exit(1) - } + return version, nil +} - // Get list of images that match the version - registry, err := image.NewRegistryList("", version) +func listImages(cmd *cobra.Command, args []string) error { + switch imagesflags.plugin { + case "e2e": + version, err := getClusterVersion(imagesflags.kubeconfig) if err != nil { - errlog.LogError(errors.Wrap(err, "couldn't init Registry List")) - os.Exit(1) + return errors.Wrap(err, "failed to get cluster version") } - images, err := registry.GetImageConfigs() + images, err := image.GetE2EImages(imagesflags.e2eRegistryConfig, version) if err != nil { - errlog.LogError(errors.Wrap(err, "couldn't get images for version")) - os.Exit(1) + return errors.Wrap(err, "couldn't get images") } - for _, v := range images { - fmt.Println(v.GetE2EImage()) + for _, image := range images { + fmt.Println(image) } default: - errlog.LogError(errors.Errorf("Unsupported plugin: %v", imagesflags.plugin)) - os.Exit(1) + return errors.Errorf("Unsupported plugin: %v", imagesflags.plugin) } + + return nil } -func pullImages(cmd *cobra.Command, args []string) { +func pullImages(cmd *cobra.Command, args []string) []error { switch imagesflags.plugin { case "e2e": - - sbc, err := getSonobuoyClientFromKubecfg(imagesflags.kubeconfig) + version, err := getClusterVersion(imagesflags.kubeconfig) if err != nil { - errlog.LogError(errors.Wrap(err, "could not create sonobuoy client")) - os.Exit(1) + return []error{errors.Wrap(err, "failed to get cluster version")} } - version, err := sbc.Version() + images, err := image.GetE2EImages(imagesflags.e2eRegistryConfig, version) if err != nil { - errlog.LogError(errors.Wrap(err, "couldn't get Sonobuoy client")) - os.Exit(1) - } - - upstreamImages, err := image.GetImages(imagesflags.e2eRegistryConfig, version) - if err != nil { - errlog.LogError(errors.Wrap(err, "couldn't init upstream registry list")) - os.Exit(1) + return []error{errors.Wrap(err, "couldn't get images")} } // Init client imageClient := image.NewImageClient() // Pull all images - errs := imageClient.PullImages(upstreamImages, numDockerRetries) - for _, err := range errs { - errlog.LogError(err) - } - - if len(errs) > 0 { - os.Exit(1) - } - + return imageClient.PullImages(images, numDockerRetries) default: - errlog.LogError(errors.Errorf("Unsupported plugin: %v", imagesflags.plugin)) - os.Exit(1) + return []error{errors.Errorf("Unsupported plugin: %v", imagesflags.plugin)} } + } -func downloadImages(cmd *cobra.Command, args []string) { +func downloadImages(cmd *cobra.Command, args []string) error { switch imagesflags.plugin { case "e2e": - - sbc, err := getSonobuoyClientFromKubecfg(imagesflags.kubeconfig) + version, err := getClusterVersion(imagesflags.kubeconfig) if err != nil { - errlog.LogError(errors.Wrap(err, "could not create sonobuoy client")) - os.Exit(1) + return errors.Wrap(err, "failed to get cluster version") } - version, err := sbc.Version() + images, err := image.GetE2EImages(imagesflags.e2eRegistryConfig, version) if err != nil { - errlog.LogError(errors.Wrap(err, "couldn't get Sonobuoy client")) - os.Exit(1) - } - - upstreamImages, err := image.GetImages(defaultE2ERegistries, version) - if err != nil { - errlog.LogError(errors.Wrap(err, "couldn't init upstream registry list")) - os.Exit(1) - } - - images := []string{} - for _, v := range upstreamImages { - images = append(images, v.GetE2EImage()) + return errors.Wrap(err, "couldn't get images") } // Init client @@ -224,108 +215,65 @@ func downloadImages(cmd *cobra.Command, args []string) { fileName, err := imageClient.DownloadImages(images, version) if err != nil { - errlog.LogError(err) - os.Exit(1) + return err } fmt.Println(fileName) default: - errlog.LogError(errors.Errorf("Unsupported plugin: %v", imagesflags.plugin)) - os.Exit(1) + return errors.Errorf("Unsupported plugin: %v", imagesflags.plugin) } -} -func pushImages(cmd *cobra.Command, args []string) { + return nil +} +func pushImages(cmd *cobra.Command, args []string) []error { switch imagesflags.plugin { case "e2e": - - if len(imagesflags.e2eRegistryConfig) > 0 { - // Check if the e2e file exists - if _, err := os.Stat(imagesflags.e2eRegistryConfig); err != nil { - errlog.LogError(errors.Errorf("file does not exist or cannot be opened: %v", imagesflags.e2eRegistryConfig)) - os.Exit(1) - } - } - - // Check if the e2e file exists - if _, err := os.Stat(imagesflags.e2eRegistryConfig); os.IsNotExist(err) { - errlog.LogError(errors.Errorf("file does not exist or cannot be opened: %v", imagesflags.e2eRegistryConfig)) - os.Exit(1) - } - - sbc, err := getSonobuoyClientFromKubecfg(imagesflags.kubeconfig) - if err != nil { - errlog.LogError(errors.Wrap(err, "could not create sonobuoy client")) - os.Exit(1) - } - - version, err := sbc.Version() + version, err := getClusterVersion(imagesflags.kubeconfig) if err != nil { - errlog.LogError(errors.Wrap(err, "couldn't get Sonobuoy client")) - os.Exit(1) + return []error{errors.Wrap(err, "failed to get cluster version")} } - upstreamImages, err := image.GetImages("", version) + upstreamImages, err := image.GetE2EImages("", version) if err != nil { - errlog.LogError(errors.Wrap(err, "couldn't init upstream registry list")) - os.Exit(1) + return []error{errors.Wrap(err, "couldn't get images")} } - privateImages, err := image.GetImages(imagesflags.e2eRegistryConfig, version) + privateImages, err := image.GetE2EImages(imagesflags.e2eRegistryConfig, version) if err != nil { - errlog.LogError(errors.Wrap(err, "couldn't init upstream registry list")) - os.Exit(1) + return []error{errors.Wrap(err, "couldn't get images")} } // Init client imageClient := image.NewImageClient() // Push all images - errs := imageClient.PushImages(upstreamImages, privateImages, numDockerRetries) - for _, err := range errs { - errlog.LogError(err) - } - + return imageClient.PushImages(upstreamImages, privateImages, numDockerRetries) default: - errlog.LogError(errors.Errorf("Unsupported plugin: %v", imagesflags.plugin)) - os.Exit(1) + return []error{errors.Errorf("Unsupported plugin: %v", imagesflags.plugin)} } - } -func deleteImages(cmd *cobra.Command, args []string) { +func deleteImages(cmd *cobra.Command, args []string) []error { switch imagesflags.plugin { case "e2e": - sbc, err := getSonobuoyClientFromKubecfg(imagesflags.kubeconfig) + version, err := getClusterVersion(imagesflags.kubeconfig) if err != nil { - errlog.LogError(errors.Wrap(err, "could not create sonobuoy client")) - os.Exit(1) + return []error{errors.Wrap(err, "failed to get cluster version")} } - version, err := sbc.Version() + images, err := image.GetE2EImages(imagesflags.e2eRegistryConfig, version) if err != nil { - errlog.LogError(errors.Wrap(err, "couldn't get Sonobuoy client")) - os.Exit(1) - } - - images, err := image.GetImages(imagesflags.e2eRegistryConfig, version) - if err != nil { - errlog.LogError(errors.Wrap(err, "couldn't init registry list")) - os.Exit(1) + return []error{errors.Wrap(err, "couldn't get images")} } // Init client imageClient := image.NewImageClient() - errs := imageClient.DeleteImages(images, numDockerRetries) - for _, err := range errs { - errlog.LogError(err) - } + return imageClient.DeleteImages(images, numDockerRetries) default: - errlog.LogError(errors.Errorf("Unsupported plugin: %v", imagesflags.plugin)) - os.Exit(1) + return []error{errors.Errorf("Unsupported plugin: %v", imagesflags.plugin)} } } diff --git a/pkg/image/image.go b/pkg/image/image.go index 7469e3bb2..fe91344a1 100644 --- a/pkg/image/image.go +++ b/pkg/image/image.go @@ -19,8 +19,8 @@ package image import ( "fmt" - "github.com/vmware-tanzu/sonobuoy/pkg/image/docker" "github.com/pkg/errors" + "github.com/vmware-tanzu/sonobuoy/pkg/image/docker" ) type ImageClient struct { @@ -33,36 +33,36 @@ func NewImageClient() ImageClient { } } -func (i ImageClient) PullImages(images map[string]Config, retries int) []error { +func (i ImageClient) PullImages(images []string, retries int) []error { errs := []error{} - for _, v := range images { - err := i.dockerClient.PullIfNotPresent(v.GetE2EImage(), retries) + for _, image := range images { + err := i.dockerClient.PullIfNotPresent(image, retries) if err != nil { - errs = append(errs, errors.Wrapf(err, "couldn't pull image: %v", v.GetE2EImage())) + errs = append(errs, errors.Wrapf(err, "couldn't pull image: %v", image)) } } return errs } -func (i ImageClient) PushImages(upstreamImages, privateImages map[string]Config, retries int) []error { +func (i ImageClient) PushImages(upstreamImages, privateImages []string, retries int) []error { errs := []error{} - for k, v := range upstreamImages { + for k, upstreamImg := range upstreamImages { privateImg := privateImages[k] // Skip if the source/dest are equal - if privateImg.GetE2EImage() == v.GetE2EImage() { - fmt.Printf("Skipping public image: %s\n", v.GetE2EImage()) + if privateImg == upstreamImg { + fmt.Printf("Skipping public image: %s\n", upstreamImg) continue } - err := i.dockerClient.Tag(v.GetE2EImage(), privateImg.GetE2EImage(), retries) + err := i.dockerClient.Tag(upstreamImg, privateImg, retries) if err != nil { - errs = append(errs, errors.Wrapf(err, "couldn't tag image: %v", v.GetE2EImage())) + errs = append(errs, errors.Wrapf(err, "couldn't tag image %q as %q", upstreamImg, privateImg)) } - err = i.dockerClient.Push(privateImg.GetE2EImage(), retries) + err = i.dockerClient.Push(privateImg, retries) if err != nil { - errs = append(errs, errors.Wrapf(err, "couldn't push image: %v", v.GetE2EImage())) + errs = append(errs, errors.Wrapf(err, "couldn't push image: %v", privateImg)) } } return errs @@ -79,28 +79,28 @@ func (i ImageClient) DownloadImages(images []string, version string) (string, er return fileName, nil } -func (i ImageClient) DeleteImages(images map[string]Config, retries int) []error { +func (i ImageClient) DeleteImages(images []string, retries int) []error { errs := []error{} - for _, v := range images { - err := i.dockerClient.Rmi(v.GetE2EImage(), retries) + for _, image := range images { + err := i.dockerClient.Rmi(image, retries) if err != nil { - errs = append(errs, errors.Wrapf(err, "couldn't delete image: %v", v.GetE2EImage())) + errs = append(errs, errors.Wrapf(err, "couldn't delete image: %v", image)) } } return errs } -// GetImages gets a map of image Configs -func GetImages(e2eRegistryConfig, version string) (map[string]Config, error) { +// GetE2EImages gets a map of E2E image names +func GetE2EImages(e2eRegistryConfig, version string) ([]string, error) { // Get list of upstream images that match the version reg, err := NewRegistryList(e2eRegistryConfig, version) if err != nil { - return nil, errors.Wrap(err, "couldn't init Registry List") + return nil, errors.Wrap(err, "couldn't create image registry list") } - imgs, err := reg.GetImageConfigs() + imgs, err := reg.GetImageNames() if err != nil { return nil, errors.Wrap(err, "couldn't get images for version") } diff --git a/pkg/image/images_test.go b/pkg/image/images_test.go index d165360da..fbf036704 100644 --- a/pkg/image/images_test.go +++ b/pkg/image/images_test.go @@ -19,17 +19,11 @@ package image import ( "testing" - "github.com/vmware-tanzu/sonobuoy/pkg/image/docker" "github.com/pkg/errors" + "github.com/vmware-tanzu/sonobuoy/pkg/image/docker" ) -var imgs = map[string]Config{ - "test": Config{ - name: "test1", - registry: "foo.io/sonobuoy", - version: "x.y", - }, -} +var imgs = []string{"test1/foo.io/sonobuoy:x.y"} type FakeDockerClient struct { imageExists bool @@ -83,17 +77,11 @@ func (l FakeDockerClient) Save(images []string, filename string) error { } func TestPushImages(t *testing.T) { - var privateImgs = map[string]Config{ - "test": Config{ - name: "test1", - registry: "private.io/sonobuoy", - version: "x.y", - }, - } + var privateImgs = []string{"test1/private.io/sonobuoy:x.y"} tests := map[string]struct { client docker.Docker - privateImgs map[string]Config + privateImgs []string wantErrorCount int }{ "simple": { diff --git a/pkg/image/manifest.go b/pkg/image/manifest.go index a54b642b8..a051a3beb 100644 --- a/pkg/image/manifest.go +++ b/pkg/image/manifest.go @@ -59,11 +59,11 @@ type RegistryList struct { Images map[int]Config `yaml:"-"` } -// Config holds an images registry, name, and version +// Config holds an image's fully qualified name components registry, name, and tag type Config struct { registry string name string - version string + tag string } // NewRegistryList returns a default registry or one that matches a config file passed @@ -89,12 +89,12 @@ func NewRegistryList(repoConfig, k8sVersion string) (*RegistryList, error) { fileContent, err := ioutil.ReadFile(repoConfig) if err != nil { - panic(fmt.Errorf("Error reading '%v' file contents: %v", repoConfig, err)) + return nil, fmt.Errorf("Error reading '%v' file contents: %v", repoConfig, err) } err = yaml.Unmarshal(fileContent, ®istry) if err != nil { - panic(fmt.Errorf("Error unmarshalling '%v' YAML file: %v", repoConfig, err)) + return nil, fmt.Errorf("Error unmarshalling '%v' YAML file: %v", repoConfig, err) } } @@ -109,24 +109,33 @@ func NewRegistryList(repoConfig, k8sVersion string) (*RegistryList, error) { return registry, nil } -// GetImageConfigs returns the map of imageConfigs -func (r *RegistryList) GetImageConfigs() (map[string]Config, error) { +// GetImageNames returns the map of image Config +func (r *RegistryList) GetImageNames() ([]string, error) { + imgConfigs := map[string]Config{} switch r.K8sVersion.Segments()[0] { case 1: switch r.K8sVersion.Segments()[1] { case 13: - return r.v1_13(), nil + imgConfigs = r.v1_13() case 14: - return r.v1_14(), nil + imgConfigs = r.v1_14() case 15: - return r.v1_15(), nil + imgConfigs = r.v1_15() case 16: - return r.v1_16(), nil + imgConfigs = r.v1_16() case 17: - return r.v1_17(), nil + imgConfigs = r.v1_17() + default: + return []string{}, fmt.Errorf("No matching configuration for k8s version: %v", r.K8sVersion) } } - return map[string]Config{}, fmt.Errorf("No matching configuration for k8s version: %v", r.K8sVersion) + + imageNames := []string{} + for _, imageConfig := range imgConfigs { + imageNames = append(imageNames, imageConfig.GetFullyQualifiedImageName()) + + } + return imageNames, nil } // GetDefaultImageRegistries returns the default default image registries used for @@ -191,7 +200,7 @@ func GetDefaultImageRegistries(version string) (*RegistryList, error) { return nil, fmt.Errorf("No matching configuration for k8s version: %v", v) } -// GetE2EImage returns the fully qualified URI to an image (including version) -func (i *Config) GetE2EImage() string { - return fmt.Sprintf("%s/%s:%s", i.registry, i.name, i.version) +// GetFullyQualifiedImageName returns the fully qualified URI to an image (including tag) +func (i *Config) GetFullyQualifiedImageName() string { + return fmt.Sprintf("%s/%s:%s", i.registry, i.name, i.tag) }