Service Names & Instance IDs

This patch updates the libStorage framework logic to store instance IDs
in maps with the service name as the key instead of the driver name.
This is to account for REX-Ray #685
(rexray/rexray#685).

The instance ID format when marshaled as a string has been updated to
the following:

        DRIVER[:SERVICE]=ID,FIELDS,METADATA

This patch is backwards compatible with regards to the published
REST API. If an instance ID header does not include an optional
SERVICE component, the instance ID will be used for all services
with the specified driver type unless another instance ID header
with a specified service overrides it.

api/client/client_http.go

@@ -60,8 +60,20 @@ func (c *client) httpDo(
 	} else if iidMap, ok := ctx.Value(
 		context.AllInstanceIDsKey).(types.InstanceIDMap); ok {
 		if len(iidMap) > 0 {
-			var iids []fmt.Stringer
+			// iterate over the instance IDs and marshal them to strings,
+			// storing only the unique, marshaled output. any instance ID
+			// that specifies a service should be unique, but those that
+			// do not should collapse into a single header that only
+			// includes the driver
+			iidHdrs := map[string]bool{}
 			for _, iid := range iidMap {
+				szIID := iid.String()
+				if _, ok := iidHdrs[szIID]; !ok {
+					iidHdrs[szIID] = true
+				}
+			}
+			var iids []string
+			for iid := range iidHdrs {
 				iids = append(iids, iid)
 			}
 			ctx = ctx.WithValue(instanceIDHeaderKey, iids)

api/context/context.go

@@ -95,15 +95,16 @@ func WithStorageService(
 	parent context.Context, service types.StorageService) types.Context {
 
 	var (
-		driver     = service.Driver()
-		driverName = strings.ToLower(driver.Name())
+		serviceName = strings.ToLower(service.Name())
+		driver      = service.Driver()
+		driverName  = strings.ToLower(driver.Name())
 	)
 
 	parent = newContext(parent, DriverKey, driver, nil, nil)
 
 	// set the service's InstanceID if present
 	if iidm, ok := parent.Value(AllInstanceIDsKey).(types.InstanceIDMap); ok {
-		if iid, ok := iidm[driverName]; ok {
+		if iid, ok := iidm[serviceName]; ok {
 			parent = newContext(parent, InstanceIDKey, iid, nil, nil)
 		}
 	}

api/server/handlers/handlers_instanceid.go

@@ -12,20 +12,27 @@ import (
 // from the headers
 type instanceIDHandler struct {
 	handler types.APIFunc
+	s2d     map[string]string
 }
 
 // NewInstanceIDHandler returns a new global HTTP filter for grokking the
 // InstanceIDs from the headers
-func NewInstanceIDHandler() types.Middleware {
-	return &instanceIDHandler{}
+func NewInstanceIDHandler(svcs <-chan types.StorageService) types.Middleware {
+	iidh := &instanceIDHandler{
+		s2d: map[string]string{},
+	}
+	for s := range svcs {
+		iidh.s2d[strings.ToLower(s.Name())] = strings.ToLower(s.Driver().Name())
+	}
+	return iidh
 }
 
 func (h *instanceIDHandler) Name() string {
 	return "instanceIDs-handler"
 }
 
 func (h *instanceIDHandler) Handler(m types.APIFunc) types.APIFunc {
-	return (&instanceIDHandler{m}).Handle
+	return (&instanceIDHandler{m, h.s2d}).Handle
 }
 
 // Handle is the type's Handler function.
@@ -38,13 +45,43 @@ func (h *instanceIDHandler) Handle(
 	headers := req.Header[types.InstanceIDHeader]
 	ctx.WithField(types.InstanceIDHeader, headers).Debug("http header")
 
+	// this function has been updated to account for
+	// https://github.com/codedellemc/libstorage/pull/420 and
+	// https://github.com/codedellemc/rexray/issues/685.
+	//
+	// this handler now inspects each instance ID header and stores the
+	// unmarshaled object in one of two maps -- a map keyed by the
+	// service name and a map keyed by the driver name. the service
+	// name-keyed map is only used if the instance ID has a service
+	// name present.
+	//
+	// after the two maps are populated, all of the server's configured
+	// services are iterated. first we check the service name-keyed map
+	// for a service's instance ID, and only then if not present we
+	// check the driver name-keyed map for the service's instance ID.
+
 	valMap := types.InstanceIDMap{}
+	d2i := map[string]*types.InstanceID{}
+	s2i := map[string]*types.InstanceID{}
+
 	for _, h := range headers {
 		val := &types.InstanceID{}
 		if err := val.UnmarshalText([]byte(h)); err != nil {
 			return err
 		}
-		valMap[strings.ToLower(val.Driver)] = val
+		if len(val.Service) > 0 {
+			s2i[strings.ToLower(val.Service)] = val
+		} else {
+			d2i[strings.ToLower(val.Driver)] = val
+		}
+	}
+
+	for s, d := range h.s2d {
+		if iid, ok := s2i[s]; ok {
+			valMap[s] = iid
+		} else if iid, ok := d2i[d]; ok {
+			valMap[s] = iid
+		}
 	}
 
 	ctx = ctx.WithValue(context.AllInstanceIDsKey, valMap)

api/server/server_middleware.go

@@ -2,6 +2,7 @@ package server
 
 import (
 	"github.com/codedellemc/libstorage/api/server/handlers"
+	"github.com/codedellemc/libstorage/api/server/services"
 	"github.com/codedellemc/libstorage/api/types"
 )
 
@@ -18,7 +19,8 @@ func (s *server) initGlobalMiddleware() {
 
 	s.addGlobalMiddleware(handlers.NewTransactionHandler())
 	s.addGlobalMiddleware(handlers.NewErrorHandler())
-	s.addGlobalMiddleware(handlers.NewInstanceIDHandler())
+	s.addGlobalMiddleware(
+		handlers.NewInstanceIDHandler(services.StorageServices(s.ctx)))
 	s.addGlobalMiddleware(handlers.NewLocalDevicesHandler())
 	s.addGlobalMiddleware(handlers.NewOnRequestHandler())
 }

api/types/types_instanceid.go

@@ -7,6 +7,7 @@ import (
 	"fmt"
 	"net/url"
 	"regexp"
+	"strings"
 
 	"github.com/akutz/goof"
 )
@@ -25,6 +26,10 @@ type InstanceID struct {
 	// valid.
 	Driver string `json:"driver"`
 
+	// Service is the name of the libStorage service for which the InstanceID
+	// is valid.
+	Service string `json:"service"`
+
 	// Fields is additional, driver specific data about the Instance ID.
 	Fields map[string]string `json:"fields"`
 
@@ -98,12 +103,16 @@ func (i *InstanceID) UnmarshalMetadata(dest interface{}) error {
 }
 
 // MarshalText marshals InstanceID to a text string that adheres to the format
-// `DRIVER=ID[,METADATA]`. If metadata is present it is encoded as a base64
-// string.
+// `DRIVER[:SERVICE]=ID,FIELDS,[,METADATA]`. If metadata is present it is
+// encoded as a base64 string.
 func (i *InstanceID) MarshalText() ([]byte, error) {
 
 	t := &bytes.Buffer{}
-	fmt.Fprintf(t, "%s=%s", i.Driver, i.ID)
+	fmt.Fprint(t, i.Driver)
+	if len(i.Service) > 0 {
+		fmt.Fprintf(t, ":%s", i.Service)
+	}
+	fmt.Fprintf(t, "=%s", i.ID)
 
 	if len(i.Fields) == 0 && len(i.metadata) == 0 {
 		return t.Bytes(), nil
@@ -149,7 +158,11 @@ func (i *InstanceID) UnmarshalText(value []byte) error {
 		return goof.WithField("value", string(value), "invalid InstanceID")
 	}
 
-	i.Driver = string(m[1])
+	dos := strings.Split(string(m[1]), ":")
+	i.Driver = dos[0]
+	if len(dos) > 1 {
+		i.Service = dos[1]
+	}
 	i.ID = string(m[2])
 
 	if lm > 3 && len(m[3]) > 0 {
@@ -182,9 +195,10 @@ func (i *InstanceID) MarshalJSON() ([]byte, error) {
 	return json.Marshal(&struct {
 		ID       string            `json:"id"`
 		Driver   string            `json:"driver"`
+		Service  string            `json:"service"`
 		Fields   map[string]string `json:"fields,omitempty"`
 		Metadata json.RawMessage   `json:"metadata,omitempty"`
-	}{i.ID, i.Driver, i.Fields, i.metadata})
+	}{i.ID, i.Driver, i.Service, i.Fields, i.metadata})
 }
 
 // UnmarshalJSON marshals the InstanceID to JSON.
@@ -193,6 +207,7 @@ func (i *InstanceID) UnmarshalJSON(data []byte) error {
 	iid := &struct {
 		ID       string            `json:"id"`
 		Driver   string            `json:"driver"`
+		Service  string            `json:"service"`
 		Fields   map[string]string `json:"fields,omitempty"`
 		Metadata json.RawMessage   `json:"metadata,omitempty"`
 	}{}
@@ -203,6 +218,7 @@ func (i *InstanceID) UnmarshalJSON(data []byte) error {
 
 	i.ID = iid.ID
 	i.Driver = iid.Driver
+	i.Service = iid.Service
 	i.Fields = iid.Fields
 	i.metadata = iid.Metadata
 
@@ -223,7 +239,8 @@ func (i *InstanceID) MarshalYAML() (interface{}, error) {
 	return &struct {
 		ID       string                 `yaml:"id"`
 		Driver   string                 `yaml:"driver"`
+		Service  string                 `yaml:"service"`
 		Fields   map[string]string      `yaml:"fields,omitempty"`
 		Metadata map[string]interface{} `yaml:"metadata,omitempty"`
-	}{i.ID, i.Driver, i.Fields, metadata}, nil
+	}{i.ID, i.Driver, i.Service, i.Fields, metadata}, nil
 }

api/types/types_instanceid_test.go

@@ -60,6 +60,24 @@ func TestInstanceIDMarshalText(t *testing.T) {
 		Fields: map[string]string{"region": "west"},
 	}
 	assert.Equal(t, "vfs=1234,region=west", i4.String())
+
+	i5 := &InstanceID{
+		ID:      "1234",
+		Driver:  "vfs",
+		Service: "vfs-001",
+		Fields:  map[string]string{"region": "west"},
+	}
+	assert.Equal(t, "vfs:vfs-001=1234,region=west", i5.String())
+
+	i6 := &InstanceID{}
+	if !assert.NoError(
+		t, i6.UnmarshalText([]byte(`vfs:vfs-001=1234,region=west`))) {
+		t.FailNow()
+	}
+	assert.Equal(t, "1234", i6.ID)
+	assert.Equal(t, "vfs", i6.Driver)
+	assert.Equal(t, "vfs-001", i6.Service)
+	assert.EqualValues(t, map[string]string{"region": "west"}, i6.Fields)
 }
 
 func TestInstanceIDMarshalJSON(t *testing.T) {

api/utils/schema/schema_generated.go

@@ -110,6 +110,10 @@ const (
                     "type": "string",
                     "description": "The name of the driver that created the instance ID."
                 },
+                "service": {
+                    "type": "string",
+                    "description": "The name of the service for which the instance ID is valid."
+                },
                 "formatted": {
                     "type": "boolean",
                     "description": "A flag indicating whether or not the instance ID has been formatted by an instance inspection."

drivers/storage/libstorage/libstorage_client_xcli.go

@@ -101,7 +101,8 @@ func (c *client) InstanceID(
 	driverName := strings.ToLower(si.Driver.Name)
 
 	// check to see if the driver's instance ID is cached
-	if iid := c.instanceIDCache.GetInstanceID(driverName); iid != nil {
+	if iid := c.instanceIDCache.GetInstanceID(serviceName); iid != nil {
+		ctx.WithField("service", serviceName).Debug("found cached instance ID")
 		return iid, nil
 	}
 
@@ -115,6 +116,7 @@ func (c *client) InstanceID(
 		return nil, err
 	}
 
+	iid.Service = ""
 	ctx = ctx.WithValue(context.InstanceIDKey, iid)
 
 	if iid.HasMetadata() {
@@ -125,11 +127,27 @@ func (c *client) InstanceID(
 		}
 		ctx.Debug("received instanceID from API.InstanceInspect call")
 		iid.ID = instance.InstanceID.ID
+		iid.Driver = driverName
+
+		// Set the instance ID's Service field to be the service name. This is
+		// important as when the driver is marshalled to a string for inclusion
+		// in HTTP headers, the Service field will be included in the output.
+		//
+		// Instance ID headers without the Service field that are the same
+		// otherwise can be collapsed into a single header, reducing the
+		// amount of data that needs to be transmitted.
+		//
+		// Since this instance ID required inspection to transform it into its
+		// final format, it's likely that the instance ID is dependent upon
+		// the service and not the same for another service that uses the same
+		// driver type.
+		iid.Service = serviceName
+
 		iid.Fields = instance.InstanceID.Fields
 		iid.DeleteMetadata()
 	}
 
-	c.instanceIDCache.Set(driverName, iid)
+	c.instanceIDCache.Set(serviceName, iid)
 	ctx.Debug("cached instanceID")
 
 	ctx.Debug("xli instanceID success")

drivers/storage/libstorage/libstorage_driver.go

@@ -107,7 +107,7 @@ func (d *driver) Init(ctx types.Context, config gofig.Config) error {
 
 		d.lsxCache = &lss{Store: utils.NewStore()}
 		d.supportedCache = &lss{Store: utils.NewStore()}
-		d.instanceIDCache = &lss{Store: newIIDCache()}
+		d.instanceIDCache = newIIDCache()
 	}
 
 	d.ctx.WithFields(logFields).Info("created libStorage client")

drivers/storage/libstorage/libstorage_store.go

@@ -20,8 +20,8 @@ func (s *lss) GetExecutorInfo(lsx string) *types.ExecutorInfo {
 	return nil
 }
 
-func (s *lss) GetInstanceID(driverName string) *types.InstanceID {
-	return s.Store.GetInstanceID(driverName)
+func (s *lss) GetInstanceID(service string) *types.InstanceID {
+	return s.Store.GetInstanceID(service)
 }
 
 func (s *lss) GetLSXSupported(driverName string) types.LSXSupportedOp {

drivers/storage/libstorage/libstorage_utils.go

@@ -57,11 +57,11 @@ func (c *client) withInstanceID(
 		return ctx
 	}
 
-	if !c.instanceIDCache.IsSet(si.Driver.Name) {
+	if !c.instanceIDCache.IsSet(si.Name) {
 		return ctx
 	}
 
-	iid := c.instanceIDCache.GetInstanceID(si.Driver.Name)
+	iid := c.instanceIDCache.GetInstanceID(si.Name)
 	return ctx.WithValue(context.InstanceIDKey, iid)
 }
 

drivers/storage/vfs/tests/vfs_test.go

@@ -974,6 +974,8 @@ func instanceID() (*types.InstanceID, error) {
 	iid := &types.InstanceID{ID: hostName, Driver: vfs.Name}
 	if ok, _ := strconv.ParseBool(os.Getenv("VFS_INSTANCEID_USE_FIELDS")); ok {
 		iid.Fields = map[string]string{"region": "east"}
+	} else {
+		iid.Service = vfs.Name
 	}
 	return iid, nil
 }

libstorage.apib

@@ -34,15 +34,15 @@ The name of the header is `Libstorage-Instanceid` and it adheres to
 the following format:
 
 ```
-Libstorage-Instanceid: DRIVER=ID,FIELDS,METADATA
+Libstorage-Instanceid: DRIVER[:SERVICE]=ID,FIELDS,METADATA
 ```
 
 The header's value is made up of several pieces of information, as
 illustrated in the above example format:
 
  Name | Description
 ------|------------
-`DRIVER` | The driver for which the instance ID is intended.
+`DRIVER[:SERVICE]` | The name of the driver for which the instance ID is intended. An optional suffix of `:SERVICE` may also be present where `SERVICE` is the name of the service for which the instance ID is valid. This format takes precedence over another instance ID specified only by `DRIVER` type.
 `ID`  | The instance ID. This is a string value.
 `FIELDS` | A URL-encoded string of key-value pairs.
 `METADATA` | A base-64 encoded buffer that can be used to transport temporary "seed" data to a driver's `InstanceInspect` function in order to generate the real instance ID.
@@ -52,9 +52,17 @@ instance ID information generated by EBS and EFS:
 
 ```
 Libstorage-Instanceid: ebs=i-1234,region=west
+Libstorage-Instanceid: ebs:ebs-00=i-5678,region=west
 Libstorage-Instanceid: efs=i-1234,region%3Deast%2Czone%3D2
 ```
 
+The first instance ID header is valid for all services
+that use the `ebs` driver. However, because the second
+instance ID specifies the service name `ebs-00`, the
+second instance ID is used for that service in place
+of the first, even though the first is valid for the
+`ebs` driver type.
+
 Notice that the instance ID for the EFS driver contains
 encoded characters. That's because the `FIELDS` component
 of an instance ID is URL encoded like a query string in

libstorage.json

@@ -106,6 +106,10 @@
                     "type": "string",
                     "description": "The name of the driver that created the instance ID."
                 },
+                "service": {
+                    "type": "string",
+                    "description": "The name of the service for which the instance ID is valid."
+                },
                 "formatted": {
                     "type": "boolean",
                     "description": "A flag indicating whether or not the instance ID has been formatted by an instance inspection."