Merge pull request juju#17494 from nvinuesa/merge-3.6-main-20240610

juju#17494

Merge 3.6 into main. 

Conflits:
- agent/agentbootstrap/bootstrap.go
- agent/agentbootstrap/bootstrap_test.go
- apiserver/facades/agent/provisioner/provisioninginfo.go
- apiserver/facades/agent/provisioner/provisioninginfo_test.go
- go.mod
- go.sum

The only conflict that took some more insights is `apiserver/facades/agent/provisioner/provisioninginfo_test.go
` which required to migrate tests to use the network service.

agent/agentbootstrap/bootstrap_test.go

@@ -205,16 +205,16 @@ func (s *bootstrapSuite) TestInitializeState(c *gc.C) {
 	// Check that the model has been set up.
 	model, err := st.Model()
 	c.Assert(err, jc.ErrorIsNil)
-	c.Assert(model.UUID(), gc.Equals, modelCfg.UUID())
-	c.Assert(model.EnvironVersion(), gc.Equals, 666)
+	c.Check(model.UUID(), gc.Equals, modelCfg.UUID())
+	c.Check(model.EnvironVersion(), gc.Equals, 666)
 
 	// Check that initial admin user has been set up correctly.
 	modelTag := model.Tag().(names.ModelTag)
 	controllerTag := names.NewControllerTag(controllerCfg.ControllerUUID())
 	s.assertCanLogInAsAdmin(c, modelTag, controllerTag, testing.DefaultMongoPassword)
 	user, err := st.User(model.Owner())
 	c.Assert(err, jc.ErrorIsNil)
-	c.Assert(user.PasswordValid(testing.DefaultMongoPassword), jc.IsTrue)
+	c.Check(user.PasswordValid(testing.DefaultMongoPassword), jc.IsTrue)
 
 	// Check that controller model configuration has been added, and
 	// model constraints set.
@@ -230,11 +230,11 @@ func (s *bootstrapSuite) TestInitializeState(c *gc.C) {
 	expectedAttrs := expectedCfg.AllAttrs()
 	expectedAttrs["apt-mirror"] = "http://mirror"
 	expectedAttrs["no-proxy"] = "value"
-	c.Assert(newModelCfg.AllAttrs(), jc.DeepEquals, expectedAttrs)
+	c.Check(newModelCfg.AllAttrs(), jc.DeepEquals, expectedAttrs)
 
 	gotModelConstraints, err := st.ModelConstraints()
 	c.Assert(err, jc.ErrorIsNil)
-	c.Assert(gotModelConstraints, gc.DeepEquals, expectModelConstraints)
+	c.Check(gotModelConstraints, gc.DeepEquals, expectModelConstraints)
 
 	// Check that the hosted model has been added, model constraints
 	// set, and its config contains the same authorized-keys as the
@@ -244,39 +244,43 @@ func (s *bootstrapSuite) TestInitializeState(c *gc.C) {
 	defer initialModelSt.Release()
 	gotModelConstraints, err = initialModelSt.ModelConstraints()
 	c.Assert(err, jc.ErrorIsNil)
-	c.Assert(gotModelConstraints, gc.DeepEquals, expectModelConstraints)
+	c.Check(gotModelConstraints, gc.DeepEquals, expectModelConstraints)
+
 	initialModel, err := initialModelSt.Model()
 	c.Assert(err, jc.ErrorIsNil)
-	c.Assert(initialModel.Name(), gc.Equals, "hosted")
-	c.Assert(initialModel.CloudRegion(), gc.Equals, "dummy-region")
-	c.Assert(initialModel.EnvironVersion(), gc.Equals, 123)
+	c.Check(initialModel.Name(), gc.Equals, "hosted")
+	c.Check(initialModel.CloudRegion(), gc.Equals, "dummy-region")
+	c.Check(initialModel.EnvironVersion(), gc.Equals, 123)
+
 	hostedCfg, err := initialModel.ModelConfig(context.Background())
 	c.Assert(err, jc.ErrorIsNil)
 	_, hasUnexpected := hostedCfg.AllAttrs()["not-for-hosted"]
-	c.Assert(hasUnexpected, jc.IsFalse)
-	c.Assert(hostedCfg.AuthorizedKeys(), gc.Equals, newModelCfg.AuthorizedKeys())
+	c.Check(hasUnexpected, jc.IsFalse)
+	c.Check(hostedCfg.AuthorizedKeys(), gc.Equals, newModelCfg.AuthorizedKeys())
 
 	// Check that the bootstrap machine looks correct.
 	m, err := st.Machine("0")
 	c.Assert(err, jc.ErrorIsNil)
-	c.Assert(m.Id(), gc.Equals, "0")
-	c.Assert(m.Jobs(), gc.DeepEquals, []state.MachineJob{state.JobManageModel})
+	c.Check(m.Id(), gc.Equals, "0")
+	c.Check(m.Jobs(), gc.DeepEquals, []state.MachineJob{state.JobManageModel})
+
 	base, err := corebase.ParseBase(m.Base().OS, m.Base().Channel)
 	c.Assert(err, jc.ErrorIsNil)
-	c.Assert(m.Base().String(), gc.Equals, base.String())
-	c.Assert(m.CheckProvisioned(agent.BootstrapNonce), jc.IsTrue)
+	c.Check(m.Base().String(), gc.Equals, base.String())
+	c.Check(m.CheckProvisioned(agent.BootstrapNonce), jc.IsTrue)
+
 	gotBootstrapConstraints, err := m.Constraints()
 	c.Assert(err, jc.ErrorIsNil)
-	c.Assert(gotBootstrapConstraints, gc.DeepEquals, expectBootstrapConstraints)
-	c.Assert(err, jc.ErrorIsNil)
+	c.Check(gotBootstrapConstraints, gc.DeepEquals, expectBootstrapConstraints)
+
 	gotHW, err := m.HardwareCharacteristics()
 	c.Assert(err, jc.ErrorIsNil)
-	c.Assert(*gotHW, gc.DeepEquals, expectHW)
+	c.Check(*gotHW, gc.DeepEquals, expectHW)
 
 	// Check that the state serving info is initialised correctly.
 	stateServingInfo, err := st.StateServingInfo()
 	c.Assert(err, jc.ErrorIsNil)
-	c.Assert(stateServingInfo, jc.DeepEquals, controller.StateServingInfo{
+	c.Check(stateServingInfo, jc.DeepEquals, controller.StateServingInfo{
 		APIPort:        1234,
 		StatePort:      s.mgoInst.Port(),
 		Cert:           testing.ServerCert,
@@ -291,10 +295,11 @@ func (s *bootstrapSuite) TestInitializeState(c *gc.C) {
 	machine0 := names.NewMachineTag("0")
 	newCfg, err := agent.ReadConfig(agent.ConfigPath(dataDir, machine0))
 	c.Assert(err, jc.ErrorIsNil)
-	c.Assert(newCfg.Tag(), gc.Equals, machine0)
+	c.Check(newCfg.Tag(), gc.Equals, machine0)
+
 	info, ok := cfg.MongoInfo()
 	c.Assert(ok, jc.IsTrue)
-	c.Assert(info.Password, gc.Not(gc.Equals), testing.DefaultMongoPassword)
+	c.Check(info.Password, gc.Not(gc.Equals), testing.DefaultMongoPassword)
 
 	session, err := mongo.DialWithInfo(*info, mongotest.DialOpts())
 	c.Assert(err, jc.ErrorIsNil)

apiserver/facades/agent/provisioner/provisioninginfo.go

@@ -16,6 +16,7 @@ import (
 	"github.com/juju/juju/apiserver/common/storagecommon"
 	apiservererrors "github.com/juju/juju/apiserver/errors"
 	corebase "github.com/juju/juju/core/base"
+	"github.com/juju/juju/core/constraints"
 	"github.com/juju/juju/core/lxdprofile"
 	"github.com/juju/juju/core/model"
 	"github.com/juju/juju/core/network"
@@ -91,12 +92,16 @@ func (api *ProvisionerAPI) getProvisioningInfo(
 		return nil, errors.Trace(err)
 	}
 
-	machineSpaces, err := api.machineSpaces(m, allSpaces, endpointBindings)
+	cons, err := m.Constraints()
+	if err != nil {
+		return nil, errors.Annotate(err, "retrieving machine constraints")
+	}
+	machineSpaces, err := api.machineSpaces(cons, allSpaces, endpointBindings)
 	if err != nil {
 		return nil, errors.Trace(err)
 	}
 
-	if result.ProvisioningNetworkTopology, err = api.machineSpaceTopology(ctx, m.Id(), machineSpaces); err != nil {
+	if result.ProvisioningNetworkTopology, err = api.machineSpaceTopology(ctx, m.Id(), cons, machineSpaces); err != nil {
 		return nil, errors.Annotate(err, "matching subnets to zones")
 	}
 
@@ -310,14 +315,11 @@ func (api *ProvisionerAPI) machineTags(ctx context.Context, m *state.Machine, is
 //
 // It is the responsibility of the provider to negotiate this information
 // appropriately.
-func (api *ProvisionerAPI) machineSpaces(m *state.Machine,
+func (api *ProvisionerAPI) machineSpaces(
+	cons constraints.Value,
 	allSpaceInfos network.SpaceInfos,
 	endpointBindings map[string]*state.Bindings,
 ) ([]string, error) {
-	cons, err := m.Constraints()
-	if err != nil {
-		return nil, errors.Annotate(err, "retrieving machine constraints")
-	}
 
 	includeSpaces := set.NewStrings(cons.IncludeSpaces()...)
 	excludeSpaces := set.NewStrings(cons.ExcludeSpaces()...)
@@ -340,13 +342,21 @@ func (api *ProvisionerAPI) machineSpaces(m *state.Machine,
 	return includeSpaces.SortedValues(), nil
 }
 
-func (api *ProvisionerAPI) machineSpaceTopology(ctx context.Context, machineID string, spaceNames []string) (params.ProvisioningNetworkTopology, error) {
+func (api *ProvisionerAPI) machineSpaceTopology(
+	ctx context.Context,
+	machineID string,
+	cons constraints.Value,
+	spaceNames []string,
+) (params.ProvisioningNetworkTopology, error) {
 	var topology params.ProvisioningNetworkTopology
 
 	// If there are no space names, or if there is only one space
-	// name and that's the alpha space, we don't bother setting a
-	// topology that constrains provisioning.
-	if len(spaceNames) < 1 || (len(spaceNames) == 1 && spaceNames[0] == network.AlphaSpaceName) {
+	// name and that's the alpha space unless it was explicitly set as a
+	// constraint, we don't bother setting a topology that constrains
+	// provisioning.
+	consHasOnlyAlpha := len(cons.IncludeSpaces()) == 1 && cons.IncludeSpaces()[0] == network.AlphaSpaceName
+	if len(spaceNames) < 1 ||
+		((len(spaceNames) == 1 && spaceNames[0] == network.AlphaSpaceName) && !consHasOnlyAlpha) {
 		return topology, nil
 	}
 

apiserver/facades/agent/provisioner/provisioninginfo_test.go

@@ -416,6 +416,92 @@ func (s *withoutControllerSuite) TestConflictingNegativeConstraintWithBindingErr
 	c.Assert(result, jc.DeepEquals, expected)
 }
 
+func (s *withoutControllerSuite) TestNoSpaceConstraintsProvidedSpaceTopologyEmpty(c *gc.C) {
+	st := s.ControllerModel(c).State()
+	wordpressMachine, err := st.AddOneMachine(s.InstancePrechecker(c, st), state.MachineTemplate{
+		Base: state.UbuntuBase("12.10"),
+		Jobs: []state.MachineJob{state.JobHostUnits},
+	})
+	c.Assert(err, jc.ErrorIsNil)
+
+	f, release := s.NewFactory(c, s.ControllerModelUUID())
+	defer release()
+
+	// Simulates running `juju deploy --bind "..."`.
+	bindings := map[string]string{
+		"url": network.AlphaSpaceId,
+		"db":  network.AlphaSpaceId,
+	}
+	wordpressService := f.MakeApplication(c, &factory.ApplicationParams{
+		Charm:            f.MakeCharm(c, &factory.CharmParams{Name: "wordpress"}),
+		EndpointBindings: bindings,
+	})
+	wordpressUnit, err := wordpressService.AddUnit(state.AddUnitParams{})
+	c.Assert(err, jc.ErrorIsNil)
+	err = wordpressUnit.AssignToMachine(wordpressMachine)
+	c.Assert(err, jc.ErrorIsNil)
+
+	args := params.Entities{Entities: []params.Entity{
+		{Tag: wordpressMachine.Tag().String()},
+	}}
+	result, err := s.provisioner.ProvisioningInfo(context.Background(), args)
+	c.Assert(err, jc.ErrorIsNil)
+
+	c.Assert(result.Results, gc.HasLen, 1)
+	c.Assert(result.Results[0].Error, gc.IsNil)
+	c.Assert(result.Results[0].Result.ProvisioningNetworkTopology.SubnetAZs, gc.IsNil)
+	c.Assert(result.Results[0].Result.ProvisioningNetworkTopology.SpaceSubnets, gc.IsNil)
+}
+
+func (s *withoutControllerSuite) TestAlphaSpaceConstraintsProvidedExplicitly(c *gc.C) {
+	s.addSpacesAndSubnets(c)
+	st := s.ControllerModel(c).State()
+
+	networkService := s.serviceFactory.Network()
+	networkService.AddSubnet(context.Background(), network.SubnetInfo{
+		CIDR:              "10.10.4.0/24",
+		ProviderId:        "subnet-alpha",
+		AvailabilityZones: []string{"zone-alpha"},
+		SpaceID:           network.AlphaSpaceId,
+		VLANTag:           43,
+	})
+
+	cons := constraints.MustParse("spaces=alpha")
+	wordpressMachine, err := st.AddOneMachine(s.InstancePrechecker(c, st), state.MachineTemplate{
+		Base:        state.UbuntuBase("12.10"),
+		Jobs:        []state.MachineJob{state.JobHostUnits},
+		Constraints: cons,
+	})
+	c.Assert(err, jc.ErrorIsNil)
+
+	f, release := s.NewFactory(c, s.ControllerModelUUID())
+	defer release()
+
+	// Simulates running `juju deploy --bind "..."`.
+	bindings := map[string]string{
+		"url": network.AlphaSpaceId,
+		"db":  network.AlphaSpaceId,
+	}
+	wordpressService := f.MakeApplication(c, &factory.ApplicationParams{
+		Charm:            f.MakeCharm(c, &factory.CharmParams{Name: "wordpress"}),
+		EndpointBindings: bindings,
+	})
+	wordpressUnit, err := wordpressService.AddUnit(state.AddUnitParams{})
+	c.Assert(err, jc.ErrorIsNil)
+	err = wordpressUnit.AssignToMachine(wordpressMachine)
+	c.Assert(err, jc.ErrorIsNil)
+
+	args := params.Entities{Entities: []params.Entity{
+		{Tag: wordpressMachine.Tag().String()},
+	}}
+	result, err := s.provisioner.ProvisioningInfo(context.Background(), args)
+	c.Assert(err, jc.ErrorIsNil)
+
+	c.Assert(result.Results, gc.HasLen, 1)
+	c.Assert(result.Results[0].Error, gc.IsNil)
+	c.Assert(result.Results[0].Result.ProvisioningNetworkTopology.SubnetAZs, gc.DeepEquals, map[string][]string{"subnet-alpha": {"zone-alpha"}})
+	c.Assert(result.Results[0].Result.ProvisioningNetworkTopology.SpaceSubnets, gc.DeepEquals, map[string][]string{"alpha": {"subnet-alpha"}})
+}
 func (s *withoutControllerSuite) addSpacesAndSubnets(c *gc.C) network.SpaceInfos {
 	networkService := s.serviceFactory.Network()
 	// Add a couple of spaces.

cmd/juju/controller/register.go

@@ -71,7 +71,7 @@ type registerCommand struct {
 	arg     string
 	replace bool
 
-	// onRunError is executed if non-nil if there is an error at the end
+	// onRunError is executed if non-nil and there is an error at the end
 	// of the Run method.
 	onRunError func()
 }
@@ -171,7 +171,7 @@ func (c *registerCommand) run(ctx *cmd.Context) error {
 	}
 
 	// Check if user is trying to register an already known controller by
-	// by providing the IP of one of its endpoints.
+	// providing the IP of one of its endpoints.
 	if registrationParams.publicHost != "" {
 		if err := ensureNotKnownEndpoint(c.store, registrationParams.publicHost); err != nil {
 			return errors.Trace(err)
@@ -374,12 +374,6 @@ func (c *registerCommand) nonPublicControllerDetails(ctx *cmd.Context, registrat
 	}
 	resp, err := c.secretKeyLogin(controllerDetails, req, controllerName)
 	if err != nil {
-		// If we got here and got an error, the registration token supplied
-		// will be expired.
-		// Log the error as it will be useful for debugging, but give user a
-		// suggestion for the way forward instead of error details.
-		logger.Infof("while validating secret key: %v", err)
-		err = errors.Errorf("Provided registration token may have been expired.\nA controller administrator must reset your user to issue a new token.\nSee %q for more information.", "juju help change-user-password")
 		return errRet(errors.Trace(err))
 	}
 
@@ -585,12 +579,12 @@ func (c *registerCommand) secretKeyLogin(
 ) (_ *params.SecretKeyLoginResponse, err error) {
 	cookieJar, err := c.CookieJar(c.store, controllerName)
 	if err != nil {
-		return nil, errors.Annotate(err, "getting API context")
+		return nil, errors.Annotatef(err, "internal error: cannot get API context")
 	}
 
 	buf, err := json.Marshal(&request)
 	if err != nil {
-		return nil, errors.Annotate(err, "marshalling request")
+		return nil, errors.Annotatef(err, "internal error: cannot marshell controller api request")
 	}
 	r := bytes.NewReader(buf)
 
@@ -610,7 +604,8 @@ func (c *registerCommand) secretKeyLogin(
 	}
 	conn, err := c.apiOpen(apiInfo, opts)
 	if err != nil {
-		return nil, errors.Trace(err)
+		logger.Infof("opening api connection: %s", err)
+		return nil, controllerUnreachableError(controllerName, controllerDetails.APIEndpoints)
 	}
 	apiAddr := conn.Addr()
 	defer func() {
@@ -629,7 +624,7 @@ func (c *registerCommand) secretKeyLogin(
 	urlString := fmt.Sprintf("https://%s/register", apiAddr)
 	httpReq, err := http.NewRequest("POST", urlString, r)
 	if err != nil {
-		return nil, errors.Trace(err)
+		return nil, errors.Annotatef(err, "internal error: creating new http request")
 	}
 	httpReq.Header.Set("Content-Type", "application/json")
 	httpReq.Header.Set(httpbakery.BakeryProtocolHeader, fmt.Sprint(bakery.LatestVersion))
@@ -640,21 +635,24 @@ func (c *registerCommand) secretKeyLogin(
 	)
 	httpResp, err := httpClient.Do(httpReq)
 	if err != nil {
-		return nil, errors.Trace(err)
+		logger.Infof("connecting to controller: %s", err)
+		return nil, controllerUnreachableError(controllerName, controllerDetails.APIEndpoints)
 	}
 	defer func() { _ = httpResp.Body.Close() }()
 
 	if httpResp.StatusCode != http.StatusOK {
 		var resp params.ErrorResult
 		if err := json.NewDecoder(httpResp.Body).Decode(&resp); err != nil {
-			return nil, errors.Trace(err)
+			return nil, errors.Annotatef(err, "internal error: cannot decode http response")
 		}
-		return nil, resp.Error
+		logger.Infof("error response, %s", resp.Error)
+		return nil, errors.Errorf("Provided registration token may have expired."+
+			"\nA controller administrator must reset your user to issue a new token.\nSee %q for more information.", "juju help change-user-password")
 	}
 
 	var resp params.SecretKeyLoginResponse
 	if err := json.NewDecoder(httpResp.Body).Decode(&resp); err != nil {
-		return nil, errors.Annotatef(err, "cannot decode login response")
+		return nil, errors.Annotatef(err, "internal error: cannot decode controller response")
 	}
 	return &resp, nil
 }
@@ -795,3 +793,9 @@ func genAlreadyRegisteredError(controller, user string) error {
 	}
 	return errors.New(buf.String())
 }
+
+func controllerUnreachableError(name string, endpoints []string) error {
+	return fmt.Errorf("Cannot reach controller %q at: %s."+
+		"\nCheck that the controller ip is reachable from your network.",
+		name, strings.Join(endpoints, ", "))
+}

cmd/juju/controller/register_test.go

@@ -564,10 +564,8 @@ Enter a name for this controller: »foo
 	s.apiOpenError = errors.New("open failed")
 	err := s.run(c, prompter, registrationData)
 	c.Assert(c.GetTestLog(), gc.Matches, "(.|\n)*open failed(.|\n)*")
-	c.Assert(err, gc.ErrorMatches, `
-Provided registration token may have been expired.
-A controller administrator must reset your user to issue a new token.
-See "juju help change-user-password" for more information.`[1:])
+	c.Assert(err, gc.ErrorMatches, `Cannot reach controller "foo" at: `+s.apiConnection.Addr()+".\n"+
+		"Check that the controller ip is reachable from your network.")
 }
 
 func (s *RegisterSuite) TestRegisterServerError(c *gc.C) {
@@ -596,7 +594,7 @@ Enter a name for this controller: »foo
 	err = s.run(c, prompter, registrationData)
 	c.Assert(c.GetTestLog(), gc.Matches, "(.|\n)* xyz(.|\n)*")
 	c.Assert(err, gc.ErrorMatches, `
-Provided registration token may have been expired.
+Provided registration token may have expired.
 A controller administrator must reset your user to issue a new token.
 See "juju help change-user-password" for more information.`[1:])