Issue 1435283: unit address sometimes changes

apiserver/backups/restore.go

@@ -9,7 +9,6 @@ import (
 	"github.com/juju/errors"
 
 	"github.com/juju/juju/apiserver/params"
-	"github.com/juju/juju/network"
 	"github.com/juju/juju/state"
 	"github.com/juju/juju/state/backups"
 )
@@ -27,14 +26,13 @@ func (a *API) Restore(p params.RestoreArgs) error {
 		return errors.Trace(err)
 	}
 
-	addr := network.SelectInternalAddress(machine.Addresses(), false)
-	if addr == "" {
-		return errors.Errorf("machine %q has no internal address", machine)
+	addr, err := machine.PrivateAddress()
+	if err != nil {
+		return errors.Annotatef(err, "error fetching internal address for machine %q", machine)
 	}
-
-	publicAddress := network.SelectPublicAddress(machine.Addresses())
-	if publicAddress == "" {
-		return errors.Errorf("machine %q has no public address", machine)
+	publicAddress, err := machine.PublicAddress()
+	if err != nil {
+		return errors.Annotatef(err, "error fetching public address for machine %q", machine)
 	}
 
 	info, err := a.st.RestoreInfoSetter()
@@ -58,8 +56,8 @@ func (a *API) Restore(p params.RestoreArgs) error {
 	logger.Infof("beginning server side restore of backup %q", p.BackupId)
 	// Restore
 	restoreArgs := backups.RestoreArgs{
-		PrivateAddress: addr,
-		PublicAddress:  publicAddress,
+		PrivateAddress: addr.Value,
+		PublicAddress:  publicAddress.Value,
 		NewInstId:      instanceId,
 		NewInstTag:     machine.Tag(),
 		NewInstSeries:  machine.Series(),

apiserver/client/client.go

@@ -177,24 +177,24 @@ func (c *Client) PublicAddress(p params.PublicAddress) (results params.PublicAdd
 		if err != nil {
 			return results, err
 		}
-		addr := network.SelectPublicAddress(machine.Addresses())
-		if addr == "" {
-			return results, fmt.Errorf("machine %q has no public address", machine)
+		addr, err := machine.PublicAddress()
+		if err != nil {
+			return results, errors.Annotatef(err, "error fetching address for machine %q", machine)
 		}
-		return params.PublicAddressResults{PublicAddress: addr}, nil
+		return params.PublicAddressResults{PublicAddress: addr.Value}, nil
 
 	case names.IsValidUnit(p.Target):
 		unit, err := c.api.state.Unit(p.Target)
 		if err != nil {
 			return results, err
 		}
-		addr, ok := unit.PublicAddress()
-		if !ok {
-			return results, fmt.Errorf("unit %q has no public address", unit)
+		addr, err := unit.PublicAddress()
+		if err != nil {
+			return results, errors.Annotatef(err, "error fetching address for unit %q", unit)
 		}
-		return params.PublicAddressResults{PublicAddress: addr}, nil
+		return params.PublicAddressResults{PublicAddress: addr.Value}, nil
 	}
-	return results, fmt.Errorf("unknown unit or machine %q", p.Target)
+	return results, errors.Errorf("unknown unit or machine %q", p.Target)
 }
 
 // PrivateAddress implements the server side of Client.PrivateAddress.
@@ -205,22 +205,22 @@ func (c *Client) PrivateAddress(p params.PrivateAddress) (results params.Private
 		if err != nil {
 			return results, err
 		}
-		addr := network.SelectInternalAddress(machine.Addresses(), false)
-		if addr == "" {
-			return results, fmt.Errorf("machine %q has no internal address", machine)
+		addr, err := machine.PrivateAddress()
+		if err != nil {
+			return results, errors.Annotatef(err, "error fetching address for machine %q", machine)
 		}
-		return params.PrivateAddressResults{PrivateAddress: addr}, nil
+		return params.PrivateAddressResults{PrivateAddress: addr.Value}, nil
 
 	case names.IsValidUnit(p.Target):
 		unit, err := c.api.state.Unit(p.Target)
 		if err != nil {
 			return results, err
 		}
-		addr, ok := unit.PrivateAddress()
-		if !ok {
-			return results, fmt.Errorf("unit %q has no internal address", unit)
+		addr, err := unit.PrivateAddress()
+		if err != nil {
+			return results, errors.Annotatef(err, "error fetching address for unit %q", unit)
 		}
-		return params.PrivateAddressResults{PrivateAddress: addr}, nil
+		return params.PrivateAddressResults{PrivateAddress: addr.Value}, nil
 	}
 	return results, fmt.Errorf("unknown unit or machine %q", p.Target)
 }

apiserver/client/client_test.go

@@ -2567,13 +2567,14 @@ func (s *clientSuite) TestClientPublicAddressErrors(c *gc.C) {
 	_, err := s.APIState.Client().PublicAddress("wordpress")
 	c.Assert(err, gc.ErrorMatches, `unknown unit or machine "wordpress"`)
 	_, err = s.APIState.Client().PublicAddress("0")
-	c.Assert(err, gc.ErrorMatches, `machine "0" has no public address`)
+	c.Assert(err, gc.ErrorMatches, `error fetching address for machine "0": public no address`)
 	_, err = s.APIState.Client().PublicAddress("wordpress/0")
-	c.Assert(err, gc.ErrorMatches, `unit "wordpress/0" has no public address`)
+	c.Assert(err, gc.ErrorMatches, `error fetching address for unit "wordpress/0": public no address`)
 }
 
 func (s *clientSuite) TestClientPublicAddressMachine(c *gc.C) {
 	s.setUpScenario(c)
+	network.ResetGlobalPreferIPv6()
 
 	// Internally, network.SelectPublicAddress is used; the "most public"
 	// address is returned.
@@ -2609,13 +2610,14 @@ func (s *clientSuite) TestClientPrivateAddressErrors(c *gc.C) {
 	_, err := s.APIState.Client().PrivateAddress("wordpress")
 	c.Assert(err, gc.ErrorMatches, `unknown unit or machine "wordpress"`)
 	_, err = s.APIState.Client().PrivateAddress("0")
-	c.Assert(err, gc.ErrorMatches, `machine "0" has no internal address`)
+	c.Assert(err, gc.ErrorMatches, `error fetching address for machine "0": private no address`)
 	_, err = s.APIState.Client().PrivateAddress("wordpress/0")
-	c.Assert(err, gc.ErrorMatches, `unit "wordpress/0" has no internal address`)
+	c.Assert(err, gc.ErrorMatches, `error fetching address for unit "wordpress/0": private no address`)
 }
 
 func (s *clientSuite) TestClientPrivateAddress(c *gc.C) {
 	s.setUpScenario(c)
+	network.ResetGlobalPreferIPv6()
 
 	// Internally, network.SelectInternalAddress is used; the public
 	// address if no cloud-local one is available.

apiserver/client/filtering.go

@@ -166,12 +166,18 @@ func unitMatchExposure(u *state.Unit, patterns []string) (bool, bool, error) {
 }
 
 func unitMatchSubnet(u *state.Unit, patterns []string) (bool, bool, error) {
-	pub, pubOK := u.PublicAddress()
-	priv, privOK := u.PrivateAddress()
-	if !pubOK && !privOK {
+	pub, pubErr := u.PublicAddress()
+	if pubErr != nil && !network.IsNoAddress(pubErr) {
+		return true, false, errors.Trace(pubErr)
+	}
+	priv, privErr := u.PrivateAddress()
+	if privErr != nil && !network.IsNoAddress(privErr) {
+		return true, false, errors.Trace(privErr)
+	}
+	if pubErr != nil && privErr != nil {
 		return true, false, nil
 	}
-	return matchSubnet(patterns, pub, priv)
+	return matchSubnet(patterns, pub.Value, priv.Value)
 }
 
 func unitMatchPort(u *state.Unit, patterns []string) (bool, bool, error) {

apiserver/client/run.go

@@ -28,16 +28,16 @@ import (
 // by the function that actually tries to execute the command.
 func remoteParamsForMachine(machine *state.Machine, command string, timeout time.Duration) *RemoteExec {
 	// magic boolean parameters are bad :-(
-	address := network.SelectInternalAddress(machine.Addresses(), false)
+	address, ok := network.SelectInternalAddress(machine.Addresses(), false)
 	execParams := &RemoteExec{
 		ExecParams: ssh.ExecParams{
 			Command: command,
 			Timeout: timeout,
 		},
 		MachineId: machine.Id(),
 	}
-	if address != "" {
-		execParams.Host = fmt.Sprintf("ubuntu@%s", address)
+	if ok {
+		execParams.Host = fmt.Sprintf("ubuntu@%s", address.Value)
 	}
 	return execParams
 }

apiserver/client/status.go

@@ -428,7 +428,14 @@ func makeMachineStatus(machine *state.Machine) (status api.MachineStatus) {
 		if err != nil {
 			status.InstanceState = "error"
 		}
-		status.DNSName = network.SelectPublicAddress(machine.Addresses())
+		addr, err := machine.PublicAddress()
+		if err != nil {
+			// Usually this indicates that no addresses have been set on the
+			// machine yet.
+			addr = network.Address{}
+			logger.Warningf("error fetching public address: %q", err)
+		}
+		status.DNSName = addr.Value
 	} else {
 		if errors.IsNotProvisioned(err) {
 			status.InstanceId = "pending"
@@ -609,7 +616,14 @@ func (context *statusContext) processUnits(units map[string]*state.Unit, service
 
 func (context *statusContext) processUnit(unit *state.Unit, serviceCharm string) api.UnitStatus {
 	var result api.UnitStatus
-	result.PublicAddress, _ = unit.PublicAddress()
+	addr, err := unit.PublicAddress()
+	if err != nil {
+		// Usually this indicates that no addresses have been set on the
+		// machine yet.
+		addr = network.Address{}
+		logger.Warningf("error fetching public address: %q", err)
+	}
+	result.PublicAddress = addr.Value
 	unitPorts, _ := unit.OpenedPorts()
 	for _, port := range unitPorts {
 		result.OpenedPorts = append(result.OpenedPorts, port.String())

apiserver/uniter/uniter_base.go

@@ -136,10 +136,11 @@ func (u *uniterBaseAPI) PublicAddress(args params.Entities) (params.StringResult
 			var unit *state.Unit
 			unit, err = u.getUnit(tag)
 			if err == nil {
-				address, ok := unit.PublicAddress()
-				if ok {
-					result.Results[i].Result = address
-				} else {
+				var address network.Address
+				address, err = unit.PublicAddress()
+				if err == nil {
+					result.Results[i].Result = address.Value
+				} else if network.IsNoAddress(err) {
 					err = common.NoAddressSetError(tag, "public")
 				}
 			}
@@ -169,10 +170,11 @@ func (u *uniterBaseAPI) PrivateAddress(args params.Entities) (params.StringResul
 			var unit *state.Unit
 			unit, err = u.getUnit(tag)
 			if err == nil {
-				address, ok := unit.PrivateAddress()
-				if ok {
-					result.Results[i].Result = address
-				} else {
+				var address network.Address
+				address, err = unit.PrivateAddress()
+				if err == nil {
+					result.Results[i].Result = address.Value
+				} else if network.IsNoAddress(err) {
 					err = common.NoAddressSetError(tag, "private")
 				}
 			}
@@ -1024,7 +1026,7 @@ func (u *uniterBaseAPI) EnterScope(args params.RelationUnits) (params.ErrorResul
 			// private address (we already know it).
 			privateAddress, _ := relUnit.PrivateAddress()
 			settings := map[string]interface{}{
-				"private-address": privateAddress,
+				"private-address": privateAddress.Value,
 			}
 			err = relUnit.EnterScope(settings)
 		}

apiserver/uniter/uniter_base_test.go

@@ -442,9 +442,9 @@ func (s *uniterBaseSuite) testPublicAddress(
 		network.NewScopedAddress("1.2.3.4", network.ScopePublic),
 	)
 	c.Assert(err, jc.ErrorIsNil)
-	address, ok := s.wordpressUnit.PublicAddress()
-	c.Assert(address, gc.Equals, "1.2.3.4")
-	c.Assert(ok, jc.IsTrue)
+	address, err := s.wordpressUnit.PublicAddress()
+	c.Assert(address.Value, gc.Equals, "1.2.3.4")
+	c.Assert(err, jc.ErrorIsNil)
 
 	result, err = facade.PublicAddress(args)
 	c.Assert(err, jc.ErrorIsNil)
@@ -487,9 +487,9 @@ func (s *uniterBaseSuite) testPrivateAddress(
 		network.NewScopedAddress("1.2.3.4", network.ScopeCloudLocal),
 	)
 	c.Assert(err, jc.ErrorIsNil)
-	address, ok := s.wordpressUnit.PrivateAddress()
-	c.Assert(address, gc.Equals, "1.2.3.4")
-	c.Assert(ok, jc.IsTrue)
+	address, err := s.wordpressUnit.PrivateAddress()
+	c.Assert(address.Value, gc.Equals, "1.2.3.4")
+	c.Assert(err, jc.ErrorIsNil)
 
 	result, err = facade.PrivateAddress(args)
 	c.Assert(err, jc.ErrorIsNil)

cmd/jujud/bootstrap_test.go

@@ -667,7 +667,8 @@ func (s *BootstrapSuite) makeTestEnv(c *gc.C) {
 
 	addresses, err := inst.Addresses()
 	c.Assert(err, jc.ErrorIsNil)
-	s.bootstrapName = network.SelectPublicAddress(addresses)
+	addr, _ := network.SelectPublicAddress(addresses)
+	s.bootstrapName = addr.Value
 	s.envcfg = env.Config()
 	s.b64yamlEnvcfg = b64yaml(s.envcfg.AllAttrs()).encode()
 }

doc/bazaar-pipelines.txt

@@ -1,86 +0,0 @@
-Bazaar Pipelines
-================
-
-Pipelines are implemented using a bazaar plugin.
-
-  $ mkdir -p ~/.bazaar/plugins
-  $ bzr branch lp:bzr-pipeline ~/.bazaar/plugins/pipeline
-
-Basic info for pipelines can be found using `bzr help pipeline`.
-
-Pipelines require lightweight checkouts, but that is how `cobzr` and how the
-recommendations are specified in the `bazaar-usage.txt` document.
-
-
-Why use pipelines
-=================
-
-Pipelines could be thought of as a doubly linked list of dependent branches.
-
-Often when working you need to break up the implementation, either because the
-work can be more easily reviewed as a collection of small independent changes,
-or the work can be landed incrementally.
-
-Another reason is to avoid mixing new work with other refactoring that occurs
-during the process of writing the new work.  Often when adding new features,
-other parts of code need to change.  The branch is easier to review if the
-prerequisite changes happen seperately.
-
-Sometimes you don't know you want to refactor things until half of it is done
-already.  In this situation you can create a new pipe before the current one,
-and move the changes into it.
-
-  $ bzr add-pipe --before some-refactoring-work
-  $ bzr merge -i :next
-
-This enters you into an interactive merge of the changes from the next branch
-in the pipeline.
-
-
-Merging trunk
-=============
-
-When merging trunk into a pipeline, you should move to the first branch in the
-pipeline.
-
-  $ bzr switch :first
-  $ bzr merge <trunk>
-  # resolve any conflicts that may be there
-  $ bzr commit -m "Merge trunk"
-  $ bzr pump
-
-The pump command is effectively merging each pipe into the next pipe and
-commits without changing the current active pipe.  The pump command starts
-with the active pipe.  If there are conflicts from any particular merge, the
-pumping stops, and the active branch is set to be the branch that had the
-conflicts ready for you to fix the conflicts.
-
-
-Useful aliases
-==============
-
-  $ bzr alias pipes="show-pipeline"
-
-Show the branches in the pipeline.  All branches are considered a pipeline
-with one branch, so you can run this on any branch (actually a lightweight
-checkout). The current pipe is shown with an `*` at the start of the line.
-
-  $ bzr alias next="switch-pipe :next"
-  $ bzr alias prev="switch-pipe :prev"
-
-These two aliases allow you to move around the pipeline using:
-
-  $ bzr next   # move to the next branch in the pipeline
-  $ bzr prev   # move to the previous branch in the pipeline
-
-
-  $ bzr alias pdiff="diff -r branch::prev"
-
-Show me the differences that this branch has introduced compared to the
-previous branch in the pipeline.
-
-  $ bzr alias unpumped="missing --mine :next"
-
-Show the revisions that are in the current branch that are not yet in the next
-branch in the pipeline.
-