diff --git a/api/client/client_http.go b/api/client/client_http.go index 5a80e544..685132bc 100644 --- a/api/client/client_http.go +++ b/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) diff --git a/api/context/context.go b/api/context/context.go index 3f9ab9ab..abe7b186 100644 --- a/api/context/context.go +++ b/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) } } diff --git a/api/server/handlers/handlers_instanceid.go b/api/server/handlers/handlers_instanceid.go index 724db34e..3241af4c 100644 --- a/api/server/handlers/handlers_instanceid.go +++ b/api/server/handlers/handlers_instanceid.go @@ -12,12 +12,19 @@ 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 { @@ -25,7 +32,7 @@ func (h *instanceIDHandler) Name() string { } 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) diff --git a/api/server/server_middleware.go b/api/server/server_middleware.go index 142224bc..d632ac2e 100644 --- a/api/server/server_middleware.go +++ b/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()) } diff --git a/api/types/types_instanceid.go b/api/types/types_instanceid.go index 634efc29..273dc597 100644 --- a/api/types/types_instanceid.go +++ b/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 } diff --git a/api/types/types_instanceid_test.go b/api/types/types_instanceid_test.go index 0cde982c..d505f048 100644 --- a/api/types/types_instanceid_test.go +++ b/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) { diff --git a/api/utils/schema/schema_generated.go b/api/utils/schema/schema_generated.go index 8243f12b..bbd9780a 100644 --- a/api/utils/schema/schema_generated.go +++ b/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." diff --git a/drivers/storage/libstorage/libstorage_client_xcli.go b/drivers/storage/libstorage/libstorage_client_xcli.go index b16e9d26..02cccf0c 100644 --- a/drivers/storage/libstorage/libstorage_client_xcli.go +++ b/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") diff --git a/drivers/storage/libstorage/libstorage_driver.go b/drivers/storage/libstorage/libstorage_driver.go index acc4bd89..8a9b488a 100644 --- a/drivers/storage/libstorage/libstorage_driver.go +++ b/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") diff --git a/drivers/storage/libstorage/libstorage_store.go b/drivers/storage/libstorage/libstorage_store.go index 631d379e..cadd2f0b 100644 --- a/drivers/storage/libstorage/libstorage_store.go +++ b/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 { diff --git a/drivers/storage/libstorage/libstorage_utils.go b/drivers/storage/libstorage/libstorage_utils.go index c3f9d8e8..290ac53d 100644 --- a/drivers/storage/libstorage/libstorage_utils.go +++ b/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) } diff --git a/drivers/storage/vfs/tests/vfs_test.go b/drivers/storage/vfs/tests/vfs_test.go index 088a964a..30e50072 100644 --- a/drivers/storage/vfs/tests/vfs_test.go +++ b/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 } diff --git a/libstorage.apib b/libstorage.apib index 2eaefe43..0703a16d 100644 --- a/libstorage.apib +++ b/libstorage.apib @@ -34,7 +34,7 @@ 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 @@ -42,7 +42,7 @@ 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 diff --git a/libstorage.json b/libstorage.json index 4a549f22..b7aa4a6c 100644 --- a/libstorage.json +++ b/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."