provider/openstack: shorten hostnames

Shorten the hostnames we apply to instances
created by the OpenStack provider. We were
including the full model UUID, which is quite
long. Instead, we should include just the
suffix, and the model name.

When listing instances, don't do a prefix
filter on the model UUID. Instead, just look
for "juju-.*" and then match the model UUID
in the client code.

Example old hostname:
    juju-fd943864-df2e-4da1-8e7d-5116a87d4e7c-machine-14

Example new hostname:
    juju-df7591-controller-0

Fixes https://bugs.launchpad.net/juju/+bug/1615601

instance/namespace.go

@@ -19,13 +19,17 @@ const uuidSuffixDigits = 6
 type Namespace interface {
 	// Prefix returns the common part of the hostnames. i.e. 'juju-xxxxxx-'
 	Prefix() string
+
 	// Hostname returns a name suitable to be used for a machine hostname.
 	// This function returns an error if the machine tags is invalid.
 	Hostname(machineID string) (string, error)
 
 	// MachineTag does the reverse of the Hostname method, and extracts the
 	// Tag from the hostname.
 	MachineTag(hostname string) (names.MachineTag, error)
+
+	// Value returns the input prefixed with the namespace prefix.
+	Value(string) string
 }
 
 type namespace struct {
@@ -50,7 +54,12 @@ func (n *namespace) Hostname(machineID string) (string, error) {
 		return "", errors.Errorf("machine ID %q is not a valid machine", machineID)
 	}
 	machineID = strings.Replace(machineID, "/", "-", -1)
-	return n.Prefix() + machineID, nil
+	return n.Value(machineID), nil
+}
+
+// Value returns the input prefixed with the namespace prefix.
+func (n *namespace) Value(s string) string {
+	return n.Prefix() + s
 }
 
 // Hostname implements Namespace.

provider/openstack/cinder.go

@@ -56,7 +56,12 @@ func (env *Environ) cinderProvider() (*cinderProvider, error) {
 	if err != nil {
 		return nil, errors.Trace(err)
 	}
-	return &cinderProvider{storageAdapter, env.name, env.uuid}, nil
+	return &cinderProvider{
+		storageAdapter: storageAdapter,
+		envName:        env.name,
+		modelUUID:      env.uuid,
+		namespace:      env.namespace,
+	}, nil
 }
 
 var newOpenstackStorage = func(env *Environ) (OpenstackStorage, error) {
@@ -85,6 +90,7 @@ type cinderProvider struct {
 	storageAdapter OpenstackStorage
 	envName        string
 	modelUUID      string
+	namespace      instance.Namespace
 }
 
 var _ storage.Provider = (*cinderProvider)(nil)
@@ -103,6 +109,7 @@ func (p *cinderProvider) VolumeSource(providerConfig *storage.Config) (storage.V
 		storageAdapter: p.storageAdapter,
 		envName:        p.envName,
 		modelUUID:      p.modelUUID,
+		namespace:      p.namespace,
 	}
 	return source, nil
 }
@@ -147,6 +154,7 @@ type cinderVolumeSource struct {
 	storageAdapter OpenstackStorage
 	envName        string // non unique, informational only
 	modelUUID      string
+	namespace      instance.Namespace
 }
 
 var _ storage.VolumeSource = (*cinderVolumeSource)(nil)
@@ -174,7 +182,7 @@ func (s *cinderVolumeSource) createVolume(arg storage.VolumeParams) (*storage.Vo
 		// The Cinder documentation incorrectly states the
 		// size parameter is in GB. It is actually GiB.
 		Size: int(math.Ceil(float64(arg.Size / 1024))),
-		Name: resourceName(arg.Tag, s.envName),
+		Name: resourceName(s.namespace, s.envName, arg.Tag.String()),
 		// TODO(axw) use the AZ of the initially attached machine.
 		AvailabilityZone: "",
 		Metadata:         metadata,

provider/openstack/export_test.go

@@ -77,7 +77,20 @@ var (
 func NewCinderVolumeSource(s OpenstackStorage) storage.VolumeSource {
 	const envName = "testenv"
 	modelUUID := testing.ModelTag.Id()
-	return &cinderVolumeSource{s, envName, modelUUID}
+	return &cinderVolumeSource{
+		storageAdapter: s,
+		envName:        envName,
+		modelUUID:      modelUUID,
+		namespace:      fakeNamespace{},
+	}
+}
+
+type fakeNamespace struct {
+	instance.Namespace
+}
+
+func (fakeNamespace) Value(s string) string {
+	return "juju-" + s
 }
 
 // Include images for arches currently supported.  i386 is no longer

provider/openstack/local_test.go

@@ -427,6 +427,13 @@ func (s *localServerSuite) TestStartInstanceHardwareCharacteristics(c *gc.C) {
 	c.Assert(hc.CpuPower, gc.IsNil)
 }
 
+func (s *localServerSuite) TestInstanceName(c *gc.C) {
+	inst, _ := testing.AssertStartInstance(c, s.env, s.ControllerUUID, "100")
+	serverDetail := openstack.InstanceServerDetail(inst)
+	envName := s.env.Config().Name()
+	c.Assert(serverDetail.Name, gc.Matches, "juju-06f00d-"+envName+"-100")
+}
+
 func (s *localServerSuite) TestStartInstanceNetwork(c *gc.C) {
 	cfg, err := s.env.Config().Apply(coretesting.Attrs{
 		// A label that corresponds to a nova test service network

provider/openstack/provider.go

@@ -22,7 +22,6 @@ import (
 	gooseerrors "gopkg.in/goose.v1/errors"
 	"gopkg.in/goose.v1/identity"
 	"gopkg.in/goose.v1/nova"
-	"gopkg.in/juju/names.v2"
 
 	"github.com/juju/juju/cloud"
 	"github.com/juju/juju/cloudconfig/instancecfg"
@@ -77,16 +76,21 @@ func (p EnvironProvider) Open(args environs.OpenParams) (environs.Environ, error
 	if err := validateCloudSpec(args.Cloud); err != nil {
 		return nil, errors.Annotate(err, "validating cloud spec")
 	}
+	uuid := args.Config.UUID()
+	namespace, err := instance.NewNamespace(uuid)
+	if err != nil {
+		return nil, errors.Annotate(err, "creating instance namespace")
+	}
 
 	e := &Environ{
-		name:  args.Config.Name(),
-		uuid:  args.Config.UUID(),
-		cloud: args.Cloud,
+		name:      args.Config.Name(),
+		uuid:      uuid,
+		cloud:     args.Cloud,
+		namespace: namespace,
 	}
 	e.firewaller = p.FirewallerFactory.GetFirewaller(e)
 	e.configurator = p.Configurator
-	err := e.SetConfig(args.Config)
-	if err != nil {
+	if err := e.SetConfig(args.Config); err != nil {
 		return nil, err
 	}
 	return e, nil
@@ -148,9 +152,10 @@ func (p EnvironProvider) newConfig(cfg *config.Config) (*environConfig, error) {
 }
 
 type Environ struct {
-	name  string
-	uuid  string
-	cloud environs.CloudSpec
+	name      string
+	uuid      string
+	cloud     environs.CloudSpec
+	namespace instance.Namespace
 
 	ecfgMutex    sync.Mutex
 	ecfgUnlocked *environConfig
@@ -928,8 +933,9 @@ func (e *Environ) StartInstance(args environs.StartInstanceParams) (*environs.St
 		groupNames = append(groupNames, nova.SecurityGroupName{g.Name})
 	}
 	machineName := resourceName(
-		names.NewMachineTag(args.InstanceConfig.MachineId),
-		e.Config().UUID(),
+		e.namespace,
+		e.name,
+		args.InstanceConfig.MachineId,
 	)
 
 	tryStartNovaInstance := func(
@@ -1064,20 +1070,24 @@ func (e *Environ) listServers(ids []instance.Id) ([]nova.ServerDetail, error) {
 		}
 		return wantedServers, nil
 	}
-	// List all servers that may be in the environment
-	servers, err := e.nova().ListServersDetail(e.machinesFilter())
+	// List all instances in the environment.
+	instances, err := e.AllInstances()
 	if err != nil {
 		return nil, err
 	}
-	// Create a set of the ids of servers that are wanted
-	idSet := make(map[string]struct{}, len(ids))
-	for _, id := range ids {
-		idSet[string(id)] = struct{}{}
-	}
 	// Return only servers with the wanted ids that are currently alive
-	for _, server := range servers {
-		if _, ok := idSet[server.Id]; ok && e.isAliveServer(server) {
-			wantedServers = append(wantedServers, server)
+	for _, inst := range instances {
+		inst := inst.(*openstackInstance)
+		serverDetail := *inst.serverDetail
+		if !e.isAliveServer(serverDetail) {
+			continue
+		}
+		for _, id := range ids {
+			if inst.Id() != id {
+				continue
+			}
+			wantedServers = append(wantedServers, serverDetail)
+			break
 		}
 	}
 	return wantedServers, nil
@@ -1159,16 +1169,15 @@ func (e *Environ) Instances(ids []instance.Id) ([]instance.Instance, error) {
 
 // AllInstances returns all instances in this environment.
 func (e *Environ) AllInstances() ([]instance.Instance, error) {
-	filter := e.machinesFilter()
 	tagFilter := tagValue{tags.JujuModel, e.ecfg().UUID()}
-	return e.allInstances(filter, tagFilter, e.ecfg().useFloatingIP())
+	return e.allInstances(tagFilter, e.ecfg().useFloatingIP())
 }
 
 // allControllerManagedInstances returns all instances managed by this
 // environment's controller, matching the optionally specified filter.
 func (e *Environ) allControllerManagedInstances(controllerUUID string, updateFloatingIPAddresses bool) ([]instance.Instance, error) {
 	tagFilter := tagValue{tags.JujuController, controllerUUID}
-	return e.allInstances(nil, tagFilter, updateFloatingIPAddresses)
+	return e.allInstances(tagFilter, updateFloatingIPAddresses)
 }
 
 type tagValue struct {
@@ -1177,8 +1186,8 @@ type tagValue struct {
 
 // allControllerManagedInstances returns all instances managed by this
 // environment's controller, matching the optionally specified filter.
-func (e *Environ) allInstances(filter *nova.Filter, tagFilter tagValue, updateFloatingIPAddresses bool) ([]instance.Instance, error) {
-	servers, err := e.nova().ListServersDetail(filter)
+func (e *Environ) allInstances(tagFilter tagValue, updateFloatingIPAddresses bool) ([]instance.Instance, error) {
+	servers, err := e.nova().ListServersDetail(jujuMachineFilter())
 	if err != nil {
 		return nil, err
 	}
@@ -1281,15 +1290,16 @@ func allControllerManagedVolumes(storageAdapter OpenstackStorage, controllerUUID
 	return volIds, nil
 }
 
-func resourceName(tag names.Tag, envName string) string {
-	return fmt.Sprintf("juju-%s-%s", envName, tag)
+func resourceName(namespace instance.Namespace, envName, resourceId string) string {
+	return namespace.Value(envName + "-" + resourceId)
 }
 
-// machinesFilter returns a nova.Filter matching all machines in the environment.
-func (e *Environ) machinesFilter() *nova.Filter {
+// jujuMachineFilter returns a nova.Filter matching machines created by Juju.
+// The machines are not filtered to any particular environment. To do that,
+// instance tags must be compared.
+func jujuMachineFilter() *nova.Filter {
 	filter := nova.NewFilter()
-	modelUUID := e.Config().UUID()
-	filter.Set(nova.FilterServer, fmt.Sprintf("juju-%s-machine-\\d*", modelUUID))
+	filter.Set(nova.FilterServer, "juju-.*")
 	return filter
 }