airship/airshipctl
Code Issues Proposed changes

Refactor image* commands

Browse Source 
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 <raliev@mirantis.com>
Relates-To: #327
This commit is contained in:
Ruslan Aliev 2020-08-27 18:48:12 -05:00
parent a2b8d45cb0
commit e81116b1a9
5 changed files with 51 additions and 69 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
cmd/image/build.go
View File

@ -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)
},
}

12
cmd/image/image.go
View File

@ -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()
},
}
cfgFactory := config.CreateFactory(&rootSettings.AirshipConfigPath, &rootSettings.KubeConfigPath)
imageBuildCmd := NewImageBuildCommand(rootSettings)
imageRootCmd.AddCommand(imageBuildCmd)
imageRootCmd.AddCommand(NewImageBuildCommand(cfgFactory))
return imageRootCmd
}

3
cmd/image/image_test.go
View File

@ -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{}),
},
}

7
pkg/bootstrap/isogen/command.go
View File

@ -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
}

90
pkg/bootstrap/isogen/command_test.go
View File

@ -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)
})
}