From e81116b1a984ca6332209703a20799624371e168 Mon Sep 17 00:00:00 2001 From: Ruslan Aliev Date: Thu, 27 Aug 2020 18:48:12 -0500 Subject: [PATCH] Refactor image* commands All the image 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: I3c51f201ec70b194ce1f127ff43a28292cf691f3 Signed-off-by: Ruslan Aliev Relates-To: #327 --- cmd/image/build.go | 6 +- cmd/image/image.go | 14 ++--- cmd/image/image_test.go | 3 +- pkg/bootstrap/isogen/command.go | 7 +-- pkg/bootstrap/isogen/command_test.go | 90 ++++++++++++---------------- 5 files changed, 51 insertions(+), 69 deletions(-) diff --git a/cmd/image/build.go b/cmd/image/build.go index f4aca7b57..5b4a15109 100644 --- a/cmd/image/build.go +++ b/cmd/image/build.go @@ -18,16 +18,16 @@ import ( "github.com/spf13/cobra" "opendev.org/airship/airshipctl/pkg/bootstrap/isogen" - "opendev.org/airship/airshipctl/pkg/environment" + "opendev.org/airship/airshipctl/pkg/config" ) // NewImageBuildCommand creates a new command with the capability to build an ISO image. -func NewImageBuildCommand(rootSettings *environment.AirshipCTLSettings) *cobra.Command { +func NewImageBuildCommand(cfgFactory config.Factory) *cobra.Command { cmd := &cobra.Command{ Use: "build", Short: "Build ISO image", RunE: func(cmd *cobra.Command, args []string) error { - return isogen.GenerateBootstrapIso(rootSettings) + return isogen.GenerateBootstrapIso(cfgFactory) }, } diff --git a/cmd/image/image.go b/cmd/image/image.go index ab964074a..10960e710 100644 --- a/cmd/image/image.go +++ b/cmd/image/image.go @@ -17,6 +17,7 @@ package image import ( "github.com/spf13/cobra" + "opendev.org/airship/airshipctl/pkg/config" "opendev.org/airship/airshipctl/pkg/environment" ) @@ -25,18 +26,11 @@ func NewImageCommand(rootSettings *environment.AirshipCTLSettings) *cobra.Comman imageRootCmd := &cobra.Command{ Use: "image", Short: "Manage ISO image creation", - 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() - }, } - imageBuildCmd := NewImageBuildCommand(rootSettings) - imageRootCmd.AddCommand(imageBuildCmd) + cfgFactory := config.CreateFactory(&rootSettings.AirshipConfigPath, &rootSettings.KubeConfigPath) + + imageRootCmd.AddCommand(NewImageBuildCommand(cfgFactory)) return imageRootCmd } diff --git a/cmd/image/image_test.go b/cmd/image/image_test.go index 5d919d878..2582a3f28 100644 --- a/cmd/image/image_test.go +++ b/cmd/image/image_test.go @@ -18,6 +18,7 @@ import ( "testing" "opendev.org/airship/airshipctl/cmd/image" + "opendev.org/airship/airshipctl/pkg/environment" "opendev.org/airship/airshipctl/testutil" ) @@ -26,7 +27,7 @@ func TestImage(t *testing.T) { { Name: "image-with-help", CmdLine: "-h", - Cmd: image.NewImageCommand(nil), + Cmd: image.NewImageCommand(&environment.AirshipCTLSettings{}), }, } diff --git a/pkg/bootstrap/isogen/command.go b/pkg/bootstrap/isogen/command.go index ef3ca1db8..e1f4e3926 100644 --- a/pkg/bootstrap/isogen/command.go +++ b/pkg/bootstrap/isogen/command.go @@ -26,7 +26,6 @@ import ( "opendev.org/airship/airshipctl/pkg/config" "opendev.org/airship/airshipctl/pkg/container" "opendev.org/airship/airshipctl/pkg/document" - "opendev.org/airship/airshipctl/pkg/environment" "opendev.org/airship/airshipctl/pkg/log" "opendev.org/airship/airshipctl/pkg/util" ) @@ -36,11 +35,11 @@ const ( ) // GenerateBootstrapIso will generate data for cloud init and start ISO builder container -func GenerateBootstrapIso(settings *environment.AirshipCTLSettings) error { +func GenerateBootstrapIso(cfgFactory config.Factory) error { ctx := context.Background() - globalConf := settings.Config - if err := globalConf.EnsureComplete(); err != nil { + globalConf, err := cfgFactory() + if err != nil { return err } diff --git a/pkg/bootstrap/isogen/command_test.go b/pkg/bootstrap/isogen/command_test.go index 30c9b209e..7a0a27498 100644 --- a/pkg/bootstrap/isogen/command_test.go +++ b/pkg/bootstrap/isogen/command_test.go @@ -21,8 +21,6 @@ import ( "strings" "testing" - "opendev.org/airship/airshipctl/pkg/environment" - "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -232,82 +230,72 @@ func TestVerifyInputs(t *testing.T) { } func TestGenerateBootstrapIso(t *testing.T) { + airshipConfigPath := "testdata/config/config" + kubeConfigPath := "testdata/config/kubeconfig" t.Run("EnsureCompleteError", func(t *testing.T) { - settings := &environment.AirshipCTLSettings{ - AirshipConfigPath: "testdata/config/config", - KubeConfigPath: "testdata/config/kubeconfig", - Config: &config.Config{}, - } - expectedErr := config.ErrMissingConfig{What: "Current Context is not defined"} - settings.InitConfig() - settings.Config.CurrentContext = "" - actualErr := GenerateBootstrapIso(settings) + settings, err := config.CreateFactory(&airshipConfigPath, &kubeConfigPath)() + require.NoError(t, err) + expectedErr := config.ErrMissingConfig{What: "Context with name ''"} + settings.CurrentContext = "" + actualErr := GenerateBootstrapIso(func() (*config.Config, error) { + return settings, nil + }) assert.Equal(t, expectedErr, actualErr) }) t.Run("ContextEntryPointError", func(t *testing.T) { - settings := &environment.AirshipCTLSettings{ - AirshipConfigPath: "testdata/config/config", - KubeConfigPath: "testdata/config/kubeconfig", - Config: &config.Config{}, - } + settings, err := config.CreateFactory(&airshipConfigPath, &kubeConfigPath)() + require.NoError(t, err) expectedErr := config.ErrMissingPrimaryRepo{} - settings.InitConfig() - settings.Config.Manifests["default"].Repositories = make(map[string]*config.Repository) - actualErr := GenerateBootstrapIso(settings) + settings.Manifests["default"].Repositories = make(map[string]*config.Repository) + actualErr := GenerateBootstrapIso(func() (*config.Config, error) { + return settings, nil + }) assert.Equal(t, expectedErr, actualErr) }) t.Run("NewBundleByPathError", func(t *testing.T) { - settings := &environment.AirshipCTLSettings{ - AirshipConfigPath: "testdata/config/config", - KubeConfigPath: "testdata/config/kubeconfig", - Config: &config.Config{}, - } + settings, err := config.CreateFactory(&airshipConfigPath, &kubeConfigPath)() + require.NoError(t, err) expectedErr := config.ErrMissingPhaseDocument{PhaseName: "bootstrap"} - settings.InitConfig() - settings.Config.Manifests["default"].TargetPath = "/nonexistent" - actualErr := GenerateBootstrapIso(settings) + settings.Manifests["default"].TargetPath = "/nonexistent" + actualErr := GenerateBootstrapIso(func() (*config.Config, error) { + return settings, nil + }) assert.Equal(t, expectedErr, actualErr) }) t.Run("SelectOneError", func(t *testing.T) { - settings := &environment.AirshipCTLSettings{ - AirshipConfigPath: "testdata/config/config", - KubeConfigPath: "testdata/config/kubeconfig", - Config: &config.Config{}, - } + settings, err := config.CreateFactory(&airshipConfigPath, &kubeConfigPath)() + require.NoError(t, err) expectedErr := document.ErrDocNotFound{ Selector: document.NewSelector().ByGvk("airshipit.org", "v1alpha1", "ImageConfiguration")} - settings.InitConfig() - settings.Config.Manifests["default"].SubPath = "missingkinddoc/site/test-site" - actualErr := GenerateBootstrapIso(settings) + settings.Manifests["default"].SubPath = "missingkinddoc/site/test-site" + actualErr := GenerateBootstrapIso(func() (*config.Config, error) { + return settings, nil + }) assert.Equal(t, expectedErr, actualErr) }) t.Run("ToObjectError", func(t *testing.T) { - settings := &environment.AirshipCTLSettings{ - AirshipConfigPath: "testdata/config/config", - KubeConfigPath: "testdata/config/kubeconfig", - Config: &config.Config{}, - } + settings, err := config.CreateFactory(&airshipConfigPath, &kubeConfigPath)() + require.NoError(t, err) expectedErrMessage := "missing metadata.name in object" - settings.InitConfig() - settings.Config.Manifests["default"].SubPath = "missingmetadoc/site/test-site" - actualErr := GenerateBootstrapIso(settings) + settings.Manifests["default"].SubPath = "missingmetadoc/site/test-site" + actualErr := GenerateBootstrapIso(func() (*config.Config, error) { + return settings, nil + }) assert.Contains(t, actualErr.Error(), expectedErrMessage) }) t.Run("verifyInputsError", func(t *testing.T) { - settings := &environment.AirshipCTLSettings{ - AirshipConfigPath: "testdata/config/config", - KubeConfigPath: "testdata/config/kubeconfig", - Config: &config.Config{}, - } + settings, err := config.CreateFactory(&airshipConfigPath, &kubeConfigPath)() + require.NoError(t, err) expectedErr := config.ErrMissingConfig{What: "Must specify volume bind for ISO builder container"} - settings.InitConfig() - settings.Config.Manifests["default"].SubPath = "missingvoldoc/site/test-site" - actualErr := GenerateBootstrapIso(settings) + settings.Manifests["default"].SubPath = "missingvoldoc/site/test-site" + actualErr := GenerateBootstrapIso(func() (*config.Config, error) { + return settings, nil + }) assert.Equal(t, expectedErr, actualErr) }) }