From a2b8d45cb091640a7a61226a333610980fc237c9 Mon Sep 17 00:00:00 2001 From: Ruslan Aliev Date: Thu, 27 Aug 2020 18:24:51 -0500 Subject: [PATCH] Refactor baremetal* commands All the baremetal 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: If75cd3f1a8bc22fc47ea132188bd9fec989ee434 Signed-off-by: Ruslan Aliev Relates-To: #327 --- cmd/baremetal/baremetal.go | 32 ++++++++------------------------ cmd/baremetal/baremetal_test.go | 3 ++- cmd/baremetal/ejectmedia.go | 10 +++++++--- cmd/baremetal/poweroff.go | 10 +++++++--- cmd/baremetal/poweron.go | 10 +++++++--- cmd/baremetal/powerstatus.go | 10 +++++++--- cmd/baremetal/reboot.go | 10 +++++++--- cmd/baremetal/remotedirect.go | 12 ++++++++---- pkg/remote/management.go | 7 +++---- pkg/remote/management_test.go | 21 +++++++++------------ pkg/remote/remote_direct.go | 6 +----- 11 files changed, 66 insertions(+), 65 deletions(-) diff --git a/cmd/baremetal/baremetal.go b/cmd/baremetal/baremetal.go index 1ee178384..3d0de2099 100644 --- a/cmd/baremetal/baremetal.go +++ b/cmd/baremetal/baremetal.go @@ -17,6 +17,7 @@ package baremetal import ( "github.com/spf13/cobra" + "opendev.org/airship/airshipctl/pkg/config" "opendev.org/airship/airshipctl/pkg/environment" "opendev.org/airship/airshipctl/pkg/remote" ) @@ -39,33 +40,16 @@ func NewBaremetalCommand(rootSettings *environment.AirshipCTLSettings) *cobra.Co baremetalRootCmd := &cobra.Command{ Use: "baremetal", Short: "Perform actions on baremetal hosts", - 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() - }, } - ejectMediaCmd := NewEjectMediaCommand(rootSettings) - baremetalRootCmd.AddCommand(ejectMediaCmd) + cfgFactory := config.CreateFactory(&rootSettings.AirshipConfigPath, &rootSettings.KubeConfigPath) - powerOffCmd := NewPowerOffCommand(rootSettings) - baremetalRootCmd.AddCommand(powerOffCmd) - - powerOnCmd := NewPowerOnCommand(rootSettings) - baremetalRootCmd.AddCommand(powerOnCmd) - - powerStatusCmd := NewPowerStatusCommand(rootSettings) - baremetalRootCmd.AddCommand(powerStatusCmd) - - rebootCmd := NewRebootCommand(rootSettings) - baremetalRootCmd.AddCommand(rebootCmd) - - remoteDirectCmd := NewRemoteDirectCommand(rootSettings) - baremetalRootCmd.AddCommand(remoteDirectCmd) + baremetalRootCmd.AddCommand(NewEjectMediaCommand(cfgFactory)) + baremetalRootCmd.AddCommand(NewPowerOffCommand(cfgFactory)) + baremetalRootCmd.AddCommand(NewPowerOnCommand(cfgFactory)) + baremetalRootCmd.AddCommand(NewPowerStatusCommand(cfgFactory)) + baremetalRootCmd.AddCommand(NewRebootCommand(cfgFactory)) + baremetalRootCmd.AddCommand(NewRemoteDirectCommand(cfgFactory)) return baremetalRootCmd } diff --git a/cmd/baremetal/baremetal_test.go b/cmd/baremetal/baremetal_test.go index ec6d3221f..3a49b1b16 100644 --- a/cmd/baremetal/baremetal_test.go +++ b/cmd/baremetal/baremetal_test.go @@ -20,6 +20,7 @@ import ( "github.com/stretchr/testify/assert" "opendev.org/airship/airshipctl/cmd/baremetal" + "opendev.org/airship/airshipctl/pkg/environment" "opendev.org/airship/airshipctl/testutil" ) @@ -28,7 +29,7 @@ func TestBaremetal(t *testing.T) { { Name: "baremetal-with-help", CmdLine: "-h", - Cmd: baremetal.NewBaremetalCommand(nil), + Cmd: baremetal.NewBaremetalCommand(&environment.AirshipCTLSettings{}), }, { Name: "baremetal-ejectmedia-with-help", diff --git a/cmd/baremetal/ejectmedia.go b/cmd/baremetal/ejectmedia.go index c1927a260..721147b08 100644 --- a/cmd/baremetal/ejectmedia.go +++ b/cmd/baremetal/ejectmedia.go @@ -20,12 +20,11 @@ import ( "github.com/spf13/cobra" "opendev.org/airship/airshipctl/pkg/config" - "opendev.org/airship/airshipctl/pkg/environment" "opendev.org/airship/airshipctl/pkg/remote" ) // NewEjectMediaCommand provides a command to eject media attached to a baremetal host. -func NewEjectMediaCommand(rootSettings *environment.AirshipCTLSettings) *cobra.Command { +func NewEjectMediaCommand(cfgFactory config.Factory) *cobra.Command { var labels string var name string var phase string @@ -35,8 +34,13 @@ func NewEjectMediaCommand(rootSettings *environment.AirshipCTLSettings) *cobra.C Short: "Eject media attached to a baremetal host", Args: cobra.NoArgs, RunE: func(cmd *cobra.Command, args []string) error { + cfg, err := cfgFactory() + if err != nil { + return err + } + selectors := GetHostSelections(name, labels) - m, err := remote.NewManager(rootSettings, phase, selectors...) + m, err := remote.NewManager(cfg, phase, selectors...) if err != nil { return err } diff --git a/cmd/baremetal/poweroff.go b/cmd/baremetal/poweroff.go index 14a44c50d..f7ccd9e66 100644 --- a/cmd/baremetal/poweroff.go +++ b/cmd/baremetal/poweroff.go @@ -20,12 +20,11 @@ import ( "github.com/spf13/cobra" "opendev.org/airship/airshipctl/pkg/config" - "opendev.org/airship/airshipctl/pkg/environment" "opendev.org/airship/airshipctl/pkg/remote" ) // NewPowerOffCommand provides a command to shutdown a remote host. -func NewPowerOffCommand(rootSettings *environment.AirshipCTLSettings) *cobra.Command { +func NewPowerOffCommand(cfgFactory config.Factory) *cobra.Command { var labels string var name string var phase string @@ -35,8 +34,13 @@ func NewPowerOffCommand(rootSettings *environment.AirshipCTLSettings) *cobra.Com Short: "Shutdown a baremetal host", Args: cobra.NoArgs, RunE: func(cmd *cobra.Command, args []string) error { + cfg, err := cfgFactory() + if err != nil { + return err + } + selectors := GetHostSelections(name, labels) - m, err := remote.NewManager(rootSettings, phase, selectors...) + m, err := remote.NewManager(cfg, phase, selectors...) if err != nil { return err } diff --git a/cmd/baremetal/poweron.go b/cmd/baremetal/poweron.go index 0d30496a6..3e2923393 100644 --- a/cmd/baremetal/poweron.go +++ b/cmd/baremetal/poweron.go @@ -20,12 +20,11 @@ import ( "github.com/spf13/cobra" "opendev.org/airship/airshipctl/pkg/config" - "opendev.org/airship/airshipctl/pkg/environment" "opendev.org/airship/airshipctl/pkg/remote" ) // NewPowerOnCommand provides a command with the capability to power on baremetal hosts. -func NewPowerOnCommand(rootSettings *environment.AirshipCTLSettings) *cobra.Command { +func NewPowerOnCommand(cfgFactory config.Factory) *cobra.Command { var labels string var name string var phase string @@ -35,8 +34,13 @@ func NewPowerOnCommand(rootSettings *environment.AirshipCTLSettings) *cobra.Comm Short: "Power on a host", Args: cobra.NoArgs, RunE: func(cmd *cobra.Command, args []string) error { + cfg, err := cfgFactory() + if err != nil { + return err + } + selectors := GetHostSelections(name, labels) - m, err := remote.NewManager(rootSettings, phase, selectors...) + m, err := remote.NewManager(cfg, phase, selectors...) if err != nil { return err } diff --git a/cmd/baremetal/powerstatus.go b/cmd/baremetal/powerstatus.go index a983e7a73..2456dbe77 100644 --- a/cmd/baremetal/powerstatus.go +++ b/cmd/baremetal/powerstatus.go @@ -20,12 +20,11 @@ import ( "github.com/spf13/cobra" "opendev.org/airship/airshipctl/pkg/config" - "opendev.org/airship/airshipctl/pkg/environment" "opendev.org/airship/airshipctl/pkg/remote" ) // NewPowerStatusCommand provides a command to retrieve the power status of a baremetal host. -func NewPowerStatusCommand(rootSettings *environment.AirshipCTLSettings) *cobra.Command { +func NewPowerStatusCommand(cfgFactory config.Factory) *cobra.Command { var labels string var name string var phase string @@ -35,8 +34,13 @@ func NewPowerStatusCommand(rootSettings *environment.AirshipCTLSettings) *cobra. Short: "Retrieve the power status of a baremetal host", Args: cobra.NoArgs, RunE: func(cmd *cobra.Command, args []string) error { + cfg, err := cfgFactory() + if err != nil { + return err + } + selectors := GetHostSelections(name, labels) - m, err := remote.NewManager(rootSettings, phase, selectors...) + m, err := remote.NewManager(cfg, phase, selectors...) if err != nil { return err } diff --git a/cmd/baremetal/reboot.go b/cmd/baremetal/reboot.go index 2c8b370e1..c0956f365 100644 --- a/cmd/baremetal/reboot.go +++ b/cmd/baremetal/reboot.go @@ -20,12 +20,11 @@ import ( "github.com/spf13/cobra" "opendev.org/airship/airshipctl/pkg/config" - "opendev.org/airship/airshipctl/pkg/environment" "opendev.org/airship/airshipctl/pkg/remote" ) // NewRebootCommand provides a command with the capability to reboot baremetal hosts. -func NewRebootCommand(rootSettings *environment.AirshipCTLSettings) *cobra.Command { +func NewRebootCommand(cfgFactory config.Factory) *cobra.Command { var labels string var name string var phase string @@ -35,8 +34,13 @@ func NewRebootCommand(rootSettings *environment.AirshipCTLSettings) *cobra.Comma Short: "Reboot a host", Args: cobra.NoArgs, RunE: func(cmd *cobra.Command, args []string) error { + cfg, err := cfgFactory() + if err != nil { + return err + } + selectors := GetHostSelections(name, labels) - m, err := remote.NewManager(rootSettings, phase, selectors...) + m, err := remote.NewManager(cfg, phase, selectors...) if err != nil { return err } diff --git a/cmd/baremetal/remotedirect.go b/cmd/baremetal/remotedirect.go index 436ac85aa..5f0a31678 100644 --- a/cmd/baremetal/remotedirect.go +++ b/cmd/baremetal/remotedirect.go @@ -19,17 +19,21 @@ import ( "opendev.org/airship/airshipctl/pkg/config" "opendev.org/airship/airshipctl/pkg/document" - "opendev.org/airship/airshipctl/pkg/environment" "opendev.org/airship/airshipctl/pkg/remote" ) // NewRemoteDirectCommand provides a command with the capability to perform remote direct operations. -func NewRemoteDirectCommand(rootSettings *environment.AirshipCTLSettings) *cobra.Command { +func NewRemoteDirectCommand(cfgFactory config.Factory) *cobra.Command { cmd := &cobra.Command{ Use: "remotedirect", Short: "Bootstrap the ephemeral host", RunE: func(cmd *cobra.Command, args []string) error { - manager, err := remote.NewManager(rootSettings, + cfg, err := cfgFactory() + if err != nil { + return err + } + + manager, err := remote.NewManager(cfg, config.BootstrapPhase, remote.ByLabel(document.EphemeralHostSelector)) if err != nil { @@ -41,7 +45,7 @@ func NewRemoteDirectCommand(rootSettings *environment.AirshipCTLSettings) *cobra } ephemeralHost := manager.Hosts[0] - return ephemeralHost.DoRemoteDirect(rootSettings) + return ephemeralHost.DoRemoteDirect(cfg) }, } diff --git a/pkg/remote/management.go b/pkg/remote/management.go index fcd411b06..85f022974 100644 --- a/pkg/remote/management.go +++ b/pkg/remote/management.go @@ -20,7 +20,6 @@ import ( "opendev.org/airship/airshipctl/pkg/config" "opendev.org/airship/airshipctl/pkg/document" - "opendev.org/airship/airshipctl/pkg/environment" "opendev.org/airship/airshipctl/pkg/log" "opendev.org/airship/airshipctl/pkg/remote/power" "opendev.org/airship/airshipctl/pkg/remote/redfish" @@ -116,8 +115,8 @@ func ByName(name string) HostSelector { // NewManager provides a manager that exposes the capability to perform remote direct functionality and other // out-of-band management on multiple hosts. -func NewManager(settings *environment.AirshipCTLSettings, phase string, hosts ...HostSelector) (*Manager, error) { - managementCfg, err := settings.Config.CurrentContextManagementConfig() +func NewManager(cfg *config.Config, phase string, hosts ...HostSelector) (*Manager, error) { + managementCfg, err := cfg.CurrentContextManagementConfig() if err != nil { return nil, err } @@ -126,7 +125,7 @@ func NewManager(settings *environment.AirshipCTLSettings, phase string, hosts .. return nil, err } - entrypoint, err := settings.Config.CurrentContextEntryPoint(phase) + entrypoint, err := cfg.CurrentContextEntryPoint(phase) if err != nil { return nil, err } diff --git a/pkg/remote/management_test.go b/pkg/remote/management_test.go index bc4f1983f..a7f7539c3 100644 --- a/pkg/remote/management_test.go +++ b/pkg/remote/management_test.go @@ -23,39 +23,36 @@ import ( "opendev.org/airship/airshipctl/pkg/config" "opendev.org/airship/airshipctl/pkg/document" - "opendev.org/airship/airshipctl/pkg/environment" "opendev.org/airship/airshipctl/pkg/remote/redfish" redfishdell "opendev.org/airship/airshipctl/pkg/remote/redfish/vendors/dell" "opendev.org/airship/airshipctl/testutil" ) -type Configuration func(*environment.AirshipCTLSettings) +type Configuration func(*config.Config) // initSettings initializes the global airshipctl settings with test data by accepting functions as arguments that // manipulate configuration sections. -func initSettings(t *testing.T, configs ...Configuration) *environment.AirshipCTLSettings { +func initSettings(t *testing.T, configs ...Configuration) *config.Config { t.Helper() - settings := &environment.AirshipCTLSettings{Config: testutil.DummyConfig()} - + config := testutil.DummyConfig() for _, cfg := range configs { - cfg(settings) + cfg(config) } - - return settings + return config } // withManagementConfig initializes the management config when used as an argument to initSettings. func withManagementConfig(cfg *config.ManagementConfiguration) Configuration { - return func(settings *environment.AirshipCTLSettings) { - settings.Config.ManagementConfiguration["dummy_management_config"] = cfg + return func(settings *config.Config) { + settings.ManagementConfiguration["dummy_management_config"] = cfg } } // withTestDataPath sets the test data path when used as an argument to initSettings. func withTestDataPath(path string) Configuration { - return func(settings *environment.AirshipCTLSettings) { - manifest, err := settings.Config.CurrentContextManifest() + return func(settings *config.Config) { + manifest, err := settings.CurrentContextManifest() if err != nil { panic(fmt.Sprintf("Unable to initialize management tests. Current Context error %q", err)) } diff --git a/pkg/remote/remote_direct.go b/pkg/remote/remote_direct.go index f5c9a37ae..3e0fed0c4 100644 --- a/pkg/remote/remote_direct.go +++ b/pkg/remote/remote_direct.go @@ -18,16 +18,12 @@ import ( api "opendev.org/airship/airshipctl/pkg/api/v1alpha1" "opendev.org/airship/airshipctl/pkg/config" "opendev.org/airship/airshipctl/pkg/document" - "opendev.org/airship/airshipctl/pkg/environment" "opendev.org/airship/airshipctl/pkg/log" - "opendev.org/airship/airshipctl/pkg/remote/power" ) // DoRemoteDirect bootstraps the ephemeral node. -func (b baremetalHost) DoRemoteDirect(settings *environment.AirshipCTLSettings) error { - cfg := settings.Config - +func (b baremetalHost) DoRemoteDirect(cfg *config.Config) error { root, err := cfg.CurrentContextEntryPoint(config.BootstrapPhase) if err != nil { return err