Change logic of config set-management-config command

* command internal logic was revised and optimized
 * added ability to delete/reset management config fields
 * unit tests were reorganized, new cases were added

Change-Id: I911df180e6986cb1f4dab6aea01216c8a4e15353
Signed-off-by: Ruslan Aliev <raliev@mirantis.com>
Relates-To: #348
This commit is contained in:
Ruslan Aliev 2021-03-30 01:48:07 -05:00
parent 3dd66f7685
commit 1684ad45cf
18 changed files with 272 additions and 256 deletions

View File

@ -15,9 +15,8 @@
package config
import (
"fmt"
"github.com/spf13/cobra"
"github.com/spf13/pflag"
"opendev.org/airship/airshipctl/pkg/config"
"opendev.org/airship/airshipctl/pkg/remote/redfish"
@ -32,74 +31,67 @@ const (
flagUseProxy = "use-proxy"
flagUseProxyDescription = "Use the proxy configuration specified in the local environment"
flagSystemActionRetries = "system-action-retries"
flagSystemActionRetriesDescription = "Set the number of attempts to poll a host for a status"
flagSystemRebootDelay = "system-reboot-delay"
flagSystemRebootDelayDescription = "Set the number of seconds to wait between power actions (e.g. shutdown, startup)"
)
// NewSetManagementConfigCommand creates a command for creating and modifying clusters
// in the airshipctl config file.
func NewSetManagementConfigCommand(cfgFactory config.Factory) *cobra.Command {
var insecure bool
var managementType string
var useProxy bool
o := &config.ManagementConfiguration{}
cmd := &cobra.Command{
Use: "set-management-config NAME",
Short: "Modify an out-of-band management configuration",
Args: cobra.ExactArgs(1),
RunE: func(cmd *cobra.Command, args []string) error {
cfg, err := cfgFactory()
if err != nil {
return err
}
name := args[0]
managementCfg, err := cfg.GetManagementConfiguration(name)
if err != nil {
return err
}
var modified bool
if cmd.Flags().Changed(flagInsecure) && insecure != managementCfg.Insecure {
modified = true
managementCfg.Insecure = insecure
fmt.Fprintf(cmd.OutOrStdout(),
"Option 'insecure' set to '%t' for management configuration '%s'.\n",
managementCfg.Insecure, name)
}
if cmd.Flags().Changed(flagManagementType) && managementType != managementCfg.Type {
modified = true
if err = managementCfg.SetType(managementType); err != nil {
return err
}
fmt.Fprintf(cmd.OutOrStdout(),
"Option 'type' set to '%s' for management configuration '%s'.\n",
managementCfg.Type, name)
}
if cmd.Flags().Changed(flagUseProxy) && useProxy != managementCfg.UseProxy {
modified = true
managementCfg.UseProxy = useProxy
fmt.Fprintf(cmd.OutOrStdout(),
"Option 'useproxy' set to '%t' for management configuration '%s'\n",
managementCfg.UseProxy, name)
}
if !modified {
fmt.Fprintf(cmd.OutOrStdout(),
"Management configuration '%s' not modified. No new settings.\n", name)
return nil
}
return cfg.PersistConfig(true)
},
RunE: setManagementConfigRunE(cfgFactory, o),
}
flags := cmd.Flags()
flags.BoolVar(&insecure, flagInsecure, false, flagInsecureDescription)
flags.StringVar(&managementType, flagManagementType, redfish.ClientType, flagManagementTypeDescription)
flags.BoolVar(&useProxy, flagUseProxy, true, flagUseProxyDescription)
addSetManagementConfigFlags(cmd, o)
return cmd
}
func addSetManagementConfigFlags(cmd *cobra.Command, o *config.ManagementConfiguration) {
flags := cmd.Flags()
flags.BoolVar(&o.Insecure, flagInsecure, false, flagInsecureDescription)
flags.StringVar(&o.Type, flagManagementType, redfish.ClientType, flagManagementTypeDescription)
flags.BoolVar(&o.UseProxy, flagUseProxy, true, flagUseProxyDescription)
flags.IntVar(&o.SystemActionRetries, flagSystemActionRetries,
config.DefaultSystemActionRetries, flagSystemActionRetriesDescription)
flags.IntVar(&o.SystemRebootDelay, flagSystemRebootDelay,
config.DefaultSystemRebootDelay, flagSystemRebootDelayDescription)
}
func setManagementConfigRunE(cfgFactory config.Factory, o *config.ManagementConfiguration) func(
cmd *cobra.Command, args []string) error {
return func(cmd *cobra.Command, args []string) error {
// Go through all the flags that have been set
var opts []config.ManagementConfigOption
fn := func(flag *pflag.Flag) {
switch flag.Name {
case flagInsecure:
opts = append(opts, config.SetManagementConfigInsecure(o.Insecure))
case flagManagementType:
opts = append(opts, config.SetManagementConfigMgmtType(o.Type))
case flagUseProxy:
opts = append(opts, config.SetManagementConfigUseProxy(o.UseProxy))
case flagSystemActionRetries:
opts = append(opts, config.SetManagementConfigSystemActionRetries(o.SystemActionRetries))
case flagSystemRebootDelay:
opts = append(opts, config.SetManagementConfigSystemRebootDelay(o.SystemRebootDelay))
}
}
cmd.Flags().Visit(fn)
options := &config.RunSetManagementConfigOptions{
CfgFactory: cfgFactory,
MgmtCfgName: args[0],
Writer: cmd.OutOrStdout(),
}
return options.RunSetManagementConfig(opts...)
}
}

View File

@ -15,164 +15,22 @@
package config_test
import (
"fmt"
"testing"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
cmd "opendev.org/airship/airshipctl/cmd/config"
"opendev.org/airship/airshipctl/pkg/config"
redfishdell "opendev.org/airship/airshipctl/pkg/remote/redfish/vendors/dell"
"opendev.org/airship/airshipctl/testutil"
)
func TestConfigSetManagementConfigurationCmd(t *testing.T) {
conf, cleanup := testutil.InitConfig(t)
defer cleanup(t)
settings := func() (*config.Config, error) {
return conf, nil
}
cmdTests := []*testutil.CmdTest{
{
Name: "config-cmd-set-management-config-with-help",
CmdLine: "--help",
Cmd: cmd.NewSetManagementConfigCommand(nil),
},
{
Name: "config-cmd-set-management-config-no-args",
CmdLine: "",
Cmd: cmd.NewSetManagementConfigCommand(nil),
Error: fmt.Errorf("accepts %d arg(s), received %d", 1, 0),
},
{
Name: "config-cmd-set-management-config-excess-args",
CmdLine: "arg1 arg2",
Cmd: cmd.NewSetManagementConfigCommand(nil),
Error: fmt.Errorf("accepts %d arg(s), received %d", 1, 2),
},
{
Name: "config-cmd-set-management-config-not-found",
CmdLine: fmt.Sprintf("%s-test", config.AirshipDefaultContext),
Cmd: cmd.NewSetManagementConfigCommand(settings),
Error: config.ErrManagementConfigurationNotFound{
Name: fmt.Sprintf("%s-test", config.AirshipDefaultContext),
},
},
}
for _, tt := range cmdTests {
testutil.RunTest(t, tt)
}
}
func TestConfigSetManagementConfigurationInsecure(t *testing.T) {
conf, cleanup := testutil.InitConfig(t)
defer cleanup(t)
conf.ManagementConfiguration[config.AirshipDefaultManagementConfiguration] = config.NewManagementConfiguration()
settings := func() (*config.Config, error) {
return conf, nil
}
require.False(t, conf.ManagementConfiguration[config.AirshipDefaultContext].Insecure)
testutil.RunTest(t, &testutil.CmdTest{
Name: "config-cmd-set-management-config-change-insecure",
CmdLine: fmt.Sprintf("%s --insecure=true", config.AirshipDefaultContext),
Cmd: cmd.NewSetManagementConfigCommand(settings),
})
testutil.RunTest(t, &testutil.CmdTest{
Name: "config-cmd-set-management-config-no-change",
CmdLine: fmt.Sprintf("%s --insecure=true", config.AirshipDefaultContext),
Cmd: cmd.NewSetManagementConfigCommand(settings),
})
assert.True(t, conf.ManagementConfiguration[config.AirshipDefaultContext].Insecure)
}
func TestConfigSetManagementConfigurationType(t *testing.T) {
conf, cleanup := testutil.InitConfig(t)
defer cleanup(t)
conf.ManagementConfiguration[config.AirshipDefaultManagementConfiguration] = config.NewManagementConfiguration()
settings := func() (*config.Config, error) {
return conf, nil
}
require.NotEqual(t, redfishdell.ClientType,
conf.ManagementConfiguration[config.AirshipDefaultContext].Type)
cmdTests := []*testutil.CmdTest{
{
Name: "config-cmd-set-management-config-unknown-type",
CmdLine: fmt.Sprintf("%s --management-type=foo", config.AirshipDefaultContext),
Cmd: cmd.NewSetManagementConfigCommand(settings),
Error: config.ErrUnknownManagementType{Type: "foo"},
},
{
Name: "config-cmd-set-management-config-change-type",
CmdLine: fmt.Sprintf("%s --management-type=%s", config.AirshipDefaultContext,
redfishdell.ClientType),
Cmd: cmd.NewSetManagementConfigCommand(settings),
},
}
for _, tt := range cmdTests {
testutil.RunTest(t, tt)
}
assert.Equal(t, redfishdell.ClientType,
conf.ManagementConfiguration[config.AirshipDefaultContext].Type)
}
func TestConfigSetManagementConfigurationUseProxy(t *testing.T) {
conf, cleanup := testutil.InitConfig(t)
defer cleanup(t)
conf.ManagementConfiguration[config.AirshipDefaultManagementConfiguration] = config.NewManagementConfiguration()
settings := func() (*config.Config, error) {
return conf, nil
}
require.False(t, conf.ManagementConfiguration[config.AirshipDefaultContext].UseProxy)
testutil.RunTest(t, &testutil.CmdTest{
Name: "config-cmd-set-management-config-change-use-proxy",
CmdLine: fmt.Sprintf("%s --use-proxy=true", config.AirshipDefaultContext),
Cmd: cmd.NewSetManagementConfigCommand(settings),
})
assert.True(t, conf.ManagementConfiguration[config.AirshipDefaultContext].UseProxy)
}
func TestConfigSetManagementConfigurationMultipleOptions(t *testing.T) {
conf, cleanup := testutil.InitConfig(t)
defer cleanup(t)
conf.ManagementConfiguration[config.AirshipDefaultManagementConfiguration] = config.NewManagementConfiguration()
settings := func() (*config.Config, error) {
return conf, nil
}
require.NotEqual(t, redfishdell.ClientType,
conf.ManagementConfiguration[config.AirshipDefaultContext].Type)
require.False(t, conf.ManagementConfiguration[config.AirshipDefaultContext].UseProxy)
testutil.RunTest(t, &testutil.CmdTest{
Name: "config-cmd-set-management-config-change-type",
CmdLine: fmt.Sprintf("%s --management-type=%s --use-proxy=true", config.AirshipDefaultContext,
redfishdell.ClientType),
Cmd: cmd.NewSetManagementConfigCommand(settings),
})
assert.Equal(t, redfishdell.ClientType,
conf.ManagementConfiguration[config.AirshipDefaultContext].Type)
assert.True(t, conf.ManagementConfiguration[config.AirshipDefaultContext].UseProxy)
}

View File

@ -1,10 +0,0 @@
Error: accepts 1 arg(s), received 2
Usage:
set-management-config NAME [flags]
Flags:
-h, --help help for set-management-config
--insecure Ignore SSL certificate verification on out-of-band management requests
--management-type string Set the out-of-band management type (default "redfish")
--use-proxy Use the proxy configuration specified in the local environment (default true)

View File

@ -1,10 +0,0 @@
Error: accepts 1 arg(s), received 0
Usage:
set-management-config NAME [flags]
Flags:
-h, --help help for set-management-config
--insecure Ignore SSL certificate verification on out-of-band management requests
--management-type string Set the out-of-band management type (default "redfish")
--use-proxy Use the proxy configuration specified in the local environment (default true)

View File

@ -1,10 +0,0 @@
Error: Unknown management configuration 'default-test'.
Usage:
set-management-config NAME [flags]
Flags:
-h, --help help for set-management-config
--insecure Ignore SSL certificate verification on out-of-band management requests
--management-type string Set the out-of-band management type (default "redfish")
--use-proxy Use the proxy configuration specified in the local environment (default true)

View File

@ -4,7 +4,9 @@ Usage:
set-management-config NAME [flags]
Flags:
-h, --help help for set-management-config
--insecure Ignore SSL certificate verification on out-of-band management requests
--management-type string Set the out-of-band management type (default "redfish")
--use-proxy Use the proxy configuration specified in the local environment (default true)
-h, --help help for set-management-config
--insecure Ignore SSL certificate verification on out-of-band management requests
--management-type string Set the out-of-band management type (default "redfish")
--system-action-retries int Set the number of attempts to poll a host for a status (default 30)
--system-reboot-delay int Set the number of seconds to wait between power actions (e.g. shutdown, startup) (default 30)
--use-proxy Use the proxy configuration specified in the local environment (default true)

View File

@ -1 +0,0 @@
Option 'insecure' set to 'true' for management configuration 'default'.

View File

@ -1 +0,0 @@
Management configuration 'default' not modified. No new settings.

View File

@ -1,2 +0,0 @@
Option 'type' set to 'redfish-dell' for management configuration 'default'.
Option 'useproxy' set to 'true' for management configuration 'default'

View File

@ -1 +0,0 @@
Option 'type' set to 'redfish-dell' for management configuration 'default'.

View File

@ -1,10 +0,0 @@
Error: Unknown management type 'foo'. Known types include 'redfish' and 'redfish-dell'.
Usage:
set-management-config NAME [flags]
Flags:
-h, --help help for set-management-config
--insecure Ignore SSL certificate verification on out-of-band management requests
--management-type string Set the out-of-band management type (default "redfish")
--use-proxy Use the proxy configuration specified in the local environment (default true)

View File

@ -1 +0,0 @@
Option 'useproxy' set to 'true' for management configuration 'default'

View File

@ -13,10 +13,12 @@ airshipctl config set-management-config NAME [flags]
### Options
```
-h, --help help for set-management-config
--insecure Ignore SSL certificate verification on out-of-band management requests
--management-type string Set the out-of-band management type (default "redfish")
--use-proxy Use the proxy configuration specified in the local environment (default true)
-h, --help help for set-management-config
--insecure Ignore SSL certificate verification on out-of-band management requests
--management-type string Set the out-of-band management type (default "redfish")
--system-action-retries int Set the number of attempts to poll a host for a status (default 30)
--system-reboot-delay int Set the number of seconds to wait between power actions (e.g. shutdown, startup) (default 30)
--use-proxy Use the proxy configuration specified in the local environment (default true)
```
### Options inherited from parent commands

View File

@ -537,3 +537,21 @@ func (c *Config) WorkDir() (dir string, err error) {
}
return dir, err
}
// AddManagementConfig creates a new instance of ManagementConfig object
func (c *Config) AddManagementConfig(mgmtCfgName string, opts ...ManagementConfigOption) *ManagementConfiguration {
// Create the new Airshipctl config ManagementConfig
nMgmtCfg := NewManagementConfiguration()
c.ManagementConfiguration[mgmtCfgName] = nMgmtCfg
// We can use Modify to populate the correct information
c.ModifyManagementConfig(nMgmtCfg, opts...)
return nMgmtCfg
}
// ModifyManagementConfig updates ManagementConfig object with given options
func (c *Config) ModifyManagementConfig(mgmtConfig *ManagementConfiguration, opts ...ManagementConfigOption) {
for _, o := range opts {
o(mgmtConfig)
}
}

View File

@ -83,6 +83,86 @@ func (o *RunSetContextOptions) RunSetContext(opts ...ContextOption) error {
return cfg.PersistConfig(true)
}
// ManagementConfigOption is a function that allows to modify ManagementConfig object
type ManagementConfigOption func(mgmtConf *ManagementConfiguration)
// SetManagementConfigInsecure sets Insecure option in ManagementConfig object
func SetManagementConfigInsecure(insecure bool) ManagementConfigOption {
return func(mgmtConf *ManagementConfiguration) {
mgmtConf.Insecure = insecure
}
}
// SetManagementConfigMgmtType sets Type in ManagementConfig object
func SetManagementConfigMgmtType(mgmtType string) ManagementConfigOption {
return func(mgmtCfg *ManagementConfiguration) {
mgmtCfg.Type = mgmtType
}
}
// SetManagementConfigUseProxy sets UseProxy in ManagementConfig object
func SetManagementConfigUseProxy(useProxy bool) ManagementConfigOption {
return func(mgmtCfg *ManagementConfiguration) {
mgmtCfg.UseProxy = useProxy
}
}
// SetManagementConfigSystemActionRetries sets SystemActionRetries in ManagementConfig object
func SetManagementConfigSystemActionRetries(sysActionRetries int) ManagementConfigOption {
return func(mgmtCfg *ManagementConfiguration) {
mgmtCfg.SystemActionRetries = sysActionRetries
}
}
// SetManagementConfigSystemRebootDelay sets SystemRebootDelay in ManagementConfig object
func SetManagementConfigSystemRebootDelay(sysRebootDelay int) ManagementConfigOption {
return func(mgmtCfg *ManagementConfiguration) {
mgmtCfg.SystemRebootDelay = sysRebootDelay
}
}
// RunSetManagementConfigOptions are options required to create/modify airshipctl management config
type RunSetManagementConfigOptions struct {
CfgFactory Factory
MgmtCfgName string
Writer io.Writer
}
// RunSetManagementConfig validates the given command line options and invokes add/modify ManagementConfig
func (o *RunSetManagementConfigOptions) RunSetManagementConfig(opts ...ManagementConfigOption) error {
cfg, err := o.CfgFactory()
if err != nil {
return err
}
if o.MgmtCfgName == "" {
return ErrEmptyManagementConfigurationName{}
}
infoMsg := fmt.Sprintf("management configuration with name %s", o.MgmtCfgName)
mgmtCfg, err := cfg.GetManagementConfiguration(o.MgmtCfgName)
if err != nil {
// context didn't exist, create it
cfg.AddManagementConfig(o.MgmtCfgName, opts...)
infoMsg = fmt.Sprintf("%s created\n", infoMsg)
} else {
// Context exists, lets update
cfg.ModifyManagementConfig(mgmtCfg, opts...)
infoMsg = fmt.Sprintf("%s modified\n", infoMsg)
}
// Verify we didn't break anything
if err = cfg.EnsureComplete(); err != nil {
return err
}
if _, err := o.Writer.Write([]byte(infoMsg)); err != nil {
return err
}
// Update configuration file just in time persistence approach
return cfg.PersistConfig(true)
}
// RunUseContext validates the given context name and updates it as current context
func RunUseContext(desiredContext string, airconfig *Config) error {
if _, err := airconfig.GetContext(desiredContext); err != nil {

View File

@ -128,6 +128,88 @@ func TestRunSetContext(t *testing.T) {
}
}
func TestRunSetManagementConfig(t *testing.T) {
ioBuffer := bytes.NewBuffer(nil)
tests := []struct {
name string
options config.RunSetManagementConfigOptions
mgmtCfgOpts []config.ManagementConfigOption
expectedOut string
err error
}{
{
name: "create new mgmt cfg",
options: config.RunSetManagementConfigOptions{
CfgFactory: prepareConfig(),
MgmtCfgName: "new_mgmt_cfg",
Writer: ioBuffer,
},
mgmtCfgOpts: []config.ManagementConfigOption{config.SetManagementConfigInsecure(true),
config.SetManagementConfigUseProxy(true), config.SetManagementConfigMgmtType("redfish")},
err: nil,
expectedOut: "management configuration with name new_mgmt_cfg created\n",
},
{
name: "modify mgmt cfg",
options: config.RunSetManagementConfigOptions{
CfgFactory: prepareConfig(),
MgmtCfgName: "dummy_management_config",
Writer: ioBuffer,
},
mgmtCfgOpts: []config.ManagementConfigOption{config.SetManagementConfigSystemRebootDelay(25),
config.SetManagementConfigSystemActionRetries(20)},
err: nil,
expectedOut: "management configuration with name dummy_management_config modified\n",
},
{
name: "bad config",
options: config.RunSetManagementConfigOptions{
CfgFactory: func() (*config.Config, error) {
return nil, config.ErrMissingConfig{What: "bad config"}
},
},
err: config.ErrMissingConfig{What: "bad config"},
},
{
name: "no mgmt cfg name provided",
options: config.RunSetManagementConfigOptions{
CfgFactory: prepareConfig(),
MgmtCfgName: "",
},
err: config.ErrEmptyManagementConfigurationName{},
},
{
name: "incomplete config",
options: config.RunSetManagementConfigOptions{
CfgFactory: func() (*config.Config, error) {
cfg := testutil.DummyConfig()
cfg.Contexts = map[string]*config.Context{}
return cfg, nil
},
MgmtCfgName: "dummy_management_config",
},
err: config.ErrMissingConfig{What: "At least one Context needs to be defined"},
},
}
for _, tt := range tests {
tt := tt
t.Run(tt.name, func(t *testing.T) {
ioBuffer.Reset()
err := tt.options.RunSetManagementConfig(tt.mgmtCfgOpts...)
if tt.err != nil {
require.Error(t, err)
require.Equal(t, tt.err, err)
} else {
require.NoError(t, err)
}
if tt.expectedOut != "" {
require.Equal(t, tt.expectedOut, ioBuffer.String())
}
})
}
}
func TestRunUseContext(t *testing.T) {
t.Run("testUseContext", func(t *testing.T) {
conf := testutil.DummyConfig()

View File

@ -521,3 +521,23 @@ func TestWorkDir(t *testing.T) {
assert.NoError(t, err)
assert.NotEmpty(t, wd)
}
func TestAddManagementConfig(t *testing.T) {
conf, cleanup := testutil.InitConfig(t)
defer cleanup(t)
managementConfig := conf.AddManagementConfig("new_mgmt_context", config.SetManagementConfigUseProxy(false))
assert.EqualValues(t, conf.ManagementConfiguration["new_mgmt_context"], managementConfig)
}
func TestModifyManagementConfig(t *testing.T) {
conf, cleanup := testutil.InitConfig(t)
defer cleanup(t)
managementConfig := conf.AddManagementConfig("modified_mgmt_config")
conf.ModifyManagementConfig(managementConfig, config.SetManagementConfigSystemActionRetries(60))
assert.EqualValues(t, conf.ManagementConfiguration["modified_mgmt_config"].SystemActionRetries,
managementConfig.SystemActionRetries)
assert.EqualValues(t, conf.ManagementConfiguration["modified_mgmt_config"], managementConfig)
}

View File

@ -144,6 +144,14 @@ func (e ErrManagementConfigurationNotFound) Error() string {
return fmt.Sprintf("Unknown management configuration '%s'.", e.Name)
}
// ErrEmptyManagementConfigurationName returned when attempted to create/modify management config with empty name
type ErrEmptyManagementConfigurationName struct {
}
func (e ErrEmptyManagementConfigurationName) Error() string {
return fmt.Sprintf("management config name must not be empty")
}
// ErrMissingCurrentContext returned in case --current used without setting current-context
type ErrMissingCurrentContext struct {
}