From 4b6d2328de524b8dae63b05e4ee0baf57999c4e0 Mon Sep 17 00:00:00 2001 From: Drew Walters Date: Fri, 1 May 2020 16:26:26 +0000 Subject: [PATCH] Fix power command host selectors Both host selectors are always passed to create a manager when a power command is invoked; however, an empty label selector will include all hosts. This change adds a helper function to determine which selectors will be used based on the flags passed to airshipctl. When a flag is not used, a selector is not created for it. Change-Id: I48684cdf6d16073bf85468f6286a22a0aba602f7 Signed-off-by: Drew Walters --- cmd/baremetal/baremetal.go | 16 ++++++++++++++++ cmd/baremetal/baremetal_test.go | 17 +++++++++++++++++ cmd/baremetal/ejectmedia.go | 3 ++- cmd/baremetal/poweroff.go | 3 ++- cmd/baremetal/poweron.go | 3 ++- cmd/baremetal/powerstatus.go | 3 ++- cmd/baremetal/reboot.go | 3 ++- pkg/remote/errors.go | 7 +++++++ pkg/remote/management.go | 4 ++++ pkg/remote/management_test.go | 7 +++++++ 10 files changed, 61 insertions(+), 5 deletions(-) diff --git a/cmd/baremetal/baremetal.go b/cmd/baremetal/baremetal.go index 797412943..1b0b19539 100644 --- a/cmd/baremetal/baremetal.go +++ b/cmd/baremetal/baremetal.go @@ -18,6 +18,7 @@ import ( "github.com/spf13/cobra" "opendev.org/airship/airshipctl/pkg/environment" + "opendev.org/airship/airshipctl/pkg/remote" ) const ( @@ -63,3 +64,18 @@ func NewBaremetalCommand(rootSettings *environment.AirshipCTLSettings) *cobra.Co return cmd } + +// getHostSelections builds a list of selectors that can be passed to a manager using the name and label flags passed to +// airshipctl. +func GetHostSelections(name string, labels string) []remote.HostSelector { + var selectors []remote.HostSelector + if name != "" { + selectors = append(selectors, remote.ByName(name)) + } + + if labels != "" { + selectors = append(selectors, remote.ByLabel(labels)) + } + + return selectors +} diff --git a/cmd/baremetal/baremetal_test.go b/cmd/baremetal/baremetal_test.go index 4df6d4a35..ce0f237d8 100644 --- a/cmd/baremetal/baremetal_test.go +++ b/cmd/baremetal/baremetal_test.go @@ -17,6 +17,8 @@ package baremetal_test import ( "testing" + "github.com/stretchr/testify/assert" + "opendev.org/airship/airshipctl/cmd/baremetal" "opendev.org/airship/airshipctl/testutil" ) @@ -69,3 +71,18 @@ func TestBaremetal(t *testing.T) { testutil.RunTest(t, tt) } } + +func TestGetHostSelectionsOneSelector(t *testing.T) { + selectors := baremetal.GetHostSelections("node0", "") + assert.Len(t, selectors, 1) +} + +func TestGetHostSelectionsBothSelectors(t *testing.T) { + selectors := baremetal.GetHostSelections("node0", "airshipit.org/ephemeral-node=true") + assert.Len(t, selectors, 2) +} + +func TestGetHostSelectionsNone(t *testing.T) { + selectors := baremetal.GetHostSelections("", "") + assert.Len(t, selectors, 0) +} diff --git a/cmd/baremetal/ejectmedia.go b/cmd/baremetal/ejectmedia.go index f51c92654..c1927a260 100644 --- a/cmd/baremetal/ejectmedia.go +++ b/cmd/baremetal/ejectmedia.go @@ -35,7 +35,8 @@ 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 { - m, err := remote.NewManager(rootSettings, phase, remote.ByLabel(labels), remote.ByName(name)) + selectors := GetHostSelections(name, labels) + m, err := remote.NewManager(rootSettings, phase, selectors...) if err != nil { return err } diff --git a/cmd/baremetal/poweroff.go b/cmd/baremetal/poweroff.go index 72a21d195..14a44c50d 100644 --- a/cmd/baremetal/poweroff.go +++ b/cmd/baremetal/poweroff.go @@ -35,7 +35,8 @@ func NewPowerOffCommand(rootSettings *environment.AirshipCTLSettings) *cobra.Com Short: "Shutdown a baremetal host", Args: cobra.NoArgs, RunE: func(cmd *cobra.Command, args []string) error { - m, err := remote.NewManager(rootSettings, phase, remote.ByLabel(labels), remote.ByName(name)) + selectors := GetHostSelections(name, labels) + m, err := remote.NewManager(rootSettings, phase, selectors...) if err != nil { return err } diff --git a/cmd/baremetal/poweron.go b/cmd/baremetal/poweron.go index 4e040e719..0d30496a6 100644 --- a/cmd/baremetal/poweron.go +++ b/cmd/baremetal/poweron.go @@ -35,7 +35,8 @@ func NewPowerOnCommand(rootSettings *environment.AirshipCTLSettings) *cobra.Comm Short: "Power on a host", Args: cobra.NoArgs, RunE: func(cmd *cobra.Command, args []string) error { - m, err := remote.NewManager(rootSettings, phase, remote.ByLabel(labels), remote.ByName(name)) + selectors := GetHostSelections(name, labels) + m, err := remote.NewManager(rootSettings, phase, selectors...) if err != nil { return err } diff --git a/cmd/baremetal/powerstatus.go b/cmd/baremetal/powerstatus.go index 09ae3eba1..a983e7a73 100644 --- a/cmd/baremetal/powerstatus.go +++ b/cmd/baremetal/powerstatus.go @@ -35,7 +35,8 @@ 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 { - m, err := remote.NewManager(rootSettings, phase, remote.ByLabel(labels), remote.ByName(name)) + selectors := GetHostSelections(name, labels) + m, err := remote.NewManager(rootSettings, phase, selectors...) if err != nil { return err } diff --git a/cmd/baremetal/reboot.go b/cmd/baremetal/reboot.go index f647cc3de..2c8b370e1 100644 --- a/cmd/baremetal/reboot.go +++ b/cmd/baremetal/reboot.go @@ -35,7 +35,8 @@ func NewRebootCommand(rootSettings *environment.AirshipCTLSettings) *cobra.Comma Short: "Reboot a host", Args: cobra.NoArgs, RunE: func(cmd *cobra.Command, args []string) error { - m, err := remote.NewManager(rootSettings, phase, remote.ByLabel(labels), remote.ByName(name)) + selectors := GetHostSelections(name, labels) + m, err := remote.NewManager(rootSettings, phase, selectors...) if err != nil { return err } diff --git a/pkg/remote/errors.go b/pkg/remote/errors.go index fb908c06a..de8021101 100644 --- a/pkg/remote/errors.go +++ b/pkg/remote/errors.go @@ -56,3 +56,10 @@ type ErrMissingBootstrapInfoOption struct { func (e ErrMissingBootstrapInfoOption) Error() string { return fmt.Sprintf("missing bootstrapInfo option: %s", e.What) } + +// ErrNoHostsFound is an error that indicates that no hosts matched the selection criteria passed to a manager. +type ErrNoHostsFound struct{} + +func (e ErrNoHostsFound) Error() string { + return "no hosts selected" +} diff --git a/pkg/remote/management.go b/pkg/remote/management.go index 577c6598a..3344f1935 100644 --- a/pkg/remote/management.go +++ b/pkg/remote/management.go @@ -146,6 +146,10 @@ func NewManager(settings *environment.AirshipCTLSettings, phase string, hosts .. } } + if len(manager.Hosts) == 0 { + return manager, ErrNoHostsFound{} + } + return manager, nil } diff --git a/pkg/remote/management_test.go b/pkg/remote/management_test.go index 54bba9447..2804d0923 100644 --- a/pkg/remote/management_test.go +++ b/pkg/remote/management_test.go @@ -102,6 +102,13 @@ func TestNewManagerByNameNoHostFound(t *testing.T) { assert.Error(t, err) } +func TestNewManagerNoSelectors(t *testing.T) { + settings := initSettings(t, withTestDataPath("base")) + + _, err := NewManager(settings, config.BootstrapPhase) + assert.Error(t, err) +} + func TestNewManagerByLabelNoHostsFound(t *testing.T) { settings := initSettings(t, withTestDataPath("base"))