From a127e23a2a76c8c91f5303053c6d6790ec4ebdc9 Mon Sep 17 00:00:00 2001 From: Alastair Flynn Date: Thu, 30 May 2024 14:35:02 +0100 Subject: [PATCH 1/7] Improve error messages in the register command --- cmd/juju/controller/register.go | 42 +++++++++++++++++----------- cmd/juju/controller/register_test.go | 7 ++--- 2 files changed, 28 insertions(+), 21 deletions(-) diff --git a/cmd/juju/controller/register.go b/cmd/juju/controller/register.go index d77846a3dbd..570dc95c60b 100644 --- a/cmd/juju/controller/register.go +++ b/cmd/juju/controller/register.go @@ -69,7 +69,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() } @@ -169,7 +169,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) @@ -334,12 +334,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)) } @@ -545,12 +539,14 @@ 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") + logger.Errorf("getting API context: %s", err) + return nil, fmt.Errorf("failed to prepare request, invalid token or internal error") } buf, err := json.Marshal(&request) if err != nil { - return nil, errors.Annotate(err, "marshalling request") + logger.Errorf("marshalling request: %s", err) + return nil, fmt.Errorf("failed to prepare request, invalid token or internal error") } r := bytes.NewReader(buf) @@ -570,7 +566,8 @@ func (c *registerCommand) secretKeyLogin( } conn, err := c.apiOpen(apiInfo, opts) if err != nil { - return nil, errors.Trace(err) + logger.Errorf("opening api connection: %s", err) + return nil, controllerUnreachableError(controllerName, controllerDetails.APIEndpoints) } apiAddr := conn.Addr() defer func() { @@ -589,7 +586,8 @@ 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) + logger.Errorf("creating new http request: %s", err) + return nil, fmt.Errorf("internal error preparing request for controller") } httpReq.Header.Set("Content-Type", "application/json") httpReq.Header.Set(httpbakery.BakeryProtocolHeader, fmt.Sprint(bakery.LatestVersion)) @@ -600,21 +598,28 @@ func (c *registerCommand) secretKeyLogin( ) httpResp, err := httpClient.Do(httpReq) if err != nil { - return nil, errors.Trace(err) + logger.Errorf("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) + logger.Errorf("cannot decode http response: %s", err) + return nil, fmt.Errorf("internal error") + } else if resp.Error != nil { + } - return nil, resp.Error + logger.Errorf("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") + logger.Errorf("cannot decode login response: %s", err) + return nil, fmt.Errorf("internal error") } return &resp, nil } @@ -755,3 +760,8 @@ 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", + name, strings.Join(endpoints, ", ")) +} diff --git a/cmd/juju/controller/register_test.go b/cmd/juju/controller/register_test.go index 3c543643c4a..5fbdcf1e643 100644 --- a/cmd/juju/controller/register_test.go +++ b/cmd/juju/controller/register_test.go @@ -564,10 +564,7 @@ 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()) } func (s *RegisterSuite) TestRegisterServerError(c *gc.C) { @@ -596,7 +593,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:]) From a58b6f3b503c1e1fa1be6a065ba0bd19364524b2 Mon Sep 17 00:00:00 2001 From: Alastair Flynn Date: Thu, 30 May 2024 17:34:34 +0100 Subject: [PATCH 2/7] Add information to user errors, and add advice to controller unreachable --- cmd/juju/controller/register.go | 26 ++++++++++---------------- cmd/juju/controller/register_test.go | 3 ++- 2 files changed, 12 insertions(+), 17 deletions(-) diff --git a/cmd/juju/controller/register.go b/cmd/juju/controller/register.go index 570dc95c60b..936b6d38b6b 100644 --- a/cmd/juju/controller/register.go +++ b/cmd/juju/controller/register.go @@ -539,14 +539,12 @@ func (c *registerCommand) secretKeyLogin( ) (_ *params.SecretKeyLoginResponse, err error) { cookieJar, err := c.CookieJar(c.store, controllerName) if err != nil { - logger.Errorf("getting API context: %s", err) - return nil, fmt.Errorf("failed to prepare request, invalid token or internal error") + return nil, errors.Annotatef(err, "internal error: cannot get API context") } buf, err := json.Marshal(&request) if err != nil { - logger.Errorf("marshalling request: %s", err) - return nil, fmt.Errorf("failed to prepare request, invalid token or internal error") + return nil, errors.Annotatef(err, "internal error: cannot marshell controller api request") } r := bytes.NewReader(buf) @@ -566,7 +564,7 @@ func (c *registerCommand) secretKeyLogin( } conn, err := c.apiOpen(apiInfo, opts) if err != nil { - logger.Errorf("opening api connection: %s", err) + logger.Infof("opening api connection: %s", err) return nil, controllerUnreachableError(controllerName, controllerDetails.APIEndpoints) } apiAddr := conn.Addr() @@ -586,8 +584,7 @@ func (c *registerCommand) secretKeyLogin( urlString := fmt.Sprintf("https://%s/register", apiAddr) httpReq, err := http.NewRequest("POST", urlString, r) if err != nil { - logger.Errorf("creating new http request: %s", err) - return nil, fmt.Errorf("internal error preparing request for controller") + 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)) @@ -598,7 +595,7 @@ func (c *registerCommand) secretKeyLogin( ) httpResp, err := httpClient.Do(httpReq) if err != nil { - logger.Errorf("connecting to controller: %s", err) + logger.Infof("connecting to controller: %s", err) return nil, controllerUnreachableError(controllerName, controllerDetails.APIEndpoints) } defer func() { _ = httpResp.Body.Close() }() @@ -606,20 +603,16 @@ func (c *registerCommand) secretKeyLogin( if httpResp.StatusCode != http.StatusOK { var resp params.ErrorResult if err := json.NewDecoder(httpResp.Body).Decode(&resp); err != nil { - logger.Errorf("cannot decode http response: %s", err) - return nil, fmt.Errorf("internal error") - } else if resp.Error != nil { - + return nil, errors.Annotatef(err, "internal error: cannot decode http response") } - logger.Errorf("error response, %s", 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 { - logger.Errorf("cannot decode login response: %s", err) - return nil, fmt.Errorf("internal error") + return nil, errors.Annotatef(err, "internal error: cannot decode controller response") } return &resp, nil } @@ -762,6 +755,7 @@ func genAlreadyRegisteredError(controller, user string) error { } func controllerUnreachableError(name string, endpoints []string) error { - return fmt.Errorf("cannot reach controller %q at: %s", + return fmt.Errorf("Cannot reach controller %q at: %s."+ + "\nCheck that the controller ip is reachable from your network.", name, strings.Join(endpoints, ", ")) } diff --git a/cmd/juju/controller/register_test.go b/cmd/juju/controller/register_test.go index 5fbdcf1e643..14bfd422390 100644 --- a/cmd/juju/controller/register_test.go +++ b/cmd/juju/controller/register_test.go @@ -564,7 +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, `cannot reach controller "foo" at: `+s.apiConnection.Addr()) + 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) { From 377dbb80104fb5fbfa9d58f469d0b4b5b5847492 Mon Sep 17 00:00:00 2001 From: Joseph Phillips Date: Fri, 31 May 2024 12:51:37 +0200 Subject: [PATCH 3/7] Fix: avoid setting machine provider addresses at bootstrap. It is not necessary to set machine addresses at bootstrap. We already use the bootstrap addresses to configure Mongo, API host-ports and agent config. We can let address discovery work in the normal workflow. --- agent/agentbootstrap/bootstrap.go | 6 --- agent/agentbootstrap/bootstrap_test.go | 54 ++++++++++++++------------ 2 files changed, 29 insertions(+), 31 deletions(-) diff --git a/agent/agentbootstrap/bootstrap.go b/agent/agentbootstrap/bootstrap.go index c26b5cb3a6f..7e9e6ae608c 100644 --- a/agent/agentbootstrap/bootstrap.go +++ b/agent/agentbootstrap/bootstrap.go @@ -451,17 +451,11 @@ func initBootstrapMachine(st *state.State, args InitializeStateParams) (bootstra return nil, errors.Trace(err) } - spaceAddrs, err := args.BootstrapMachineAddresses.ToSpaceAddresses(st) - if err != nil { - return nil, errors.Trace(err) - } - base, err := corebase.GetBaseFromSeries(hostSeries) if err != nil { return nil, errors.Trace(err) } m, err := st.AddOneMachine(state.MachineTemplate{ - Addresses: spaceAddrs, Base: state.Base{OS: base.OS, Channel: base.Channel.String()}, Nonce: agent.BootstrapNonce, Constraints: args.BootstrapMachineConstraints, diff --git a/agent/agentbootstrap/bootstrap_test.go b/agent/agentbootstrap/bootstrap_test.go index efc20b6d2e4..c6b38769153 100644 --- a/agent/agentbootstrap/bootstrap_test.go +++ b/agent/agentbootstrap/bootstrap_test.go @@ -198,8 +198,8 @@ 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) @@ -207,12 +207,12 @@ func (s *bootstrapSuite) TestInitializeState(c *gc.C) { 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 controller config controllerCfg, err = st.ControllerConfig() c.Assert(err, jc.ErrorIsNil) - c.Assert(controllerCfg, jc.DeepEquals, controller.Config{ + c.Check(controllerCfg, jc.DeepEquals, controller.Config{ "controller-uuid": testing.ControllerTag.Id(), "ca-cert": testing.CACert, "state-port": 1234, @@ -243,11 +243,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 @@ -257,47 +257,50 @@ 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() 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.Assert(m.Addresses(), jc.DeepEquals, filteredAddrs) + 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 API host ports are initialised correctly. apiHostPorts, err := st.APIHostPortsForClients() c.Assert(err, jc.ErrorIsNil) - c.Assert(apiHostPorts, jc.DeepEquals, []corenetwork.SpaceHostPorts{ + c.Check(apiHostPorts, jc.DeepEquals, []corenetwork.SpaceHostPorts{ corenetwork.SpaceAddressesWithPort(filteredAddrs, 1234), }) // 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, @@ -313,17 +316,18 @@ func (s *bootstrapSuite) TestInitializeState(c *gc.C) { c.Assert(err, jc.ErrorIsNil) expectedStorageCfg, err := storage.NewConfig("spool", "loop", map[string]interface{}{"foo": "bar"}) c.Assert(err, jc.ErrorIsNil) - c.Assert(storageCfg, jc.DeepEquals, expectedStorageCfg) + c.Check(storageCfg, jc.DeepEquals, expectedStorageCfg) // Check that the machine agent's config has been written // and that we can use it to connect to mongo. 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) From 780135fc6141e5faf9370d5055f94c32279ad724 Mon Sep 17 00:00:00 2001 From: Nicolas Vinuesa Date: Wed, 5 Jun 2024 23:17:42 +0200 Subject: [PATCH 4/7] Return a non-empty network topology when only alpha space has been explicitly set as constraint. At the provisioning info facade, we fill the ProvisioningNetworkTopology field of the provisiong info struct with the subnets mapped to the AZs. We have this information because we have the constraints (and enpdoint bindings) and we then do a lookup to find the subnets that are in that space(s). We merge the constraints *and* the bound endpoint spaces (which by the way is `alpha` by default) and then return the list of spaces merged. The issue is that later we make the following check: if only the alpha space is left after this merge, then we don't bother to fill a network topology that would have to be interpreted by the providers. This check is wrong because it assumes that if we only have alpha in that resulting list it's because it was added by the default endpoint binding space. Another possibility is that the user had explicitly passed only the alpha space as a positive constraint. This patch fixes this assumption, by not only checking if the merged list contains only alpha but also if the constraints weren't explicitly set to alpha only. If that's the case then we set a network topology and providers will be able to correctly instantiate the machine on the correct spaces (and in the case of openstack by creating only the correct ports). --- .../agent/provisioner/provisioninginfo.go | 31 +++++--- .../provisioner/provisioninginfo_test.go | 77 +++++++++++++++++++ 2 files changed, 97 insertions(+), 11 deletions(-) diff --git a/apiserver/facades/agent/provisioner/provisioninginfo.go b/apiserver/facades/agent/provisioner/provisioninginfo.go index 0012e2a9027..af4781d1d55 100644 --- a/apiserver/facades/agent/provisioner/provisioninginfo.go +++ b/apiserver/facades/agent/provisioner/provisioninginfo.go @@ -16,6 +16,7 @@ import ( apiservererrors "github.com/juju/juju/apiserver/errors" "github.com/juju/juju/cloudconfig/instancecfg" 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" @@ -86,12 +87,16 @@ func (api *ProvisionerAPI) getProvisioningInfo(m *state.Machine, return nil, errors.Trace(err) } - machineSpaces, err := api.machineSpaces(m, spaceInfos, endpointBindings) + cons, err := m.Constraints() + if err != nil { + return nil, errors.Annotate(err, "retrieving machine constraints") + } + machineSpaces, err := api.machineSpaces(cons, spaceInfos, endpointBindings) if err != nil { return nil, errors.Trace(err) } - if result.ProvisioningNetworkTopology, err = api.machineSpaceTopology(m.Id(), machineSpaces); err != nil { + if result.ProvisioningNetworkTopology, err = api.machineSpaceTopology(m.Id(), cons, machineSpaces); err != nil { return nil, errors.Annotate(err, "matching subnets to zones") } @@ -310,14 +315,11 @@ func (api *ProvisionerAPI) machineTags(m *state.Machine, isController bool) (map // // 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,20 @@ func (api *ProvisionerAPI) machineSpaces(m *state.Machine, return includeSpaces.SortedValues(), nil } -func (api *ProvisionerAPI) machineSpaceTopology(machineID string, spaceNames []string) (params.ProvisioningNetworkTopology, error) { +func (api *ProvisionerAPI) machineSpaceTopology( + 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 } diff --git a/apiserver/facades/agent/provisioner/provisioninginfo_test.go b/apiserver/facades/agent/provisioner/provisioninginfo_test.go index 9a16f8ab7dd..0a70e9d4c69 100644 --- a/apiserver/facades/agent/provisioner/provisioninginfo_test.go +++ b/apiserver/facades/agent/provisioner/provisioninginfo_test.go @@ -380,6 +380,83 @@ func (s *withoutControllerSuite) TestConflictingNegativeConstraintWithBindingErr c.Assert(result, jc.DeepEquals, expected) } +func (s *withoutControllerSuite) TestNoSpaceConstraintsProvidedSpaceTopologyEmpty(c *gc.C) { + wordpressMachine, err := s.State.AddOneMachine(state.MachineTemplate{ + Base: state.UbuntuBase("12.10"), + Jobs: []state.MachineJob{state.JobHostUnits}, + }) + c.Assert(err, jc.ErrorIsNil) + + // Simulates running `juju deploy --bind "..."`. + bindings := map[string]string{ + "url": "alpha", + "db": "alpha", + } + wordpressCharm := s.AddTestingCharm(c, "wordpress") + wordpressService := s.AddTestingApplicationWithBindings(c, "wordpress", wordpressCharm, 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(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) + + alphaSpace, err := s.State.SpaceByName("alpha") + c.Assert(err, jc.ErrorIsNil) + + testing.AddSubnetsWithTemplate(c, s.State, 1, network.SubnetInfo{ + CIDR: "10.10.{{add 4 .}}.0/24", + ProviderId: "subnet-alpha", + AvailabilityZones: []string{"zone-alpha"}, + SpaceID: alphaSpace.Id(), + VLANTag: 43, + }) + + cons := constraints.MustParse("spaces=alpha") + wordpressMachine, err := s.State.AddOneMachine(state.MachineTemplate{ + Base: state.UbuntuBase("12.10"), + Jobs: []state.MachineJob{state.JobHostUnits}, + Constraints: cons, + }) + c.Assert(err, jc.ErrorIsNil) + + // Simulates running `juju deploy --bind "..."`. + bindings := map[string]string{ + "url": "alpha", + "db": "alpha", + } + wordpressCharm := s.AddTestingCharm(c, "wordpress") + wordpressService := s.AddTestingApplicationWithBindings(c, "wordpress", wordpressCharm, 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(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) { // Add a couple of spaces. space1, err := s.State.AddSpace("space1", "first space id", nil, true) From 5472ef524ae1ba15ffea62488abb2aa83085bd68 Mon Sep 17 00:00:00 2001 From: Jack Shaw Date: Thu, 6 Jun 2024 16:06:39 +0100 Subject: [PATCH 5/7] Fix aws provider name to ec2 in manual test In a previous PR, we accidentally set the provider to the wrong string "aws" instead of "ec2" like it is everywhere else Fix this Also, as a flybe, change aws query to look for "PublicIpAddress" instead of "PublicDnsName". Under some circumstances, PublicDnsName is empty --- tests/suites/manual/deploy_manual.sh | 2 +- tests/suites/manual/util.sh | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/suites/manual/deploy_manual.sh b/tests/suites/manual/deploy_manual.sh index 931ca2ac75b..f4fdd52b684 100644 --- a/tests/suites/manual/deploy_manual.sh +++ b/tests/suites/manual/deploy_manual.sh @@ -13,7 +13,7 @@ test_deploy_manual() { "lxd") run "run_deploy_manual_lxd" ;; - "aws") + "ec2") run "run_deploy_manual_aws" ;; *) diff --git a/tests/suites/manual/util.sh b/tests/suites/manual/util.sh index 71288c08dfc..a0ad16bcf06 100644 --- a/tests/suites/manual/util.sh +++ b/tests/suites/manual/util.sh @@ -25,7 +25,7 @@ launch_and_wait_addr_ec2() { aws ec2 wait instance-running --instance-ids "${instance_id}" sleep 10 - address=$(aws ec2 describe-instances --instance-ids "${instance_id}" --query 'Reservations[0].Instances[0].PublicDnsName' --output text) + address=$(aws ec2 describe-instances --instance-ids "${instance_id}" --query 'Reservations[0].Instances[0].PublicIpAddress' --output text) # shellcheck disable=SC2086 eval $addr_result="'${address}'" From 9b382ff7f2a6084d2cd6fc8e266f00cd5ed97d6c Mon Sep 17 00:00:00 2001 From: Caner Derici Date: Thu, 6 Jun 2024 20:55:25 +0000 Subject: [PATCH 6/7] fix: prevent distro-info upgrade getting stuck Bump juju/packaging v2.0.1 -> v2.1.0 This brings the DEBIAN_FRONTEND=noninteractive env variable to apt-get install, which is needed for https://bugs.launchpad.net/juju/+bug/2011637 for auto upgrades/restarts. --- go.mod | 2 +- go.sum | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/go.mod b/go.mod index 6b6406fb428..603f3228cc1 100644 --- a/go.mod +++ b/go.mod @@ -64,7 +64,7 @@ require ( github.com/juju/names/v5 v5.0.0 github.com/juju/naturalsort v1.0.0 github.com/juju/os/v2 v2.2.5 - github.com/juju/packaging/v2 v2.0.1 + github.com/juju/packaging/v2 v2.1.0 github.com/juju/persistent-cookiejar v1.0.0 github.com/juju/proxy v1.0.0 github.com/juju/pubsub/v2 v2.0.0 diff --git a/go.sum b/go.sum index 14f5f4ae296..f1260c3a0a4 100644 --- a/go.sum +++ b/go.sum @@ -549,8 +549,8 @@ github.com/juju/naturalsort v1.0.0 h1:kGmUUy3h8mJ5/SJYaqKOBR3f3owEd5R52Lh+Tjg/dN github.com/juju/naturalsort v1.0.0/go.mod h1:Zqa/vGkXr78k47zM6tFmU9phhxKz/PIdqBzpLhJ86zc= github.com/juju/os/v2 v2.2.5 h1:Ayw9aC7axKtGgzy3dFRKx84FxasfISMege0iYDsH6io= github.com/juju/os/v2 v2.2.5/go.mod h1:igGQLjgRSwUery5ZhV/1pZjZkMwnfkAwWCwh5ZfIg+c= -github.com/juju/packaging/v2 v2.0.1 h1:KeTfqx3Z0c6RcM053GJH7mplroXoRSuh/dK5vqDQLn8= -github.com/juju/packaging/v2 v2.0.1/go.mod h1:JC+FIRTJXGLt9wA+iP3ltkzv+aWVMMojB/R47uIAK0Y= +github.com/juju/packaging/v2 v2.1.0 h1:vZToCKwVXKul/8xu0FILCcW/4vLf7pUbbrd/8ra6oQo= +github.com/juju/packaging/v2 v2.1.0/go.mod h1:JC+FIRTJXGLt9wA+iP3ltkzv+aWVMMojB/R47uIAK0Y= github.com/juju/persistent-cookiejar v1.0.0 h1:Ag7+QLzqC2m+OYXy2QQnRjb3gTkEBSZagZ6QozwT3EQ= github.com/juju/persistent-cookiejar v1.0.0/go.mod h1:zrbmo4nBKaiP/Ez3F67ewkMbzGYfXyMvRtbOfuAwG0w= github.com/juju/postgrestest v1.1.0/go.mod h1:/n17Y2T6iFozzXwSCO0JYJ5gSiz2caEtSwAjh/uLXDM= From b8bbd7ce168e6c4d2796de478eaf310519077835 Mon Sep 17 00:00:00 2001 From: Caner Derici Date: Thu, 6 Jun 2024 23:08:38 +0000 Subject: [PATCH 7/7] fix(test): patch mock RunCommandWithRetry in test --- container/lxd/initialisation_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/container/lxd/initialisation_test.go b/container/lxd/initialisation_test.go index e53a54c52c5..907e5bf04b6 100644 --- a/container/lxd/initialisation_test.go +++ b/container/lxd/initialisation_test.go @@ -67,8 +67,8 @@ func (s *InitialiserSuite) SetUpTest(c *gc.C) { // with an identical signature to manager.RunCommandWithRetry which saves each // command it receives in a slice and always returns no output, error code 0 // and a nil error. -func getMockRunCommandWithRetry(calledCmds *[]string) func(string, manager.Retryable, manager.RetryPolicy) (string, int, error) { - return func(cmd string, _ manager.Retryable, _ manager.RetryPolicy) (string, int, error) { +func getMockRunCommandWithRetry(calledCmds *[]string) func(string, manager.Retryable, manager.RetryPolicy, []string) (string, int, error) { + return func(cmd string, _ manager.Retryable, _ manager.RetryPolicy, _ []string) (string, int, error) { *calledCmds = append(*calledCmds, cmd) return "", 0, nil }