From a567007199047dc7f0eb6753331d370ea3353a32 Mon Sep 17 00:00:00 2001 From: Ruslan Aliev Date: Thu, 27 Aug 2020 18:57:08 -0500 Subject: [PATCH] Refactor cluster* commands All the cluster commands was refactored for usage with new config factory. Config object now is being initialized in pkg module on demand, which provides more flexibility and cleaner code. Change-Id: I1088ecbce0c34a146a646270404ff206b152d310 Signed-off-by: Ruslan Aliev --- cmd/cluster/cluster.go | 17 ++++++----------- cmd/cluster/init.go | 6 +++--- cmd/cluster/init_test.go | 5 ++++- cmd/cluster/move.go | 6 +++--- cmd/cluster/move_test.go | 8 +------- cmd/cluster/status.go | 10 +++++----- cmd/cluster/status_test.go | 11 +++++------ pkg/clusterctl/cmd/command.go | 14 +++++++------- pkg/clusterctl/cmd/command_test.go | 4 +++- pkg/k8s/client/client.go | 14 +++++++------- pkg/k8s/client/client_test.go | 10 +++------- 11 files changed, 47 insertions(+), 58 deletions(-) diff --git a/cmd/cluster/cluster.go b/cmd/cluster/cluster.go index 91e503ec5..a37ee1903 100644 --- a/cmd/cluster/cluster.go +++ b/cmd/cluster/cluster.go @@ -17,6 +17,7 @@ package cluster import ( "github.com/spf13/cobra" + "opendev.org/airship/airshipctl/pkg/config" "opendev.org/airship/airshipctl/pkg/environment" "opendev.org/airship/airshipctl/pkg/k8s/client" ) @@ -35,19 +36,13 @@ func NewClusterCommand(rootSettings *environment.AirshipCTLSettings) *cobra.Comm Use: "cluster", Short: "Manage Kubernetes clusters", Long: clusterLong[1:], - PersistentPreRun: func(cmd *cobra.Command, args []string) { - if parentPreRun := cmd.Root().PersistentPreRun; parentPreRun != nil { - parentPreRun(cmd.Root(), args) - } - - // Load or Initialize airship Config - rootSettings.InitConfig() - }, } - clusterRootCmd.AddCommand(NewInitCommand(rootSettings)) - clusterRootCmd.AddCommand(NewMoveCommand(rootSettings)) - clusterRootCmd.AddCommand(NewStatusCommand(rootSettings, client.DefaultClient)) + cfgFactory := config.CreateFactory(&rootSettings.AirshipConfigPath, &rootSettings.KubeConfigPath) + + clusterRootCmd.AddCommand(NewInitCommand(cfgFactory)) + clusterRootCmd.AddCommand(NewMoveCommand(cfgFactory)) + clusterRootCmd.AddCommand(NewStatusCommand(cfgFactory, client.DefaultClient)) return clusterRootCmd } diff --git a/cmd/cluster/init.go b/cmd/cluster/init.go index 47cf61837..0bd5eb9bd 100644 --- a/cmd/cluster/init.go +++ b/cmd/cluster/init.go @@ -20,7 +20,7 @@ import ( "github.com/spf13/cobra" clusterctlcmd "opendev.org/airship/airshipctl/pkg/clusterctl/cmd" - "opendev.org/airship/airshipctl/pkg/environment" + "opendev.org/airship/airshipctl/pkg/config" ) const ( @@ -71,14 +71,14 @@ airshipctl cluster init ) // NewInitCommand creates a command to deploy cluster-api -func NewInitCommand(rootSettings *environment.AirshipCTLSettings) *cobra.Command { +func NewInitCommand(cfgFactory config.Factory) *cobra.Command { initCmd := &cobra.Command{ Use: "init", Short: "Deploy cluster-api provider components", Long: initLong, Example: initExample, RunE: func(cmd *cobra.Command, args []string) error { - command, err := clusterctlcmd.NewCommand(rootSettings) + command, err := clusterctlcmd.NewCommand(cfgFactory) if err != nil { return err } diff --git a/cmd/cluster/init_test.go b/cmd/cluster/init_test.go index a9ee71880..d615c3ef0 100755 --- a/cmd/cluster/init_test.go +++ b/cmd/cluster/init_test.go @@ -18,6 +18,7 @@ import ( "testing" "opendev.org/airship/airshipctl/cmd/cluster" + "opendev.org/airship/airshipctl/pkg/config" "opendev.org/airship/airshipctl/pkg/environment" "opendev.org/airship/airshipctl/testutil" ) @@ -33,7 +34,9 @@ func TestNewClusterInitCmd(t *testing.T) { { Name: "cluster-init-cmd-with-help", CmdLine: "--help", - Cmd: cluster.NewInitCommand(fakeRootSettings), + Cmd: cluster.NewInitCommand(func() (*config.Config, error) { + return fakeRootSettings.Config, nil + }), }, } for _, testcase := range tests { diff --git a/cmd/cluster/move.go b/cmd/cluster/move.go index dee3197e6..93e072107 100644 --- a/cmd/cluster/move.go +++ b/cmd/cluster/move.go @@ -20,7 +20,7 @@ import ( "github.com/spf13/cobra" clusterctlcmd "opendev.org/airship/airshipctl/pkg/clusterctl/cmd" - "opendev.org/airship/airshipctl/pkg/environment" + "opendev.org/airship/airshipctl/pkg/config" ) const ( @@ -38,7 +38,7 @@ Move Cluster API objects, provider specific objects and all dependencies to the ) // NewMoveCommand creates a command to move capi and bmo resources to the target cluster -func NewMoveCommand(rootSettings *environment.AirshipCTLSettings) *cobra.Command { +func NewMoveCommand(cfgFactory config.Factory) *cobra.Command { var toKubeconfigContext string moveCmd := &cobra.Command{ Use: "move", @@ -46,7 +46,7 @@ func NewMoveCommand(rootSettings *environment.AirshipCTLSettings) *cobra.Command Long: moveLong[1:], Example: moveExample, RunE: func(cmd *cobra.Command, args []string) error { - command, err := clusterctlcmd.NewCommand(rootSettings) + command, err := clusterctlcmd.NewCommand(cfgFactory) if err != nil { return err } diff --git a/cmd/cluster/move_test.go b/cmd/cluster/move_test.go index 0ae160d2d..5386cc947 100755 --- a/cmd/cluster/move_test.go +++ b/cmd/cluster/move_test.go @@ -18,21 +18,15 @@ import ( "testing" "opendev.org/airship/airshipctl/cmd/cluster" - "opendev.org/airship/airshipctl/pkg/environment" "opendev.org/airship/airshipctl/testutil" ) func TestNewClusterMoveCmd(t *testing.T) { - fakeRootSettings := &environment.AirshipCTLSettings{ - AirshipConfigPath: "../../testdata/k8s/config.yaml", - KubeConfigPath: "../../testdata/k8s/kubeconfig.yaml", - } - tests := []*testutil.CmdTest{ { Name: "cluster-move-cmd-with-help", CmdLine: "--help", - Cmd: cluster.NewMoveCommand(fakeRootSettings), + Cmd: cluster.NewMoveCommand(nil), }, } for _, testcase := range tests { diff --git a/cmd/cluster/status.go b/cmd/cluster/status.go index ef77cd211..da412dd83 100644 --- a/cmd/cluster/status.go +++ b/cmd/cluster/status.go @@ -20,21 +20,21 @@ import ( "github.com/spf13/cobra" "opendev.org/airship/airshipctl/pkg/cluster" + "opendev.org/airship/airshipctl/pkg/config" "opendev.org/airship/airshipctl/pkg/document" - "opendev.org/airship/airshipctl/pkg/environment" "opendev.org/airship/airshipctl/pkg/k8s/client" "opendev.org/airship/airshipctl/pkg/log" "opendev.org/airship/airshipctl/pkg/util" ) // NewStatusCommand creates a command which reports the statuses of a cluster's deployed components. -func NewStatusCommand(rootSettings *environment.AirshipCTLSettings, factory client.Factory) *cobra.Command { +func NewStatusCommand(cfgFactory config.Factory, factory client.Factory) *cobra.Command { cmd := &cobra.Command{ Use: "status", Short: "Retrieve statuses of deployed cluster components", RunE: func(cmd *cobra.Command, args []string) error { - conf := rootSettings.Config - if err := conf.EnsureComplete(); err != nil { + conf, err := cfgFactory() + if err != nil { return err } @@ -53,7 +53,7 @@ func NewStatusCommand(rootSettings *environment.AirshipCTLSettings, factory clie return err } - client, err := factory(rootSettings) + client, err := factory(conf) if err != nil { return err } diff --git a/cmd/cluster/status_test.go b/cmd/cluster/status_test.go index 16af8911b..e649476f0 100644 --- a/cmd/cluster/status_test.go +++ b/cmd/cluster/status_test.go @@ -24,7 +24,6 @@ import ( "opendev.org/airship/airshipctl/cmd/cluster" "opendev.org/airship/airshipctl/pkg/config" - "opendev.org/airship/airshipctl/pkg/environment" "opendev.org/airship/airshipctl/pkg/k8s/client" "opendev.org/airship/airshipctl/pkg/k8s/client/fake" "opendev.org/airship/airshipctl/testutil" @@ -69,7 +68,7 @@ func TestNewClusterStatusCmd(t *testing.T) { for _, tt := range tests { tt := tt - testClientFactory := func(_ *environment.AirshipCTLSettings) (client.Interface, error) { + testClientFactory := func(_ *config.Config) (client.Interface, error) { return fake.NewClient( fake.WithDynamicObjects(tt.resources...), fake.WithCRDs(tt.CRDs...), @@ -80,9 +79,9 @@ func TestNewClusterStatusCmd(t *testing.T) { } } -func clusterStatusTestSettings() *environment.AirshipCTLSettings { - return &environment.AirshipCTLSettings{ - Config: &config.Config{ +func clusterStatusTestSettings() config.Factory { + return func() (*config.Config, error) { + return &config.Config{ Clusters: map[string]*config.ClusterPurpose{"testCluster": nil}, AuthInfos: map[string]*config.AuthInfo{"testAuthInfo": nil}, Contexts: map[string]*config.Context{ @@ -92,7 +91,7 @@ func clusterStatusTestSettings() *environment.AirshipCTLSettings { "testManifest": {TargetPath: fixturesPath}, }, CurrentContext: "testContext", - }, + }, nil } } diff --git a/pkg/clusterctl/cmd/command.go b/pkg/clusterctl/cmd/command.go index 5982328d9..62c6d0069 100644 --- a/pkg/clusterctl/cmd/command.go +++ b/pkg/clusterctl/cmd/command.go @@ -19,7 +19,6 @@ import ( "opendev.org/airship/airshipctl/pkg/clusterctl/client" "opendev.org/airship/airshipctl/pkg/config" "opendev.org/airship/airshipctl/pkg/document" - "opendev.org/airship/airshipctl/pkg/environment" "opendev.org/airship/airshipctl/pkg/log" ) @@ -33,15 +32,16 @@ type Command struct { } // NewCommand returns instance of Command -func NewCommand(rs *environment.AirshipCTLSettings) (*Command, error) { - bundle, err := getBundle(rs.Config) +func NewCommand(cfgFactory config.Factory) (*Command, error) { + cfg, err := cfgFactory() if err != nil { return nil, err } - if err = rs.Config.EnsureComplete(); err != nil { + bundle, err := getBundle(cfg) + if err != nil { return nil, err } - root, err := rs.Config.CurrentContextTargetPath() + root, err := cfg.CurrentContextTargetPath() if err != nil { return nil, err } @@ -53,14 +53,14 @@ func NewCommand(rs *environment.AirshipCTLSettings) (*Command, error) { if err != nil { return nil, err } - kubeConfigPath := rs.Config.KubeConfigPath() + kubeConfigPath := cfg.KubeConfigPath() return &Command{ kubeconfigPath: kubeConfigPath, documentRoot: root, client: client, options: options, - kubeconfigContext: rs.Config.CurrentContext, + kubeconfigContext: cfg.CurrentContext, }, nil } diff --git a/pkg/clusterctl/cmd/command_test.go b/pkg/clusterctl/cmd/command_test.go index 8976e5a6d..bc13ee4fe 100644 --- a/pkg/clusterctl/cmd/command_test.go +++ b/pkg/clusterctl/cmd/command_test.go @@ -120,7 +120,9 @@ func TestNewCommand(t *testing.T) { t.Run(tt.name, func(t *testing.T) { rs.Config.Manifests = manifests rs.Config.CurrentContext = context - command, err := NewCommand(rs) + command, err := NewCommand(func() (*config.Config, error) { + return rs.Config, nil + }) if expectErr { assert.Error(t, err) assert.Nil(t, command) diff --git a/pkg/k8s/client/client.go b/pkg/k8s/client/client.go index 54e362374..23862ecd2 100644 --- a/pkg/k8s/client/client.go +++ b/pkg/k8s/client/client.go @@ -22,7 +22,7 @@ import ( "k8s.io/client-go/kubernetes" "k8s.io/client-go/tools/clientcmd" - "opendev.org/airship/airshipctl/pkg/environment" + "opendev.org/airship/airshipctl/pkg/config" "opendev.org/airship/airshipctl/pkg/k8s/kubectl" k8sutils "opendev.org/airship/airshipctl/pkg/k8s/utils" ) @@ -55,19 +55,19 @@ type Client struct { var _ Interface = &Client{} // Factory is a function which creates Interfaces -type Factory func(*environment.AirshipCTLSettings) (Interface, error) +type Factory func(*config.Config) (Interface, error) // DefaultClient is a factory which generates a default client var DefaultClient Factory = NewClient // NewClient creates a Client initialized from the passed in settings -func NewClient(settings *environment.AirshipCTLSettings) (Interface, error) { +func NewClient(cfg *config.Config) (Interface, error) { client := new(Client) var err error - f := k8sutils.FactoryFromKubeConfigPath(settings.KubeConfigPath) + f := k8sutils.FactoryFromKubeConfigPath(cfg.KubeConfigPath()) - pathToBufferDir := filepath.Dir(settings.AirshipConfigPath) + pathToBufferDir := filepath.Dir(cfg.LoadedConfigPath()) client.kubectl = kubectl.NewKubectl(f).WithBufferDir(pathToBufferDir) client.clientSet, err = f.KubernetesClientSet() @@ -81,12 +81,12 @@ func NewClient(settings *environment.AirshipCTLSettings) (Interface, error) { } // kubectl factories can't create CRD clients... - config, err := clientcmd.BuildConfigFromFlags("", settings.KubeConfigPath) + kubeConfig, err := clientcmd.BuildConfigFromFlags("", cfg.KubeConfigPath()) if err != nil { return nil, err } - client.apixClient, err = apix.NewForConfig(config) + client.apixClient, err = apix.NewForConfig(kubeConfig) if err != nil { return nil, err } diff --git a/pkg/k8s/client/client_test.go b/pkg/k8s/client/client_test.go index 401eda7c0..926e6d6dd 100644 --- a/pkg/k8s/client/client_test.go +++ b/pkg/k8s/client/client_test.go @@ -21,7 +21,6 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - "opendev.org/airship/airshipctl/pkg/environment" "opendev.org/airship/airshipctl/pkg/k8s/client" "opendev.org/airship/airshipctl/testutil" ) @@ -41,13 +40,10 @@ func TestNewClient(t *testing.T) { adir, err := filepath.Abs(airshipConfigDir) require.NoError(t, err) - settings := &environment.AirshipCTLSettings{ - Config: conf, - AirshipConfigPath: adir, - KubeConfigPath: akp, - } + conf.SetLoadedConfigPath(adir) + conf.SetKubeConfigPath(akp) - client, err := client.NewClient(settings) + client, err := client.NewClient(conf) assert.NoError(t, err) assert.NotNil(t, client) assert.NotNil(t, client.ClientSet())