From 7dfb0eb98c71fa586dc487382def5e0a33ea2f1e Mon Sep 17 00:00:00 2001 From: Kostiantyn Kalynovskyi Date: Tue, 9 Feb 2021 22:52:17 +0000 Subject: [PATCH] Don't allow to run baremetal actions without flags With this change we will be sure that no commands against bmh hosts will be applied without selector flags specified, now if you want to target all hosts in inventory you must specify --all flag Relates-To: #364 Related-To: #364 Closes: #364 Change-Id: I0d615ea226528162e07acc96fa61b26e031145aa --- cmd/baremetal/baremetal.go | 28 ++++++++++ cmd/baremetal/ejectmedia.go | 22 ++++++-- cmd/baremetal/poweroff.go | 22 ++++++-- cmd/baremetal/poweron.go | 22 ++++++-- cmd/baremetal/reboot.go | 22 ++++++-- .../baremetal-ejectmedia-with-help.golden | 20 ++++++- .../baremetal-poweroff-with-help.golden | 20 ++++++- .../baremetal-poweron-with-help.golden | 20 ++++++- .../baremetal-reboot-with-help.golden | 20 ++++++- .../baremetal-with-help.golden | 8 +-- docs/source/cli/airshipctl_baremetal.md | 8 +-- .../cli/airshipctl_baremetal_ejectmedia.md | 26 ++++++++- .../cli/airshipctl_baremetal_poweroff.md | 26 ++++++++- .../cli/airshipctl_baremetal_poweron.md | 26 ++++++++- .../source/cli/airshipctl_baremetal_reboot.md | 26 ++++++++- pkg/inventory/command.go | 30 +++++++++++ pkg/inventory/command_test.go | 53 +++++++++++++++++++ pkg/inventory/errors.go | 26 +++++++++ pkg/inventory/testdata/hosts.yaml | 4 +- 19 files changed, 395 insertions(+), 34 deletions(-) create mode 100644 pkg/inventory/errors.go diff --git a/cmd/baremetal/baremetal.go b/cmd/baremetal/baremetal.go index c60254790..04f8718c9 100644 --- a/cmd/baremetal/baremetal.go +++ b/cmd/baremetal/baremetal.go @@ -15,6 +15,7 @@ package baremetal import ( + "fmt" "time" "github.com/spf13/cobra" @@ -40,6 +41,29 @@ const ( flagTimeout = "timeout" flagTimeoutDescription = "timeout on baremetal action" + + flagAll = "all" + flagAllDescription = "specify this to target all hosts in the inventory" +) + +var ( + selectorsDescription = fmt.Sprintf(`The command will target baremetal hosts from airship inventory kustomize root +based on the --%s, --%s and --%s flags provided. If no flags are +provided airshipctl will try to select all baremetal hosts in the inventory`, flagName, flagNamespace, flagLabel) + + bmhActionExampleTempalte = ` +Perform action against hosts with name rdm9r3s3 in all namespaces where the host is found +# airshipctl baremetal %[1]s --name rdm9r3s3 + +Perform action against hosts with name rdm9r3s3 in namespace metal3 +# airshipctl baremetal %[1]s --name rdm9r3s3 --namespace metal3 + +Perform action against all hosts defined in inventory +# airshipctl baremetal %[1]s --all + +Perform action against hosts with a label 'foo=bar' +# airshipctl baremetal %[1]s --labels "foo=bar" +` ) // NewBaremetalCommand creates a new command for interacting with baremetal using airshipctl. @@ -67,3 +91,7 @@ func initFlags(options *inventory.CommandOptions, cmd *cobra.Command) { flags.StringVarP(&options.Namespace, flagNamespace, flagNamespaceSort, "", flagNamespaceDescription) flags.DurationVar(&options.Timeout, flagTimeout, 10*time.Minute, flagTimeoutDescription) } + +func initAllFlag(options *inventory.CommandOptions, cmd *cobra.Command) { + cmd.Flags().BoolVar(&options.All, flagAll, false, flagAllDescription) +} diff --git a/cmd/baremetal/ejectmedia.go b/cmd/baremetal/ejectmedia.go index 212cf1d62..9393c933e 100644 --- a/cmd/baremetal/ejectmedia.go +++ b/cmd/baremetal/ejectmedia.go @@ -15,6 +15,8 @@ package baremetal import ( + "fmt" + "github.com/spf13/cobra" "opendev.org/airship/airshipctl/pkg/config" @@ -22,18 +24,32 @@ import ( "opendev.org/airship/airshipctl/pkg/inventory/ifc" ) +var ( + ejectMediaCommand = "ejectmedia" + + ejectMediaLong = fmt.Sprintf(` +Eject media attached to a baremetal hosts +%s +`, selectorsDescription) + + ejectMediaExample = fmt.Sprintf(bmhActionExampleTempalte, ejectMediaCommand) +) + // NewEjectMediaCommand provides a command to eject media attached to a baremetal host. func NewEjectMediaCommand(cfgFactory config.Factory, options *inventory.CommandOptions) *cobra.Command { cmd := &cobra.Command{ - Use: "ejectmedia", - Short: "Eject media attached to a baremetal host", - Args: cobra.NoArgs, + Use: ejectMediaCommand, + Short: "Eject media attached to a baremetal hosts", + Long: ejectMediaLong[1:], + Example: ejectMediaExample[1:], + Args: cobra.NoArgs, RunE: func(cmd *cobra.Command, args []string) error { return options.BMHAction(ifc.BaremetalOperationEjectVirtualMedia) }, } initFlags(options, cmd) + initAllFlag(options, cmd) return cmd } diff --git a/cmd/baremetal/poweroff.go b/cmd/baremetal/poweroff.go index efee2bbf3..1705a25de 100644 --- a/cmd/baremetal/poweroff.go +++ b/cmd/baremetal/poweroff.go @@ -15,6 +15,8 @@ package baremetal import ( + "fmt" + "github.com/spf13/cobra" "opendev.org/airship/airshipctl/pkg/config" @@ -22,18 +24,32 @@ import ( "opendev.org/airship/airshipctl/pkg/inventory/ifc" ) +var ( + powerOffCommand = "poweroff" + + powerOffLong = fmt.Sprintf(` +Power off baremetal hosts +%s +`, selectorsDescription) + + powerOffExample = fmt.Sprintf(bmhActionExampleTempalte, powerOffCommand) +) + // NewPowerOffCommand provides a command to shutdown a remote host. func NewPowerOffCommand(cfgFactory config.Factory, options *inventory.CommandOptions) *cobra.Command { cmd := &cobra.Command{ - Use: "poweroff", - Short: "Shutdown a baremetal host", - Args: cobra.NoArgs, + Use: powerOffCommand, + Short: "Shutdown a baremetal hosts", + Long: powerOffLong[1:], + Example: powerOffExample[1:], + Args: cobra.NoArgs, RunE: func(cmd *cobra.Command, args []string) error { return options.BMHAction(ifc.BaremetalOperationPowerOff) }, } initFlags(options, cmd) + initAllFlag(options, cmd) return cmd } diff --git a/cmd/baremetal/poweron.go b/cmd/baremetal/poweron.go index b014ab79c..53c85e023 100644 --- a/cmd/baremetal/poweron.go +++ b/cmd/baremetal/poweron.go @@ -15,6 +15,8 @@ Licensed under the Apache License, Version 2.0 (the "License"); package baremetal import ( + "fmt" + "github.com/spf13/cobra" "opendev.org/airship/airshipctl/pkg/config" @@ -22,18 +24,32 @@ import ( "opendev.org/airship/airshipctl/pkg/inventory/ifc" ) +var ( + powerOnCommand = "poweron" + + powerOnLong = fmt.Sprintf(` +Power on baremetal hosts +%s +`, selectorsDescription) + + powerOnExample = fmt.Sprintf(bmhActionExampleTempalte, powerOnCommand) +) + // NewPowerOnCommand provides a command with the capability to power on baremetal hosts. func NewPowerOnCommand(cfgFactory config.Factory, options *inventory.CommandOptions) *cobra.Command { cmd := &cobra.Command{ - Use: "poweron", - Short: "Power on a host", - Args: cobra.NoArgs, + Use: powerOnCommand, + Short: "Power on a hosts", + Long: powerOnLong[1:], + Example: powerOnExample[1:], + Args: cobra.NoArgs, RunE: func(cmd *cobra.Command, args []string) error { return options.BMHAction(ifc.BaremetalOperationPowerOn) }, } initFlags(options, cmd) + initAllFlag(options, cmd) return cmd } diff --git a/cmd/baremetal/reboot.go b/cmd/baremetal/reboot.go index 036c1b59e..be3481bd7 100644 --- a/cmd/baremetal/reboot.go +++ b/cmd/baremetal/reboot.go @@ -15,6 +15,8 @@ Licensed under the Apache License, Version 2.0 (the "License"); package baremetal import ( + "fmt" + "github.com/spf13/cobra" "opendev.org/airship/airshipctl/pkg/config" @@ -22,18 +24,32 @@ import ( "opendev.org/airship/airshipctl/pkg/inventory/ifc" ) +var ( + rebootCommand = "reboot" + + rebootLong = fmt.Sprintf(` +Reboot baremetal hosts +%s +`, selectorsDescription) + + rebootExample = fmt.Sprintf(bmhActionExampleTempalte, rebootCommand) +) + // NewRebootCommand provides a command with the capability to reboot baremetal hosts. func NewRebootCommand(cfgFactory config.Factory, options *inventory.CommandOptions) *cobra.Command { cmd := &cobra.Command{ - Use: "reboot", - Short: "Reboot a host", - Args: cobra.NoArgs, + Use: rebootCommand, + Long: rebootLong[1:], + Short: "Reboot a hosts", + Example: rebootExample[1:], + Args: cobra.NoArgs, RunE: func(cmd *cobra.Command, args []string) error { return options.BMHAction(ifc.BaremetalOperationReboot) }, } initFlags(options, cmd) + initAllFlag(options, cmd) return cmd } diff --git a/cmd/baremetal/testdata/TestBaremetalGoldenOutput/baremetal-ejectmedia-with-help.golden b/cmd/baremetal/testdata/TestBaremetalGoldenOutput/baremetal-ejectmedia-with-help.golden index db90cf2d8..11e6d2298 100644 --- a/cmd/baremetal/testdata/TestBaremetalGoldenOutput/baremetal-ejectmedia-with-help.golden +++ b/cmd/baremetal/testdata/TestBaremetalGoldenOutput/baremetal-ejectmedia-with-help.golden @@ -1,9 +1,27 @@ -Eject media attached to a baremetal host +Eject media attached to a baremetal hosts +The command will target baremetal hosts from airship inventory kustomize root +based on the --name, --namespace and --labels flags provided. If no flags are +provided airshipctl will try to select all baremetal hosts in the inventory Usage: ejectmedia [flags] +Examples: +Perform action against hosts with name rdm9r3s3 in all namespaces where the host is found +# airshipctl baremetal ejectmedia --name rdm9r3s3 + +Perform action against hosts with name rdm9r3s3 in namespace metal3 +# airshipctl baremetal ejectmedia --name rdm9r3s3 --namespace metal3 + +Perform action against all hosts defined in inventory +# airshipctl baremetal ejectmedia --all + +Perform action against hosts with a label 'foo=bar' +# airshipctl baremetal ejectmedia --labels "foo=bar" + + Flags: + --all specify this to target all hosts in the inventory -h, --help help for ejectmedia -l, --labels string Label(s) to filter desired baremetal host documents --name string Name to filter desired baremetal host document diff --git a/cmd/baremetal/testdata/TestBaremetalGoldenOutput/baremetal-poweroff-with-help.golden b/cmd/baremetal/testdata/TestBaremetalGoldenOutput/baremetal-poweroff-with-help.golden index b6c263e72..d5ce74613 100644 --- a/cmd/baremetal/testdata/TestBaremetalGoldenOutput/baremetal-poweroff-with-help.golden +++ b/cmd/baremetal/testdata/TestBaremetalGoldenOutput/baremetal-poweroff-with-help.golden @@ -1,9 +1,27 @@ -Shutdown a baremetal host +Power off baremetal hosts +The command will target baremetal hosts from airship inventory kustomize root +based on the --name, --namespace and --labels flags provided. If no flags are +provided airshipctl will try to select all baremetal hosts in the inventory Usage: poweroff [flags] +Examples: +Perform action against hosts with name rdm9r3s3 in all namespaces where the host is found +# airshipctl baremetal poweroff --name rdm9r3s3 + +Perform action against hosts with name rdm9r3s3 in namespace metal3 +# airshipctl baremetal poweroff --name rdm9r3s3 --namespace metal3 + +Perform action against all hosts defined in inventory +# airshipctl baremetal poweroff --all + +Perform action against hosts with a label 'foo=bar' +# airshipctl baremetal poweroff --labels "foo=bar" + + Flags: + --all specify this to target all hosts in the inventory -h, --help help for poweroff -l, --labels string Label(s) to filter desired baremetal host documents --name string Name to filter desired baremetal host document diff --git a/cmd/baremetal/testdata/TestBaremetalGoldenOutput/baremetal-poweron-with-help.golden b/cmd/baremetal/testdata/TestBaremetalGoldenOutput/baremetal-poweron-with-help.golden index fca30b5f1..976009921 100644 --- a/cmd/baremetal/testdata/TestBaremetalGoldenOutput/baremetal-poweron-with-help.golden +++ b/cmd/baremetal/testdata/TestBaremetalGoldenOutput/baremetal-poweron-with-help.golden @@ -1,9 +1,27 @@ -Power on a host +Power on baremetal hosts +The command will target baremetal hosts from airship inventory kustomize root +based on the --name, --namespace and --labels flags provided. If no flags are +provided airshipctl will try to select all baremetal hosts in the inventory Usage: poweron [flags] +Examples: +Perform action against hosts with name rdm9r3s3 in all namespaces where the host is found +# airshipctl baremetal poweron --name rdm9r3s3 + +Perform action against hosts with name rdm9r3s3 in namespace metal3 +# airshipctl baremetal poweron --name rdm9r3s3 --namespace metal3 + +Perform action against all hosts defined in inventory +# airshipctl baremetal poweron --all + +Perform action against hosts with a label 'foo=bar' +# airshipctl baremetal poweron --labels "foo=bar" + + Flags: + --all specify this to target all hosts in the inventory -h, --help help for poweron -l, --labels string Label(s) to filter desired baremetal host documents --name string Name to filter desired baremetal host document diff --git a/cmd/baremetal/testdata/TestBaremetalGoldenOutput/baremetal-reboot-with-help.golden b/cmd/baremetal/testdata/TestBaremetalGoldenOutput/baremetal-reboot-with-help.golden index 95cbfc99f..e40a87edf 100644 --- a/cmd/baremetal/testdata/TestBaremetalGoldenOutput/baremetal-reboot-with-help.golden +++ b/cmd/baremetal/testdata/TestBaremetalGoldenOutput/baremetal-reboot-with-help.golden @@ -1,9 +1,27 @@ -Reboot a host +Reboot baremetal hosts +The command will target baremetal hosts from airship inventory kustomize root +based on the --name, --namespace and --labels flags provided. If no flags are +provided airshipctl will try to select all baremetal hosts in the inventory Usage: reboot [flags] +Examples: +Perform action against hosts with name rdm9r3s3 in all namespaces where the host is found +# airshipctl baremetal reboot --name rdm9r3s3 + +Perform action against hosts with name rdm9r3s3 in namespace metal3 +# airshipctl baremetal reboot --name rdm9r3s3 --namespace metal3 + +Perform action against all hosts defined in inventory +# airshipctl baremetal reboot --all + +Perform action against hosts with a label 'foo=bar' +# airshipctl baremetal reboot --labels "foo=bar" + + Flags: + --all specify this to target all hosts in the inventory -h, --help help for reboot -l, --labels string Label(s) to filter desired baremetal host documents --name string Name to filter desired baremetal host document diff --git a/cmd/baremetal/testdata/TestBaremetalGoldenOutput/baremetal-with-help.golden b/cmd/baremetal/testdata/TestBaremetalGoldenOutput/baremetal-with-help.golden index 5fe569010..698b9f28e 100644 --- a/cmd/baremetal/testdata/TestBaremetalGoldenOutput/baremetal-with-help.golden +++ b/cmd/baremetal/testdata/TestBaremetalGoldenOutput/baremetal-with-help.golden @@ -4,12 +4,12 @@ Usage: baremetal [command] Available Commands: - ejectmedia Eject media attached to a baremetal host + ejectmedia Eject media attached to a baremetal hosts help Help about any command - poweroff Shutdown a baremetal host - poweron Power on a host + poweroff Shutdown a baremetal hosts + poweron Power on a hosts powerstatus Retrieve the power status of a baremetal host - reboot Reboot a host + reboot Reboot a hosts remotedirect Bootstrap the ephemeral host Flags: diff --git a/docs/source/cli/airshipctl_baremetal.md b/docs/source/cli/airshipctl_baremetal.md index 8e7e635ff..0d6a6c732 100644 --- a/docs/source/cli/airshipctl_baremetal.md +++ b/docs/source/cli/airshipctl_baremetal.md @@ -22,10 +22,10 @@ Perform actions on baremetal hosts ### SEE ALSO * [airshipctl](airshipctl.md) - A unified entrypoint to various airship components -* [airshipctl baremetal ejectmedia](airshipctl_baremetal_ejectmedia.md) - Eject media attached to a baremetal host -* [airshipctl baremetal poweroff](airshipctl_baremetal_poweroff.md) - Shutdown a baremetal host -* [airshipctl baremetal poweron](airshipctl_baremetal_poweron.md) - Power on a host +* [airshipctl baremetal ejectmedia](airshipctl_baremetal_ejectmedia.md) - Eject media attached to a baremetal hosts +* [airshipctl baremetal poweroff](airshipctl_baremetal_poweroff.md) - Shutdown a baremetal hosts +* [airshipctl baremetal poweron](airshipctl_baremetal_poweron.md) - Power on a hosts * [airshipctl baremetal powerstatus](airshipctl_baremetal_powerstatus.md) - Retrieve the power status of a baremetal host -* [airshipctl baremetal reboot](airshipctl_baremetal_reboot.md) - Reboot a host +* [airshipctl baremetal reboot](airshipctl_baremetal_reboot.md) - Reboot a hosts * [airshipctl baremetal remotedirect](airshipctl_baremetal_remotedirect.md) - Bootstrap the ephemeral host diff --git a/docs/source/cli/airshipctl_baremetal_ejectmedia.md b/docs/source/cli/airshipctl_baremetal_ejectmedia.md index 551809d27..9f1967680 100644 --- a/docs/source/cli/airshipctl_baremetal_ejectmedia.md +++ b/docs/source/cli/airshipctl_baremetal_ejectmedia.md @@ -1,18 +1,40 @@ ## airshipctl baremetal ejectmedia -Eject media attached to a baremetal host +Eject media attached to a baremetal hosts ### Synopsis -Eject media attached to a baremetal host +Eject media attached to a baremetal hosts +The command will target baremetal hosts from airship inventory kustomize root +based on the --name, --namespace and --labels flags provided. If no flags are +provided airshipctl will try to select all baremetal hosts in the inventory + ``` airshipctl baremetal ejectmedia [flags] ``` +### Examples + +``` +Perform action against hosts with name rdm9r3s3 in all namespaces where the host is found +# airshipctl baremetal ejectmedia --name rdm9r3s3 + +Perform action against hosts with name rdm9r3s3 in namespace metal3 +# airshipctl baremetal ejectmedia --name rdm9r3s3 --namespace metal3 + +Perform action against all hosts defined in inventory +# airshipctl baremetal ejectmedia --all + +Perform action against hosts with a label 'foo=bar' +# airshipctl baremetal ejectmedia --labels "foo=bar" + +``` + ### Options ``` + --all specify this to target all hosts in the inventory -h, --help help for ejectmedia -l, --labels string Label(s) to filter desired baremetal host documents --name string Name to filter desired baremetal host document diff --git a/docs/source/cli/airshipctl_baremetal_poweroff.md b/docs/source/cli/airshipctl_baremetal_poweroff.md index 1e9084a31..b42354a4b 100644 --- a/docs/source/cli/airshipctl_baremetal_poweroff.md +++ b/docs/source/cli/airshipctl_baremetal_poweroff.md @@ -1,18 +1,40 @@ ## airshipctl baremetal poweroff -Shutdown a baremetal host +Shutdown a baremetal hosts ### Synopsis -Shutdown a baremetal host +Power off baremetal hosts +The command will target baremetal hosts from airship inventory kustomize root +based on the --name, --namespace and --labels flags provided. If no flags are +provided airshipctl will try to select all baremetal hosts in the inventory + ``` airshipctl baremetal poweroff [flags] ``` +### Examples + +``` +Perform action against hosts with name rdm9r3s3 in all namespaces where the host is found +# airshipctl baremetal poweroff --name rdm9r3s3 + +Perform action against hosts with name rdm9r3s3 in namespace metal3 +# airshipctl baremetal poweroff --name rdm9r3s3 --namespace metal3 + +Perform action against all hosts defined in inventory +# airshipctl baremetal poweroff --all + +Perform action against hosts with a label 'foo=bar' +# airshipctl baremetal poweroff --labels "foo=bar" + +``` + ### Options ``` + --all specify this to target all hosts in the inventory -h, --help help for poweroff -l, --labels string Label(s) to filter desired baremetal host documents --name string Name to filter desired baremetal host document diff --git a/docs/source/cli/airshipctl_baremetal_poweron.md b/docs/source/cli/airshipctl_baremetal_poweron.md index 417d51956..1173239a6 100644 --- a/docs/source/cli/airshipctl_baremetal_poweron.md +++ b/docs/source/cli/airshipctl_baremetal_poweron.md @@ -1,18 +1,40 @@ ## airshipctl baremetal poweron -Power on a host +Power on a hosts ### Synopsis -Power on a host +Power on baremetal hosts +The command will target baremetal hosts from airship inventory kustomize root +based on the --name, --namespace and --labels flags provided. If no flags are +provided airshipctl will try to select all baremetal hosts in the inventory + ``` airshipctl baremetal poweron [flags] ``` +### Examples + +``` +Perform action against hosts with name rdm9r3s3 in all namespaces where the host is found +# airshipctl baremetal poweron --name rdm9r3s3 + +Perform action against hosts with name rdm9r3s3 in namespace metal3 +# airshipctl baremetal poweron --name rdm9r3s3 --namespace metal3 + +Perform action against all hosts defined in inventory +# airshipctl baremetal poweron --all + +Perform action against hosts with a label 'foo=bar' +# airshipctl baremetal poweron --labels "foo=bar" + +``` + ### Options ``` + --all specify this to target all hosts in the inventory -h, --help help for poweron -l, --labels string Label(s) to filter desired baremetal host documents --name string Name to filter desired baremetal host document diff --git a/docs/source/cli/airshipctl_baremetal_reboot.md b/docs/source/cli/airshipctl_baremetal_reboot.md index 532f88230..0bec99d90 100644 --- a/docs/source/cli/airshipctl_baremetal_reboot.md +++ b/docs/source/cli/airshipctl_baremetal_reboot.md @@ -1,18 +1,40 @@ ## airshipctl baremetal reboot -Reboot a host +Reboot a hosts ### Synopsis -Reboot a host +Reboot baremetal hosts +The command will target baremetal hosts from airship inventory kustomize root +based on the --name, --namespace and --labels flags provided. If no flags are +provided airshipctl will try to select all baremetal hosts in the inventory + ``` airshipctl baremetal reboot [flags] ``` +### Examples + +``` +Perform action against hosts with name rdm9r3s3 in all namespaces where the host is found +# airshipctl baremetal reboot --name rdm9r3s3 + +Perform action against hosts with name rdm9r3s3 in namespace metal3 +# airshipctl baremetal reboot --name rdm9r3s3 --namespace metal3 + +Perform action against all hosts defined in inventory +# airshipctl baremetal reboot --all + +Perform action against hosts with a label 'foo=bar' +# airshipctl baremetal reboot --labels "foo=bar" + +``` + ### Options ``` + --all specify this to target all hosts in the inventory -h, --help help for reboot -l, --labels string Label(s) to filter desired baremetal host documents --name string Name to filter desired baremetal host document diff --git a/pkg/inventory/command.go b/pkg/inventory/command.go index b7c382389..13fabc54e 100644 --- a/pkg/inventory/command.go +++ b/pkg/inventory/command.go @@ -26,6 +26,8 @@ import ( // CommandOptions is used to store common variables from cmd flags for baremetal command group type CommandOptions struct { + All bool + Labels string Name string Namespace string @@ -42,12 +44,34 @@ func NewOptions(i ifc.Inventory) *CommandOptions { } } +func (o *CommandOptions) validateBMHAction() error { + if o.Name == "" && o.Namespace == "" && o.Labels == "" && !o.All { + return ErrInvalidOptions{Message: `must provide atleast one of the following options: ` + + `'name', 'namespace', 'labels' or 'all'`} + } else if o.All && (o.Name != "" || o.Namespace != "" || o.Labels != "") { + return ErrInvalidOptions{Message: "option 'all' can not be combined with other host selector options"} + } + return nil +} + +func (o *CommandOptions) validateSingleHostAction() error { + if o.Name == "" && o.Namespace == "" && o.Labels == "" { + return ErrInvalidOptions{Message: "No options are specified, must provide atleast 'name', 'namespace' or 'labels'"} + } + return nil +} + // BMHAction performs an action against BaremetalHost objects func (o *CommandOptions) BMHAction(op ifc.BaremetalOperation) error { + if err := o.validateBMHAction(); err != nil { + return err + } + bmhInventory, err := o.Inventory.BaremetalInventory() if err != nil { return err } + ctx, cancel := context.WithTimeout(context.Background(), o.Timeout) defer cancel() return bmhInventory.RunOperation( @@ -59,6 +83,9 @@ func (o *CommandOptions) BMHAction(op ifc.BaremetalOperation) error { // RemoteDirect perform RemoteDirect operation against single host func (o *CommandOptions) RemoteDirect() error { + if err := o.validateSingleHostAction(); err != nil { + return err + } host, err := o.getHost() if err != nil { return err @@ -70,6 +97,9 @@ func (o *CommandOptions) RemoteDirect() error { // PowerStatus get power status of the single host func (o *CommandOptions) PowerStatus(w io.Writer) error { + if err := o.validateSingleHostAction(); err != nil { + return err + } host, err := o.getHost() if err != nil { return err diff --git a/pkg/inventory/command_test.go b/pkg/inventory/command_test.go index 912d45bd4..c112c9507 100644 --- a/pkg/inventory/command_test.go +++ b/pkg/inventory/command_test.go @@ -20,6 +20,7 @@ import ( "testing" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" "opendev.org/airship/airshipctl/pkg/inventory" "opendev.org/airship/airshipctl/pkg/inventory/ifc" @@ -28,6 +29,8 @@ import ( "opendev.org/airship/airshipctl/testutil/redfishutils" ) +const testNode = "node-0" + func TestCommandOptions(t *testing.T) { t.Run("error BMHAction bmh inventory", func(t *testing.T) { inv := &mockinventory.MockInventory{} @@ -35,10 +38,31 @@ func TestCommandOptions(t *testing.T) { inv.On("BaremetalInventory").Once().Return(nil, expectedErr) co := inventory.NewOptions(inv) + co.All = true actualErr := co.BMHAction(ifc.BaremetalOperationPowerOn) assert.Equal(t, expectedErr, actualErr) }) + t.Run("error BMHAction invalid empty options", func(t *testing.T) { + inv := &mockinventory.MockInventory{} + + co := inventory.NewOptions(inv) + err := co.BMHAction(ifc.BaremetalOperationPowerOn) + require.Error(t, err) + assert.Contains(t, err.Error(), (inventory.ErrInvalidOptions{}).Error()) + }) + + t.Run("error BMHAction invalid both all and other selectors", func(t *testing.T) { + inv := &mockinventory.MockInventory{} + + co := inventory.NewOptions(inv) + co.All = true + co.Labels = "foo=bar" + err := co.BMHAction(ifc.BaremetalOperationPowerOn) + require.Error(t, err) + assert.Contains(t, err.Error(), (inventory.ErrInvalidOptions{}).Error()) + }) + t.Run("success BMHAction", func(t *testing.T) { bmhInv := &mockinventory.MockBMHInventory{} bmhInv.On("RunOperation").Once().Return(nil) @@ -47,6 +71,7 @@ func TestCommandOptions(t *testing.T) { inv.On("BaremetalInventory").Once().Return(bmhInv, nil) co := inventory.NewOptions(inv) + co.All = true actualErr := co.BMHAction(ifc.BaremetalOperationPowerOn) assert.Equal(t, nil, actualErr) }) @@ -60,6 +85,7 @@ func TestCommandOptions(t *testing.T) { inv.On("BaremetalInventory").Once().Return(bmhInv, nil) co := inventory.NewOptions(inv) + co.Name = testNode buf := bytes.NewBuffer([]byte{}) actualErr := co.PowerStatus(buf) assert.Equal(t, expectedErr, actualErr) @@ -73,6 +99,7 @@ func TestCommandOptions(t *testing.T) { inv.On("BaremetalInventory").Once().Return(nil, expectedErr) co := inventory.NewOptions(inv) + co.Name = testNode buf := bytes.NewBuffer([]byte{}) actualErr := co.PowerStatus(buf) assert.Equal(t, expectedErr, actualErr) @@ -91,6 +118,7 @@ func TestCommandOptions(t *testing.T) { inv.On("BaremetalInventory").Once().Return(bmhInv, nil) co := inventory.NewOptions(inv) + co.Name = testNode buf := bytes.NewBuffer([]byte{}) actualErr := co.PowerStatus(buf) assert.Equal(t, expectedErr, actualErr) @@ -110,6 +138,7 @@ func TestCommandOptions(t *testing.T) { inv.On("BaremetalInventory").Once().Return(bmhInv, nil) co := inventory.NewOptions(inv) + co.Name = testNode buf := bytes.NewBuffer([]byte{}) actualErr := co.PowerStatus(buf) assert.Equal(t, nil, actualErr) @@ -128,6 +157,7 @@ func TestCommandOptions(t *testing.T) { inv.On("BaremetalInventory").Once().Return(bmhInv, nil) co := inventory.NewOptions(inv) + co.Name = testNode co.IsoURL = "http://some-url" actualErr := co.RemoteDirect() assert.Equal(t, nil, actualErr) @@ -144,6 +174,7 @@ func TestCommandOptions(t *testing.T) { inv.On("BaremetalInventory").Once().Return(bmhInv, nil) co := inventory.NewOptions(inv) + co.Name = testNode actualErr := co.RemoteDirect() // Simply check if error is returned in isoURL is not specified assert.Error(t, actualErr) @@ -156,7 +187,29 @@ func TestCommandOptions(t *testing.T) { inv.On("BaremetalInventory").Once().Return(nil, expectedErr) co := inventory.NewOptions(inv) + co.Name = testNode actualErr := co.RemoteDirect() assert.Equal(t, expectedErr, actualErr) }) + + t.Run("error RemoteDirect invalid options", func(t *testing.T) { + inv := &mockinventory.MockInventory{} + + co := inventory.NewOptions(inv) + err := co.RemoteDirect() + require.Error(t, err) + assert.Contains(t, err.Error(), (inventory.ErrInvalidOptions{}).Error()) + }) + + t.Run("error RemoteDirect invalid options", func(t *testing.T) { + inv := &mockinventory.MockInventory{} + + co := inventory.NewOptions(inv) + buf := bytes.NewBuffer([]byte{}) + err := co.PowerStatus(buf) + + require.Error(t, err) + assert.Contains(t, err.Error(), (inventory.ErrInvalidOptions{}).Error()) + assert.Len(t, buf.Bytes(), 0) + }) } diff --git a/pkg/inventory/errors.go b/pkg/inventory/errors.go new file mode 100644 index 000000000..dd66ed4b8 --- /dev/null +++ b/pkg/inventory/errors.go @@ -0,0 +1,26 @@ +/* + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + https://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +*/ + +package inventory + +import "fmt" + +// ErrInvalidOptions is returned when incompetible flags are +type ErrInvalidOptions struct { + Message string +} + +func (e ErrInvalidOptions) Error() string { + return fmt.Sprintf("invalid options are supplied: %s", e.Message) +} diff --git a/pkg/inventory/testdata/hosts.yaml b/pkg/inventory/testdata/hosts.yaml index 5b7065c77..d59696264 100644 --- a/pkg/inventory/testdata/hosts.yaml +++ b/pkg/inventory/testdata/hosts.yaml @@ -4,10 +4,10 @@ kind: BareMetalHost metadata: labels: airshipit.org/ephemeral-node: "true" - name: master-0 + name: node-0 spec: online: true bootMACAddress: 00:3b:8b:0c:ec:8b bmc: address: redfish+http://nolocalhost:32201/redfish/v1/Systems/ephemeral - credentialsName: master-0-bmc-secret \ No newline at end of file + credentialsName: node-0-bmc-secret \ No newline at end of file