Merge pull request juju#17978 from nvinuesa/juju-6605

juju#17978

Before this patch it was impossible to un-set the juju-ha-space controller config value because:
- first, the empty string was rejected because it does not match the regexp on the names package for space names.
- second, because on the state layer we check that the provided space exists, and an empty string space never does.

The first reason was fixed on `controller/config.go` by simply adding the "not empty string" check before validating the space name.

For the second reason, a workaround had to be added: if the passed space name is the empty string, then we don't check for its existence. This is not good because in the `UpdateControllerConfig()` method we already have the `removeAttrs` argument but on the only usage of this method (on the client facades) we pass a nil value as `removeAttrs`.


## Checklist


- [X] Code style: imports ordered, good names, simple structure, etc
- [X] Comments saying why design decisions were made
- [X] Go unit tests, with comments saying what you're testing
- [ ] ~[Integration tests](https://github.com/juju/juju/tree/main/tests), with comments saying what you're testing~
- [ ] ~[doc.go](https://discourse.charmhub.io/t/readme-in-packages/451) added or updated in changed packages~

## QA steps

Bootstrap, set the `juju-ha-space` config value and then un-set it:

``` 
juju bootstrap lxd c
juju controller-config juju-ha-space=alpha
juju controller-config juju-ha-space
alpha
juju controller-config juju-ha-space=""
juju controller-config juju-ha-space
```

No errors should be returned.

## Links

**Launchpad bug:** https://bugs.launchpad.net/juju/+bug/2069808

**Jira card:** JUJU-6605

controller/config.go

@@ -1250,7 +1250,11 @@ func (c Config) validateSpaceConfig(key, topic string) error {
 		return nil
 	}
 	if v, ok := val.(string); ok {
-		if !names.IsValidSpace(v) {
+		// NOTE(nvinuesa): We also check for the case where the passed
+		// value is the empty string, this is needed to un-set the
+		// controller config value and not added in the regexp to
+		// validate the space.
+		if !names.IsValidSpace(v) && v != "" {
 			return errors.NotValidf("%s space name %q", topic, val)
 		}
 	} else {

controller/config_test.go

@@ -129,6 +129,11 @@ var newConfigTests = []struct {
 		controller.JujuManagementSpace: "\n",
 	},
 	expectError: `juju mgmt space name "\\n" not valid`,
+}, {
+	about: "HA space name - empty string OK",
+	config: controller.Config{
+		controller.JujuHASpace: "",
+	},
 }, {
 	about: "invalid HA space name - number",
 	config: controller.Config{

state/controller.go

@@ -172,6 +172,15 @@ func checkUpdateControllerConfig(name string) error {
 // checkSpaceIsAvailableToAllControllers checks if each controller machine has
 // at least one address in the input space. If not, an error is returned.
 func (st *State) checkSpaceIsAvailableToAllControllers(spaceName string) error {
+	// TODO(nvinuesa): We should not be checking if the space is empty
+	// at this point, instead we should be using the `removeAttrs`
+	// attribute on `UpdateControllerConfig()` to un-set the spaces.
+	// Unfortunately this is not the case and therefore we must check for
+	// the empty string (un-setting the space value) as a workaround.
+	if spaceName == "" {
+		return nil
+	}
+
 	controllerIds, err := st.ControllerIds()
 	if err != nil {
 		return errors.Annotate(err, "cannot get controller info")

state/controller_test.go

@@ -185,6 +185,29 @@ func (s *ControllerSuite) TestRemovingUnknownName(c *gc.C) {
 	c.Assert(err, gc.ErrorMatches, `unknown controller config setting "dr-worm"`)
 }
 
+func (s *ControllerSuite) TestUpdateControllerConfigAcceptEmptyStringSpace(c *gc.C) {
+	sp, err := s.State.AddSpace("ha-space", "", nil, false)
+	c.Assert(err, jc.ErrorIsNil)
+
+	m, err := s.State.AddMachine(state.UbuntuBase("12.10"), state.JobManageModel, state.JobHostUnits)
+	c.Assert(err, jc.ErrorIsNil)
+
+	addr := network.NewSpaceAddress("192.168.9.9")
+	addr.SpaceID = sp.Id()
+
+	c.Assert(m.SetProviderAddresses(addr), jc.ErrorIsNil)
+
+	err = s.State.UpdateControllerConfig(map[string]interface{}{
+		controller.JujuHASpace: "ha-space",
+	}, nil)
+	c.Assert(err, jc.ErrorIsNil)
+
+	err = s.State.UpdateControllerConfig(map[string]interface{}{
+		controller.JujuHASpace: "",
+	}, nil)
+	c.Assert(err, jc.ErrorIsNil)
+}
+
 func (s *ControllerSuite) TestUpdateControllerConfigRejectsSpaceWithoutAddresses(c *gc.C) {
 	_, err := s.State.AddSpace("mgmt-space", "", nil, false)
 	c.Assert(err, jc.ErrorIsNil)