Merge pull request #14036 from barrettj12/cfgbase-ctr

#14036

Continues work done in #13983 and #14014. I had to make a small retroactive change to ConfigCommandBase (allowing child commands to disable --reset flag), since the controller config API doesn't support resetting values. **Reviewers:** it will make more sense looking at the commits individually.

Again, there will be slight behaviour changes to the `controller-config` CLI:
- Config yaml files must now be specified using the --file flag.
- We no longer allow multiple actions to be done simultaneously (e.g. setting from key=value args & setting from file), as this creates ambiguity as to which should be done first. As a result, I removed several unit tests in `configcommand_test.go` which didn't make sense anymore.

cmd/juju/application/config.go

@@ -99,10 +99,14 @@ See also:
 `
 )
 
+var appConfigBase = config.ConfigCommandBase{
+	Resettable: true,
+}
+
 // NewConfigCommand returns a command used to get, reset, and set application
 // charm attributes.
 func NewConfigCommand() cmd.Command {
-	return modelcmd.Wrap(&configCommand{})
+	return modelcmd.Wrap(&configCommand{configBase: appConfigBase})
 }
 
 // configCommand get, sets, and resets configuration values of an application' charm.

cmd/juju/application/export_test.go

@@ -246,7 +246,7 @@ func NewShowUnitCommandForTest(api UnitsInfoAPI, store jujuclient.ClientStore) c
 
 // NewConfigCommandForTest returns a SetCommand with the api provided as specified.
 func NewConfigCommandForTest(api ApplicationAPI, store jujuclient.ClientStore) modelcmd.ModelCommand {
-	c := modelcmd.Wrap(&configCommand{api: api})
+	c := modelcmd.Wrap(&configCommand{configBase: appConfigBase, api: api})
 	c.SetClientStore(store)
 	return c
 }

cmd/juju/config/config.go

@@ -33,7 +33,8 @@ const (
 // methods.
 type ConfigCommandBase struct {
 	// Fields to be set by child
-	CantReset []string // keys which can't be reset
+	Resettable bool     // does this command allow resetting config values?
+	CantReset  []string // keys which can't be reset
 
 	// Flag values
 	ConfigFile cmd.FileVar
@@ -51,8 +52,10 @@ type ConfigCommandBase struct {
 // SetFlags implements cmd.Command.SetFlags.
 func (c *ConfigCommandBase) SetFlags(f *gnuflag.FlagSet) {
 	f.Var(&c.ConfigFile, "file", "path to yaml-formatted configuration file")
-	f.Var(cmd.NewAppendStringsValue(&c.reset), "reset",
-		"Reset the provided comma delimited keys, deletes keys not in the model config")
+	if c.Resettable {
+		f.Var(cmd.NewAppendStringsValue(&c.reset), "reset",
+			"Reset the provided comma delimited keys, deletes keys not in the model config")
+	}
 }
 
 // Init provides a basic implementation of cmd.Command.Init.
@@ -68,7 +71,7 @@ func (c *ConfigCommandBase) Init(args []string) error {
 	}
 
 	// Check if --reset has been specified
-	if len(c.reset) != 0 {
+	if c.Resettable && len(c.reset) != 0 {
 		c.Actions = append(c.Actions, Reset)
 		err := c.parseResetKeys()
 		if err != nil {

cmd/juju/config/config_test.go

@@ -22,31 +22,40 @@ func TestPackage(t *stdtesting.T) {
 }
 
 func (s *suite) TestSetFlags(c *gc.C) {
-	cmd := ConfigCommandBase{}
-	f := flagSetForTest(c)
-	cmd.SetFlags(f)
+	for _, resettable := range []bool{true, false} {
+		cmd := ConfigCommandBase{Resettable: resettable}
+		f := flagSetForTest(c)
+		cmd.SetFlags(f)
 
-	// Collect all flags
-	expectedFlags := []string{"file", "reset"}
-	flags := []string{}
+		// Collect all flags
+		expectedFlags := []string{"file"}
+		if resettable {
+			expectedFlags = append(expectedFlags, "reset")
+		}
+
+		flags := []string{}
+		f.VisitAll(
+			func(f *gnuflag.Flag) { flags = append(flags, f.Name) },
+		)
+		c.Check(flags, jc.SameContents, expectedFlags)
+
+		c.Check(sliceContains(flags, "reset"), gc.Equals, resettable)
+	}
 
-	f.VisitAll(
-		func(f *gnuflag.Flag) { flags = append(flags, f.Name) },
-	)
-	c.Assert(flags, gc.DeepEquals, expectedFlags)
 }
 
 // parseFailTest holds args for which we expect parsing to fail.
 type parseFailTest struct {
-	about  string
-	args   []string
-	errMsg string
+	about      string
+	resettable bool
+	args       []string
+	errMsg     string
 }
 
 // testParse checks that parsing of the given args fails or succeeds
 // (depending on the value of `fail`).
 func testParse(c *gc.C, test parseFailTest, fail bool) {
-	cmd := ConfigCommandBase{}
+	cmd := ConfigCommandBase{Resettable: test.resettable}
 	parseFail := false
 	var errWriter bytes.Buffer
 
@@ -89,7 +98,8 @@ type initTest struct {
 // TestInitSucceed.
 func setupInitTest(c *gc.C, args []string, cantReset []string) (ConfigCommandBase, error) {
 	cmd := ConfigCommandBase{
-		CantReset: cantReset,
+		Resettable: true,
+		CantReset:  cantReset,
 	}
 	f := flagSetForTest(c)
 	cmd.SetFlags(f)
@@ -110,7 +120,7 @@ func (s *suite) TestInitFail(c *gc.C) {
 	for i, test := range initFailTests {
 		c.Logf("test %d: %s", i, test.about)
 		// Check parsing succeeds
-		testParse(c, parseFailTest{args: test.args}, false)
+		testParse(c, parseFailTest{resettable: true, args: test.args}, false)
 
 		_, err := setupInitTest(c, test.args, test.cantReset)
 		c.Check(err, gc.ErrorMatches, test.errMsg)
@@ -121,7 +131,7 @@ func (s *suite) TestInitSuccess(c *gc.C) {
 	for i, test := range initTests {
 		c.Logf("test %d: %s", i, test.about)
 		// Check parsing succeeds
-		testParse(c, parseFailTest{args: test.args}, false)
+		testParse(c, parseFailTest{resettable: true, args: test.args}, false)
 
 		cmd, err := setupInitTest(c, test.args, test.cantReset)
 		c.Assert(err, jc.ErrorIsNil)
@@ -149,9 +159,16 @@ var parseTests = []parseFailTest{
 		errMsg: " needs an argument: --file\n",
 	},
 	{
-		about:  "no argument provided to --reset",
-		args:   []string{"--reset"},
-		errMsg: " needs an argument: --reset\n",
+		about:      "--reset when unresettable",
+		resettable: false,
+		args:       []string{"--reset", "key1"},
+		errMsg:     " provided but not defined: --reset\n",
+	},
+	{
+		about:      "no argument provided to --reset",
+		resettable: true,
+		args:       []string{"--reset"},
+		errMsg:     " needs an argument: --reset\n",
 	},
 	{
 		about:  "undefined flag --foo",