internal/olm: refactor package internal/operator and run packagemanifests logic

internal/cmd/operator-sdk/cleanup/cmd.go

@@ -21,7 +21,7 @@ import (
 	log "github.com/sirupsen/logrus"
 	"github.com/spf13/cobra"
 
-	"github.com/operator-framework/operator-sdk/internal/operator"
+	"github.com/operator-framework/operator-sdk/internal/olm/operator"
 )
 
 func NewCmd() *cobra.Command {

internal/cmd/operator-sdk/olm/install.go

@@ -15,14 +15,14 @@
 package olm
 
 import (
-	"github.com/operator-framework/operator-sdk/internal/olm"
+	"github.com/operator-framework/operator-sdk/internal/olm/installer"
 
 	log "github.com/sirupsen/logrus"
 	"github.com/spf13/cobra"
 )
 
 func newInstallCmd() *cobra.Command {
-	mgr := &olm.Manager{}
+	mgr := &installer.Manager{}
 	cmd := &cobra.Command{
 		Use:   "install",
 		Short: "Install Operator Lifecycle Manager in your cluster",
@@ -34,7 +34,7 @@ func newInstallCmd() *cobra.Command {
 		},
 	}
 
-	cmd.Flags().StringVar(&mgr.Version, "version", olm.DefaultVersion, "version of OLM resources to install")
+	cmd.Flags().StringVar(&mgr.Version, "version", installer.DefaultVersion, "version of OLM resources to install")
 	mgr.AddToFlagSet(cmd.Flags())
 	return cmd
 }

internal/cmd/operator-sdk/olm/install_test.go

@@ -17,7 +17,8 @@ package olm
 import (
 	. "github.com/onsi/ginkgo"
 	. "github.com/onsi/gomega"
-	"github.com/operator-framework/operator-sdk/internal/olm"
+
+	"github.com/operator-framework/operator-sdk/internal/olm/installer"
 )
 
 var _ = Describe("Running an olm install command", func() {
@@ -30,7 +31,7 @@ var _ = Describe("Running an olm install command", func() {
 
 			flag := cmd.Flags().Lookup("version")
 			Expect(flag).NotTo(BeNil())
-			Expect(flag.DefValue).To(Equal(olm.DefaultVersion))
+			Expect(flag.DefValue).To(Equal(installer.DefaultVersion))
 			Expect(flag.Usage).NotTo(BeNil())
 		})
 	})

internal/cmd/operator-sdk/olm/status.go

@@ -15,14 +15,14 @@
 package olm
 
 import (
-	"github.com/operator-framework/operator-sdk/internal/olm"
-
 	log "github.com/sirupsen/logrus"
 	"github.com/spf13/cobra"
+
+	"github.com/operator-framework/operator-sdk/internal/olm/installer"
 )
 
 func newStatusCmd() *cobra.Command {
-	mgr := olm.Manager{}
+	mgr := installer.Manager{}
 	cmd := &cobra.Command{
 		Use:   "status",
 		Short: "Get the status of the Operator Lifecycle Manager installation in your cluster",
@@ -34,7 +34,7 @@ func newStatusCmd() *cobra.Command {
 		},
 	}
 
-	cmd.Flags().StringVar(&mgr.OLMNamespace, "olm-namespace", olm.DefaultOLMNamespace, "namespace where OLM is installed")
+	cmd.Flags().StringVar(&mgr.OLMNamespace, "olm-namespace", installer.DefaultOLMNamespace, "namespace where OLM is installed")
 	cmd.Flags().StringVar(&mgr.Version, "version", "", "version of OLM installed on cluster; if unset"+
 		"operator-sdk attempts to auto-discover the version")
 	mgr.AddToFlagSet(cmd.Flags())

internal/cmd/operator-sdk/olm/status_test.go

@@ -17,7 +17,8 @@ package olm
 import (
 	. "github.com/onsi/ginkgo"
 	. "github.com/onsi/gomega"
-	"github.com/operator-framework/operator-sdk/internal/olm"
+
+	"github.com/operator-framework/operator-sdk/internal/olm/installer"
 )
 
 var _ = Describe("Running an olm status command", func() {
@@ -30,7 +31,7 @@ var _ = Describe("Running an olm status command", func() {
 
 			flag := cmd.Flags().Lookup("olm-namespace")
 			Expect(flag).NotTo(BeNil())
-			Expect(flag.DefValue).To(Equal(olm.DefaultOLMNamespace))
+			Expect(flag.DefValue).To(Equal(installer.DefaultOLMNamespace))
 			Expect(flag.Usage).NotTo(BeNil())
 
 			flag = cmd.Flags().Lookup("version")

internal/cmd/operator-sdk/olm/uninstall.go

@@ -15,14 +15,14 @@
 package olm
 
 import (
-	"github.com/operator-framework/operator-sdk/internal/olm"
+	"github.com/operator-framework/operator-sdk/internal/olm/installer"
 
 	log "github.com/sirupsen/logrus"
 	"github.com/spf13/cobra"
 )
 
 func newUninstallCmd() *cobra.Command {
-	mgr := olm.Manager{}
+	mgr := installer.Manager{}
 	cmd := &cobra.Command{
 		Use:   "uninstall",
 		Short: "Uninstall Operator Lifecycle Manager from your cluster",
@@ -35,7 +35,7 @@ func newUninstallCmd() *cobra.Command {
 	}
 
 	cmd.Flags().StringVar(&mgr.Version, "version", "", "version of OLM resources to uninstall.")
-	cmd.Flags().StringVar(&mgr.OLMNamespace, "olm-namespace", olm.DefaultOLMNamespace,
+	cmd.Flags().StringVar(&mgr.OLMNamespace, "olm-namespace", installer.DefaultOLMNamespace,
 		"namespace from where OLM is to be uninstalled.")
 	mgr.AddToFlagSet(cmd.Flags())
 	return cmd

internal/cmd/operator-sdk/olm/uninstall_test.go

@@ -17,7 +17,7 @@ package olm
 import (
 	. "github.com/onsi/ginkgo"
 	. "github.com/onsi/gomega"
-	"github.com/operator-framework/operator-sdk/internal/olm"
+	"github.com/operator-framework/operator-sdk/internal/olm/installer"
 )
 
 var _ = Describe("Running an olm uninstall command", func() {
@@ -35,7 +35,7 @@ var _ = Describe("Running an olm uninstall command", func() {
 
 			flag = cmd.Flags().Lookup("olm-namespace")
 			Expect(flag).NotTo(BeNil())
-			Expect(flag.DefValue).To(Equal(olm.DefaultOLMNamespace))
+			Expect(flag.DefValue).To(Equal(installer.DefaultOLMNamespace))
 			Expect(flag.Usage).NotTo(BeNil())
 		})
 	})

internal/cmd/operator-sdk/run/bundle/cmd.go

@@ -21,18 +21,13 @@ import (
 	"github.com/sirupsen/logrus"
 	"github.com/spf13/cobra"
 
-	"github.com/operator-framework/operator-sdk/internal/operator"
-	"github.com/operator-framework/operator-sdk/internal/operator/bundle"
+	"github.com/operator-framework/operator-sdk/internal/olm/operator"
+	"github.com/operator-framework/operator-sdk/internal/olm/operator/bundle"
 )
 
-func NewCmd() *cobra.Command {
+func NewCmd(cfg *operator.Configuration) *cobra.Command {
 	var timeout time.Duration
 
-	// TODO(joelanford): move the initialization of cfg up to
-	//   the "run" subcommand when migrating packagemanifests
-	//   to this design.
-	cfg := &operator.Configuration{}
-
 	i := bundle.NewInstall(cfg)
 	cmd := &cobra.Command{
 		Use:   "bundle <bundle-image>",
@@ -48,11 +43,10 @@ func NewCmd() *cobra.Command {
 			i.BundleImage = args[0]
 
 			// TODO(joelanford): Add cleanup logic if this fails?
-			csv, err := i.Run(ctx)
+			_, err := i.Run(ctx)
 			if err != nil {
 				logrus.Fatalf("Failed to run bundle: %v\n", err)
 			}
-			logrus.Infof("CSV %q installed\n", csv.Name)
 		},
 	}
 	cmd.Flags().SortFlags = false

internal/cmd/operator-sdk/run/cmd.go

@@ -18,6 +18,7 @@ import (
 	"github.com/spf13/cobra"
 
 	"github.com/operator-framework/operator-sdk/internal/cmd/operator-sdk/run/packagemanifests"
+	"github.com/operator-framework/operator-sdk/internal/olm/operator"
 )
 
 func NewCmd() *cobra.Command {
@@ -29,10 +30,12 @@ func NewCmd() *cobra.Command {
 Currently only the package manifests format is supported via the 'packagemanifests' subcommand.`,
 	}
 
+	cfg := &operator.Configuration{}
+
 	cmd.AddCommand(
 		// TODO(joelanford): enable bundle command when implementation is complete
 		//bundle.NewCmd(),
-		packagemanifests.NewCmd(),
+		packagemanifests.NewCmd(cfg),
 	)
 
 	return cmd

internal/cmd/operator-sdk/run/cmd_test.go

@@ -30,7 +30,7 @@ var _ = Describe("Running a run command", func() {
 
 			subcommands := cmd.Commands()
 			Expect(len(subcommands)).To(Equal(1))
-			Expect(subcommands[0].Use).To(Equal("packagemanifests <packagemanifests-root-dir>"))
+			Expect(subcommands[0].Use).To(Equal("packagemanifests [packagemanifests-root-dir]"))
 		})
 	})
 })

internal/cmd/operator-sdk/run/packagemanifests/packagemanifests.go

@@ -15,64 +15,50 @@
 package packagemanifests
 
 import (
-	"fmt"
+	"context"
+	"time"
 
 	log "github.com/sirupsen/logrus"
 	"github.com/spf13/cobra"
 
-	olmoperator "github.com/operator-framework/operator-sdk/internal/olm/operator"
+	"github.com/operator-framework/operator-sdk/internal/olm/operator"
+	"github.com/operator-framework/operator-sdk/internal/olm/operator/packagemanifests"
 )
 
-type packagemanifestsCmd struct {
-	olmoperator.PackageManifestsCmd
-}
-
-func NewCmd() *cobra.Command {
-	c := &packagemanifestsCmd{}
+func NewCmd(cfg *operator.Configuration) *cobra.Command {
+	var timeout time.Duration
 
+	i := packagemanifests.NewInstall(cfg)
 	cmd := &cobra.Command{
-		Use:   "packagemanifests <packagemanifests-root-dir>",
+		Use:   "packagemanifests [packagemanifests-root-dir]",
 		Short: "Deploy an Operator in the package manifests format with OLM",
 		Long: `'run packagemanifests' deploys an Operator's package manifests with OLM. The command's argument
-must be set to a valid package manifests root directory, ex. '<project-root>/packagemanifests'.`,
-		Aliases: []string{"pm"},
-		PreRunE: func(cmd *cobra.Command, args []string) error {
-			err := c.validate(args)
-			if err != nil {
-				log.Fatalf("Failed to validate input: %v", err)
+will default to './packagemanifests' if unset; if set, the argument must be a package manifests root directory,
+ex. '<project-root>/packagemanifests'.`,
+		Aliases:           []string{"pm"},
+		Args:              cobra.MaximumNArgs(1),
+		PersistentPreRunE: func(_ *cobra.Command, _ []string) error { return cfg.Load() },
+		Run: func(cmd *cobra.Command, args []string) {
+			ctx, cancel := context.WithTimeout(cmd.Context(), timeout)
+			defer cancel()
+
+			if len(args) == 0 {
+				i.PackageManifestsDirectory = "packagemanifests"
+			} else {
+				i.PackageManifestsDirectory = args[0]
 			}
-			c.setDefaults(args)
-			return nil
-		},
-		RunE: func(cmd *cobra.Command, args []string) error {
-			log.Infof("Running operator from directory %s", c.ManifestsDir)
 
-			if err := c.Run(); err != nil {
-				log.Fatalf("Failed to run operator: %v", err)
+			// TODO(joelanford): Add cleanup logic if this fails?
+			_, err := i.Run(ctx)
+			if err != nil {
+				log.Fatalf("Failed to run packagemanifests: %v\n", err)
 			}
-			return nil
 		},
 	}
+	cmd.Flags().SortFlags = false
+	cfg.BindFlags(cmd.PersistentFlags())
+	i.BindFlags(cmd.Flags())
 
-	c.PackageManifestsCmd.AddToFlagSet(cmd.Flags())
-
+	cmd.Flags().DurationVar(&timeout, "timeout", 2*time.Minute, "install timeout")
 	return cmd
 }
-
-func (c *packagemanifestsCmd) validate(args []string) error {
-	if len(args) > 0 {
-		if len(args) > 1 {
-			return fmt.Errorf("exactly one argument is required")
-		}
-	}
-
-	return nil
-}
-
-func (c *packagemanifestsCmd) setDefaults(args []string) {
-	if len(args) != 0 {
-		c.ManifestsDir = args[0]
-	} else {
-		c.ManifestsDir = "packagemanifests"
-	}
-}

internal/cmd/operator-sdk/run/packagemanifests/packagemanifests_test.go

@@ -17,12 +17,15 @@ package packagemanifests
 import (
 	. "github.com/onsi/ginkgo"
 	. "github.com/onsi/gomega"
+
+	"github.com/operator-framework/operator-sdk/internal/olm/operator"
 )
 
 var _ = Describe("Running a run packagemanifests command", func() {
 	Describe("NewCmd", func() {
 		It("builds a cobra command", func() {
-			cmd := NewCmd()
+			cfg := &operator.Configuration{}
+			cmd := NewCmd(cfg)
 			Expect(cmd).NotTo(BeNil())
 			Expect(cmd.Use).NotTo(BeNil())
 			Expect(cmd.Short).NotTo(BeNil())
@@ -32,39 +35,4 @@ var _ = Describe("Running a run packagemanifests command", func() {
 			Expect(aliases[0]).To(Equal("pm"))
 		})
 	})
-	Describe("validate", func() {
-		var (
-			c   packagemanifestsCmd
-			err error
-		)
-		BeforeEach(func() {
-			c = packagemanifestsCmd{}
-		})
-		It("fails if provided more than 1 arg", func() {
-			err = c.validate([]string{"foo", "bar"})
-			Expect(err).NotTo(BeNil())
-			Expect(err.Error()).To(ContainSubstring("exactly one argument is required"))
-		})
-		It("succeeds and if exactly 1 arg is provided", func() {
-			arg := "baz"
-			err = c.validate([]string{arg})
-			Expect(err).To(BeNil())
-		})
-	})
-	Describe("setDefaults", func() {
-		var (
-			c packagemanifestsCmd
-		)
-		BeforeEach(func() {
-			c = packagemanifestsCmd{}
-		})
-		It("defaults to 'packagemanifests' if no args are provided", func() {
-			c.setDefaults([]string{})
-			Expect(c.ManifestsDir).To(Equal("packagemanifests"))
-		})
-		It("sets ManifestDir to the first arg if provided more than 0", func() {
-			c.setDefaults([]string{"config/potato"})
-			Expect(c.ManifestsDir).To(Equal("config/potato"))
-		})
-	})
 })

internal/olm/client/client.go

@@ -15,7 +15,7 @@
 // Package olm provides an API to install, uninstall, and check the
 // status of an Operator Lifecycle Manager installation.
 // TODO: move to OLM repository?
-package olm
+package client
 
 import (
 	"context"
@@ -56,7 +56,7 @@ type Client struct {
 	KubeClient client.Client
 }
 
-func ClientForConfig(cfg *rest.Config) (*Client, error) {
+func NewClientForConfig(cfg *rest.Config) (*Client, error) {
 	rm, err := apiutil.NewDynamicRESTMapper(cfg)
 	if err != nil {
 		return nil, fmt.Errorf("failed to create dynamic rest mapper: %v", err)

internal/olm/client/status.go

@@ -15,7 +15,7 @@
 // Package olm provides an API to install, uninstall, and check the
 // status of an Operator Lifecycle Manager installation.
 // TODO: move to OLM repository?
-package olm
+package client
 
 import (
 	"bytes"