From 34782bb5f85522654975a5e9ef91960648af94da Mon Sep 17 00:00:00 2001 From: akutz Date: Wed, 2 Nov 2016 09:46:50 -0500 Subject: [PATCH 01/28] Release Candidate 0.3.3-rc1 This patch marks release candidate 0.3.3-rc1. --- .docs/about/release-notes.md | 10 ++++++++++ VERSION | 2 +- 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/.docs/about/release-notes.md b/.docs/about/release-notes.md index b7168b73..63eda00e 100644 --- a/.docs/about/release-notes.md +++ b/.docs/about/release-notes.md @@ -3,6 +3,16 @@ Release early, release often --- +## Version 0.3.3 (TBD) +This release includes some minor fixes as well as a new way to query +attachment information about one or more volumes. + +### Enhancements +* Enhanced attachment querying ([#313](https://github.com/codedellemc/libstorage/pull/313)) + +### Bug Fixes +* AWS Config Support ([#314](https://github.com/codedellemc/libstorage/pull/314)) + ## Version 0.3.2 (2016/10/18) This release updates the project to reflect its new location at github.com/codedellemc. diff --git a/VERSION b/VERSION index d15723fb..2fa3441d 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -0.3.2 +0.3.3-rc1 From d9926abae65e69df1a6774f345d196b66e7492ca Mon Sep 17 00:00:00 2001 From: akutz Date: Wed, 2 Nov 2016 11:59:04 -0500 Subject: [PATCH 02/28] Volume Attachment Filtering Logic This patch updates the volume attachment filtering logic so that it can return both attached and unattached volumes in the same query if both bits are set in the mask. Additionally, if bit 2 is set it does not restrict the volumes being returned to only those attached to the provided instance unless the mask also includes bit 8. --- api/server/handlers/handlers_query_params.go | 4 +- api/server/router/volume/volume_routes.go | 107 ++++++++++--- api/types/types_drivers_storage.go | 24 +-- drivers/storage/vfs/tests/vfs_test.go | 152 ++++++++++++++++++- 4 files changed, 252 insertions(+), 35 deletions(-) diff --git a/api/server/handlers/handlers_query_params.go b/api/server/handlers/handlers_query_params.go index 19892dac..6bbc610f 100644 --- a/api/server/handlers/handlers_query_params.go +++ b/api/server/handlers/handlers_query_params.go @@ -49,7 +49,9 @@ func (h *queryParamsHandler) Handle( if len(v[0]) == 0 { store.Set(k, true) } else { - if b, err := strconv.ParseBool(v[0]); err == nil { + if i, err := strconv.ParseInt(v[0], 10, 64); err == nil { + store.Set(k, i) + } else if b, err := strconv.ParseBool(v[0]); err == nil { store.Set(k, b) } else { store.Set(k, v[0]) diff --git a/api/server/router/volume/volume_routes.go b/api/server/router/volume/volume_routes.go index 05da1446..c14f9a5a 100644 --- a/api/server/router/volume/volume_routes.go +++ b/api/server/router/volume/volume_routes.go @@ -5,6 +5,7 @@ import ( "strings" "sync" + log "github.com/Sirupsen/logrus" "github.com/akutz/goof" "github.com/codedellemc/libstorage/api/context" @@ -127,6 +128,66 @@ func (r *router) volumesForService( http.StatusOK) } +func handleVolAttachments( + ctx types.Context, + lf log.Fields, + iid *types.InstanceID, + vol *types.Volume, + attachments types.VolumeAttachmentsTypes) bool { + + if attachments == 0 { + vol.Attachments = nil + return true + } + + if lf == nil { + lf = log.Fields{} + } + + // if only the requesting instance's attachments are requested then + // filter the volume's attachments list + if attachments.Mine() { + atts := []*types.VolumeAttachment{} + for _, a := range vol.Attachments { + alf := log.Fields{ + "attDeviceName": a.DeviceName, + "attDountPoint": a.MountPoint, + "attVolumeID": a.VolumeID, + } + if strings.EqualFold(iid.ID, a.InstanceID.ID) { + atts = append(atts, a) + ctx.WithFields(lf).WithFields(alf).Debug( + "including volume attachment") + } else { + ctx.WithFields(lf).WithFields(alf).Debug( + "omitting volume attachment") + } + } + vol.Attachments = atts + ctx.WithFields(lf).Debug("included volume attached to instance") + } + + // if the volume has no attachments and the mask indicates that + // only attached volumes should be returned then omit this volume + if len(vol.Attachments) == 0 && + attachments.Attached() && + !attachments.Unattached() { + ctx.WithFields(lf).Debug("omitting unattached volume") + return false + } + + // if the volume has attachments and the mask indicates that + // only unattached volumes should be returned then omit this volume + if len(vol.Attachments) > 0 && + !attachments.Attached() && + attachments.Unattached() { + ctx.WithFields(lf).Debug("omitting attached volume") + return false + } + + return true +} + func getFilteredVolumes( ctx types.Context, req *http.Request, @@ -147,6 +208,8 @@ func getFilteredVolumes( return nil, utils.NewMissingInstanceIDError(storSvc.Name()) } + ctx.WithField("attachments", opts.Attachments).Debug("querying volumes") + objs, err := storSvc.Driver().Volumes(ctx, opts) if err != nil { return nil, err @@ -160,34 +223,26 @@ func getFilteredVolumes( for _, obj := range objs { + lf := log.Fields{ + "attachments": opts.Attachments, + "volumeID": obj.ID, + "volumeName": obj.Name, + } + if filterOp == types.FilterEqualityMatch && filterLeft == "name" { + ctx.WithFields(lf).Debug("checking name filter") if !strings.EqualFold(obj.Name, filterRight) { + ctx.WithFields(lf).Debug("omitted volume due to name filter") continue } } - // if only the requesting instance's attachments are requested then - // filter the volume's attachments list - if opts.Attachments.Mine() { - atts := []*types.VolumeAttachment{} - for _, a := range obj.Attachments { - if strings.EqualFold(iid.ID, a.InstanceID.ID) { - atts = append(atts, a) - } - } - obj.Attachments = atts - } - - if opts.Attachments.Attached() && len(obj.Attachments) == 0 { - continue - } - - if opts.Attachments.Unattached() && len(obj.Attachments) > 0 { + if !handleVolAttachments(ctx, lf, iid, obj, opts.Attachments) { continue } if OnVolume != nil { - ctx.Debug("invoking OnVolume handler") + ctx.WithFields(lf).Debug("invoking OnVolume handler") ok, err := OnVolume(ctx, req, store, obj) if err != nil { return nil, err @@ -212,8 +267,8 @@ func (r *router) volumeInspect( attachments := store.GetAttachments() service := context.MustService(ctx) - if _, ok := context.InstanceID(ctx); !ok && - attachments.RequiresInstanceID() { + iid, iidOK := context.InstanceID(ctx) + if !iidOK && attachments.RequiresInstanceID() { return utils.NewMissingInstanceIDError(service.Name()) } @@ -239,10 +294,12 @@ func (r *router) volumeInspect( return nil, err } - volID := strings.ToLower(store.GetString("volumeID")) + volID := store.GetString("volumeID") for _, v := range vols { - if strings.ToLower(v.Name) == volID { - + if strings.EqualFold(v.Name, volID) { + if !handleVolAttachments(ctx, nil, iid, v, attachments) { + return nil, utils.NewNotFoundError(volID) + } if OnVolume != nil { ok, err := OnVolume(ctx, req, store, v) if err != nil { @@ -273,6 +330,10 @@ func (r *router) volumeInspect( return nil, err } + if !handleVolAttachments(ctx, nil, iid, v, attachments) { + return nil, utils.NewNotFoundError(v.ID) + } + if OnVolume != nil { ok, err := OnVolume(ctx, req, store, v) if err != nil { diff --git a/api/types/types_drivers_storage.go b/api/types/types_drivers_storage.go index 8aab8c5e..a0750d0c 100644 --- a/api/types/types_drivers_storage.go +++ b/api/types/types_drivers_storage.go @@ -60,11 +60,6 @@ func ParseVolumeAttachmentTypes(v interface{}) VolumeAttachmentsTypes { switch tv := v.(type) { case VolumeAttachmentsTypes: return tv - case bool: - if tv { - return VolumeAttachmentsTrue - } - return VolumeAttachmentsRequested case int: return VolumeAttachmentsTypes(tv) case uint: @@ -92,6 +87,11 @@ func ParseVolumeAttachmentTypes(v interface{}) VolumeAttachmentsTypes { if b, err := strconv.ParseBool(tv); err == nil { return ParseVolumeAttachmentTypes(b) } + case bool: + if tv { + return VolumeAttachmentsTrue + } + return VolumeAttachmentsRequested } return VolumeAttachmentsNone } @@ -104,7 +104,7 @@ func (v VolumeAttachmentsTypes) RequiresInstanceID() bool { // Requested returns a flag that indicates attachment information is requested. func (v VolumeAttachmentsTypes) Requested() bool { - return v&VolumeAttachmentsRequested > 0 + return v.bitSet(VolumeAttachmentsRequested) } // Mine returns a flag that indicates attachment information should @@ -112,7 +112,7 @@ func (v VolumeAttachmentsTypes) Requested() bool { // instance ID request header. If this bit is set then the instance ID // header is required. func (v VolumeAttachmentsTypes) Mine() bool { - return v&VolumeAttachmentsMine > 0 + return v.bitSet(VolumeAttachmentsMine) } // Devices returns a flag that indicates an attempt should made to map devices @@ -120,19 +120,23 @@ func (v VolumeAttachmentsTypes) Mine() bool { // attachment information. If this bit is set then the instance ID and // local device headers are required. func (v VolumeAttachmentsTypes) Devices() bool { - return v&VolumeAttachmentsDevices > 0 + return v.bitSet(VolumeAttachmentsDevices) } // Attached returns a flag that indicates only volumes that are attached should // be returned. func (v VolumeAttachmentsTypes) Attached() bool { - return v&VolumeAttachmentsAttached > 0 + return v.bitSet(VolumeAttachmentsAttached) } // Unattached returns a flag that indicates only volumes that are unattached // should be returned. func (v VolumeAttachmentsTypes) Unattached() bool { - return v&VolumeAttachmentsUnattached > 0 + return v.bitSet(VolumeAttachmentsUnattached) +} + +func (v VolumeAttachmentsTypes) bitSet(b VolumeAttachmentsTypes) bool { + return v&b == b } // VolumesOpts are options when inspecting a volume. diff --git a/drivers/storage/vfs/tests/vfs_test.go b/drivers/storage/vfs/tests/vfs_test.go index 0cf27976..8debc1ab 100644 --- a/drivers/storage/vfs/tests/vfs_test.go +++ b/drivers/storage/vfs/tests/vfs_test.go @@ -137,6 +137,7 @@ func TestVolumes(t *testing.T) { t.Fatal(err) } for volumeID, volume := range vols { + volume.Attachments = nil assert.NotNil(t, reply["vfs"][volumeID]) assert.EqualValues(t, volume, reply["vfs"][volumeID]) } @@ -145,7 +146,7 @@ func TestVolumes(t *testing.T) { apitests.RunWithClientType(t, types.ControllerClient, vfs.Name, tc, tf) } -func TestVolumesWithAttachments(t *testing.T) { +func TestVolumesWithAttachmentsTrue(t *testing.T) { tc, _, vols, _ := newTestConfigAll(t) tf := func(config gofig.Config, client types.Client, t *testing.T) { reply, err := client.API().Volumes(nil, types.VolumeAttachmentsTrue) @@ -162,6 +163,153 @@ func TestVolumesWithAttachments(t *testing.T) { apitests.Run(t, vfs.Name, tc, tf) } +func TestVolumesWithAttachmentsRequested(t *testing.T) { + tc, _, vols, _ := newTestConfigAll(t) + tf := func(config gofig.Config, client types.Client, t *testing.T) { + reply, err := client.API().Volumes(nil, + types.VolumeAttachmentsRequested) + if err != nil { + t.Fatal(err) + } + + assert.NotNil(t, reply["vfs"]["vfs-000"]) + assert.NotNil(t, reply["vfs"]["vfs-001"]) + assert.NotNil(t, reply["vfs"]["vfs-002"]) + assert.EqualValues(t, vols["vfs-000"], reply["vfs"]["vfs-000"]) + assert.EqualValues(t, vols["vfs-001"], reply["vfs"]["vfs-001"]) + assert.Len(t, reply["vfs"]["vfs-002"].Attachments, 0) + } + apitests.Run(t, vfs.Name, tc, tf) +} + +func TestVolumesWithAttachmentsNone(t *testing.T) { + tc, _, _, _ := newTestConfigAll(t) + tf := func(config gofig.Config, client types.Client, t *testing.T) { + reply, err := client.API().Volumes(nil, 0) + if err != nil { + t.Fatal(err) + } + + assert.NotNil(t, reply["vfs"]["vfs-000"]) + assert.NotNil(t, reply["vfs"]["vfs-001"]) + assert.NotNil(t, reply["vfs"]["vfs-002"]) + assert.Len(t, reply["vfs"]["vfs-000"].Attachments, 0) + assert.Len(t, reply["vfs"]["vfs-001"].Attachments, 0) + assert.Len(t, reply["vfs"]["vfs-002"].Attachments, 0) + } + apitests.Run(t, vfs.Name, tc, tf) +} + +func TestVolumesWithAttachmentsAttached(t *testing.T) { + tc, _, vols, _ := newTestConfigAll(t) + tf := func(config gofig.Config, client types.Client, t *testing.T) { + reply, err := client.API().Volumes(nil, + types.VolumeAttachmentsRequested| + types.VolumeAttachmentsAttached) + if err != nil { + t.Fatal(err) + } + + assert.NotNil(t, reply["vfs"]["vfs-000"]) + assert.NotNil(t, reply["vfs"]["vfs-001"]) + assert.Nil(t, reply["vfs"]["vfs-002"]) + assert.EqualValues(t, vols["vfs-000"], reply["vfs"]["vfs-000"]) + assert.EqualValues(t, vols["vfs-001"], reply["vfs"]["vfs-001"]) + } + apitests.Run(t, vfs.Name, tc, tf) +} + +func TestVolumesWithAttachmentsUnattached(t *testing.T) { + tc, _, _, _ := newTestConfigAll(t) + tf := func(config gofig.Config, client types.Client, t *testing.T) { + reply, err := client.API().Volumes(nil, + types.VolumeAttachmentsRequested| + types.VolumeAttachmentsUnattached) + if err != nil { + t.Fatal(err) + } + + assert.Nil(t, reply["vfs"]["vfs-000"]) + assert.Nil(t, reply["vfs"]["vfs-001"]) + assert.NotNil(t, reply["vfs"]["vfs-002"]) + assert.Len(t, reply["vfs"]["vfs-002"].Attachments, 0) + } + apitests.Run(t, vfs.Name, tc, tf) +} + +func TestVolumesWithAttachmentsAttachedAndUnattached(t *testing.T) { + tc, _, vols, _ := newTestConfigAll(t) + tf := func(config gofig.Config, client types.Client, t *testing.T) { + reply, err := client.API().Volumes(nil, + types.VolumeAttachmentsRequested| + types.VolumeAttachmentsAttached| + types.VolumeAttachmentsUnattached) + if err != nil { + t.Fatal(err) + } + + assert.NotNil(t, reply["vfs"]["vfs-000"]) + assert.NotNil(t, reply["vfs"]["vfs-001"]) + assert.NotNil(t, reply["vfs"]["vfs-002"]) + assert.EqualValues(t, vols["vfs-000"], reply["vfs"]["vfs-000"]) + assert.EqualValues(t, vols["vfs-001"], reply["vfs"]["vfs-001"]) + } + apitests.Run(t, vfs.Name, tc, tf) +} + +func TestVolumesWithAttachmentsMineWithNotMyInstanceID( + t *testing.T) { + tc, _, _, _ := newTestConfigAll(t) + tf := func(config gofig.Config, client types.Client, t *testing.T) { + + ctx := context.Background() + iidm := types.InstanceIDMap{ + "vfs": &types.InstanceID{ID: "none", Driver: "vfs"}, + } + ctx = ctx.WithValue(context.AllInstanceIDsKey, iidm) + + reply, err := client.API().Volumes(ctx, + types.VolumeAttachmentsRequested|types.VolumeAttachmentsMine) + if err != nil { + t.Fatal(err) + } + + assert.NotNil(t, reply["vfs"]["vfs-000"]) + assert.NotNil(t, reply["vfs"]["vfs-001"]) + assert.NotNil(t, reply["vfs"]["vfs-002"]) + assert.Len(t, reply["vfs"]["vfs-000"].Attachments, 0) + assert.Len(t, reply["vfs"]["vfs-001"].Attachments, 0) + assert.Len(t, reply["vfs"]["vfs-002"].Attachments, 0) + } + apitests.RunWithClientType(t, types.ControllerClient, vfs.Name, tc, tf) +} + +func TestVolumesWithAttachmentsAttachedAndMineWithNotMyInstanceID( + t *testing.T) { + tc, _, _, _ := newTestConfigAll(t) + tf := func(config gofig.Config, client types.Client, t *testing.T) { + + ctx := context.Background() + iidm := types.InstanceIDMap{ + "vfs": &types.InstanceID{ID: "none", Driver: "vfs"}, + } + ctx = ctx.WithValue(context.AllInstanceIDsKey, iidm) + + reply, err := client.API().Volumes(ctx, + types.VolumeAttachmentsRequested| + types.VolumeAttachmentsAttached| + types.VolumeAttachmentsMine) + if err != nil { + t.Fatal(err) + } + + assert.Nil(t, reply["vfs"]["vfs-000"]) + assert.Nil(t, reply["vfs"]["vfs-001"]) + assert.Nil(t, reply["vfs"]["vfs-002"]) + } + apitests.RunWithClientType(t, types.ControllerClient, vfs.Name, tc, tf) +} + func TestVolumesWithAttachmentsWithControllerClient(t *testing.T) { tc, _, _, _ := newTestConfigAll(t) tf := func(config gofig.Config, client types.Client, t *testing.T) { @@ -182,6 +330,7 @@ func TestVolumesByService(t *testing.T) { t.Fatal(err) } for volumeID, volume := range vols { + volume.Attachments = nil assert.NotNil(t, reply[volumeID]) assert.EqualValues(t, volume, reply[volumeID]) } @@ -213,6 +362,7 @@ func TestVolumeInspect(t *testing.T) { if err != nil { t.Fatal(err) } + vols[reply.ID].Attachments = nil assert.NotNil(t, reply) assert.EqualValues(t, vols[reply.ID], reply) } From 22b7fb55033c4c26af6fdf3bababfa27a91f0487 Mon Sep 17 00:00:00 2001 From: akutz Date: Wed, 2 Nov 2016 13:41:26 -0500 Subject: [PATCH 03/28] Release Candidate 0.3.3-rc2 This patch marks release candidate 0.3.3-rc2. --- .docs/about/release-notes.md | 2 +- VERSION | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.docs/about/release-notes.md b/.docs/about/release-notes.md index 63eda00e..a418150f 100644 --- a/.docs/about/release-notes.md +++ b/.docs/about/release-notes.md @@ -8,7 +8,7 @@ This release includes some minor fixes as well as a new way to query attachment information about one or more volumes. ### Enhancements -* Enhanced attachment querying ([#313](https://github.com/codedellemc/libstorage/pull/313)) +* Enhanced attachment querying ([#313](https://github.com/codedellemc/libstorage/pull/313), [#316](https://github.com/codedellemc/libstorage/pull/316)) ### Bug Fixes * AWS Config Support ([#314](https://github.com/codedellemc/libstorage/pull/314)) diff --git a/VERSION b/VERSION index 2fa3441d..5261ae6c 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -0.3.3-rc1 +0.3.3-rc2 From 354802ab3276389bce4be9e758210e78078850ed Mon Sep 17 00:00:00 2001 From: Chris Short Date: Thu, 3 Nov 2016 12:02:15 -0400 Subject: [PATCH 04/28] Update README.md to resolve #270 Added language about the Apache License --- README.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/README.md b/README.md index 01cdcdb0..5f64680b 100644 --- a/README.md +++ b/README.md @@ -100,3 +100,6 @@ a portable format such as JSON. ## Documentation [![Docs](https://readthedocs.org/projects/libstorage/badge/?version=latest)](http://libstorage.readthedocs.org) The `libStorage` documentation is available at [libstorage.rtfd.org](http://libstorage.rtfd.org). + +## License +The `libStorage` project is licensed to you under the Apache License, Version 2.0. Please reference the LICENSE file for additional information. From 34ffe54ac15c9a822cdeb57364f8e88a01480134 Mon Sep 17 00:00:00 2001 From: akutz Date: Fri, 4 Nov 2016 10:38:48 -0500 Subject: [PATCH 05/28] Volume Attachment States This patch adds the AttachmentState field to the Volume object model to more summarily reflect a volume's attachment state. --- api/server/router/volume/volume_routes.go | 22 ++++++++++++ api/types/types_model.go | 44 +++++++++++++++++++---- api/utils/schema/schema_generated.go | 6 ++++ drivers/storage/vfs/tests/vfs_test.go | 2 ++ libstorage.json | 6 ++++ 5 files changed, 73 insertions(+), 7 deletions(-) diff --git a/api/server/router/volume/volume_routes.go b/api/server/router/volume/volume_routes.go index c14f9a5a..5a99e1f1 100644 --- a/api/server/router/volume/volume_routes.go +++ b/api/server/router/volume/volume_routes.go @@ -395,6 +395,9 @@ func (r *router) volumeCreate( } } + if v.AttachmentState == 0 { + v.AttachmentState = types.VolumeAvailable + } return v, nil } @@ -439,6 +442,9 @@ func (r *router) volumeCopy( } } + if v.AttachmentState == 0 { + v.AttachmentState = types.VolumeAvailable + } return v, nil } @@ -517,6 +523,10 @@ func (r *router) volumeAttach( } } + if v.AttachmentState == 0 { + v.AttachmentState = types.VolumeAttached + } + return &types.VolumeAttachResponse{ Volume: v, AttachToken: attTokn, @@ -569,6 +579,10 @@ func (r *router) volumeDetach( } } + if v.AttachmentState == 0 { + v.AttachmentState = types.VolumeAvailable + } + return v, nil } @@ -655,6 +669,10 @@ func (r *router) volumeDetachAll( } } + if v.AttachmentState == 0 { + v.AttachmentState = types.VolumeAvailable + } + volumeMap[v.ID] = v } @@ -735,6 +753,10 @@ func (r *router) volumeDetachAllForService( } } + if v.AttachmentState == 0 { + v.AttachmentState = types.VolumeAvailable + } + reply[v.ID] = v } diff --git a/api/types/types_model.go b/api/types/types_model.go index 795474f0..a2ffc598 100644 --- a/api/types/types_model.go +++ b/api/types/types_model.go @@ -124,10 +124,40 @@ type Snapshot struct { Fields map[string]string `json:"fields,omitempty" yaml:",omitempty"` } +// VolumeAttachmentStates is the volume's attachment state possibilities. +type VolumeAttachmentStates int + +const ( + // VolumeAttachmentStateUnknown indicates the driver has set the state, + // but it is explicitly unknown and should not be inferred from the list of + // attachments alone. + VolumeAttachmentStateUnknown VolumeAttachmentStates = 1 + + // VolumeAttached indicates the volume is attached to the instance + // specified in the API call that requested the volume information. + VolumeAttached VolumeAttachmentStates = 2 + + // VolumeAvailable indicates the volume is not attached to any instance. + VolumeAvailable VolumeAttachmentStates = 3 + + // VolumeUnavailable indicates the volume is attached to some instance + // other than the one specified in the API call that requested the + // volume information. + VolumeUnavailable VolumeAttachmentStates = 4 +) + // Volume provides information about a storage volume. type Volume struct { - // The volume's attachments. - Attachments []*VolumeAttachment `json:"attachments,omitempty" yaml:",omitempty"` + // Attachments is information about the instances to which the volume + // is attached. + Attachments []*VolumeAttachment `json:"attachments,omitempty" yaml:"attachments,omitempty"` + + // AttachmentState indicates whether or not a volume is attached. A client + // can surmise the same state stored in this field by inspecting a volume's + // Attachments field, but this field provides the server a means of doing + // that inspection and storing the result so the client does not have to do + // so. + AttachmentState VolumeAttachmentStates `json:"attachmentState,omitempty" yaml:"attachmentState,omitempty"` // The availability zone for which the volume is available. AvailabilityZone string `json:"availabilityZone,omitempty" yaml:"availabilityZone,omitempty"` @@ -139,17 +169,17 @@ type Volume struct { IOPS int64 `json:"iops,omitempty" yaml:"iops,omitempty"` // The name of the volume. - Name string `json:"name"` + Name string `json:"name" yaml:"name,omitempty"` // NetworkName is the name the device is known by in order to discover // locally. NetworkName string `json:"networkName,omitempty" yaml:"networkName,omitempty"` // The size of the volume. - Size int64 `json:"size,omitempty" yaml:",omitempty"` + Size int64 `json:"size,omitempty" yaml:"size,omitempty"` // The volume status. - Status string `json:"status,omitempty" yaml:",omitempty"` + Status string `json:"status,omitempty" yaml:"status,omitempty"` // ID is a piece of information that uniquely identifies the volume on // the storage platform to which the volume belongs. A volume ID is not @@ -157,10 +187,10 @@ type Volume struct { ID string `json:"id" yaml:"id"` // The volume type. - Type string `json:"type"` + Type string `json:"type" yaml:"type"` // Fields are additional properties that can be defined for this type. - Fields map[string]string `json:"fields,omitempty" yaml:",omitempty"` + Fields map[string]string `json:"fields,omitempty" yaml:"fields,omitempty"` } // VolumeName returns the volume's name. diff --git a/api/utils/schema/schema_generated.go b/api/utils/schema/schema_generated.go index 81c10916..e0a2bed5 100644 --- a/api/utils/schema/schema_generated.go +++ b/api/utils/schema/schema_generated.go @@ -31,6 +31,12 @@ const ( "description": "The volume's attachments.", "items": { "$ref": "#/definitions/volumeAttachment" } }, + "attachmentState": { + "type": "number", + "description": "Indicates the volume's attachment state - 0=none,1=unknown,2=attached,3=available,4=unavailable. A volume is marked as attached if attached to the instance specified in the requesting API call. A volume that is attached but not to the requesting instance is marked as unavailable.", + "minimum": 0, + "maximum": 4 + }, "availabilityZone": { "type": "string", "description": "The zone for which the volume is available." diff --git a/drivers/storage/vfs/tests/vfs_test.go b/drivers/storage/vfs/tests/vfs_test.go index 8debc1ab..8c28c701 100644 --- a/drivers/storage/vfs/tests/vfs_test.go +++ b/drivers/storage/vfs/tests/vfs_test.go @@ -1006,6 +1006,7 @@ const volJSON = `{ }, "status": "attached" }], + "attachmentState": 2, "fields": { "owner": "root@example.com", "priority": "2" @@ -1019,6 +1020,7 @@ const volNoAttachJSON = `{ "size": 10240, "id": "vfs-%03[1]d", "type": "myType", + "attachmentState": 3, "fields": { "owner": "root@example.com", "priority": "2" diff --git a/libstorage.json b/libstorage.json index eefd8490..ee3d4cd2 100644 --- a/libstorage.json +++ b/libstorage.json @@ -27,6 +27,12 @@ "description": "The volume's attachments.", "items": { "$ref": "#/definitions/volumeAttachment" } }, + "attachmentState": { + "type": "number", + "description": "Indicates the volume's attachment state - 0=none,1=unknown,2=attached,3=available,4=unavailable. A volume is marked as attached if attached to the instance specified in the requesting API call. A volume that is attached but not to the requesting instance is marked as unavailable.", + "minimum": 0, + "maximum": 4 + }, "availabilityZone": { "type": "string", "description": "The zone for which the volume is available." From 802b46403cdb0a0b4233675e2b63e36b6af32fd2 Mon Sep 17 00:00:00 2001 From: akutz Date: Fri, 4 Nov 2016 10:42:11 -0500 Subject: [PATCH 06/28] Update Travis Builds to Go 1.7.3 This patch updates the Travis builds to use Go 1.7.3. --- .travis.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index 4d7e72c8..77679606 100644 --- a/.travis.yml +++ b/.travis.yml @@ -3,7 +3,7 @@ go_import_path: github.com/codedellemc/libstorage language: go go: - 1.6.3 - - 1.7.1 + - 1.7.3 - tip os: From d77d2f584ff68a74049091b49d7b0e975840f0ea Mon Sep 17 00:00:00 2001 From: akutz Date: Fri, 4 Nov 2016 12:41:12 -0500 Subject: [PATCH 07/28] Release Candidate 0.3.3-rc3 This patch marks release candidate 0.3.3-rc3. --- .docs/about/release-notes.md | 2 +- VERSION | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.docs/about/release-notes.md b/.docs/about/release-notes.md index a418150f..740d7856 100644 --- a/.docs/about/release-notes.md +++ b/.docs/about/release-notes.md @@ -8,7 +8,7 @@ This release includes some minor fixes as well as a new way to query attachment information about one or more volumes. ### Enhancements -* Enhanced attachment querying ([#313](https://github.com/codedellemc/libstorage/pull/313), [#316](https://github.com/codedellemc/libstorage/pull/316)) +* Enhanced attachment querying ([#313](https://github.com/codedellemc/libstorage/pull/313), [#316](https://github.com/codedellemc/libstorage/pull/316), [#319](https://github.com/codedellemc/libstorage/pull/319)) ### Bug Fixes * AWS Config Support ([#314](https://github.com/codedellemc/libstorage/pull/314)) diff --git a/VERSION b/VERSION index 5261ae6c..ffdef902 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -0.3.3-rc2 +0.3.3-rc3 From 422de89c4e304aabc6adccbe5b5075c72ff33874 Mon Sep 17 00:00:00 2001 From: Travis Rhoden Date: Tue, 8 Nov 2016 14:52:50 -0700 Subject: [PATCH 08/28] vbox: fix Supported() when using dmesg First version of this code incorrectly used Command.StdoutPipe in such a way that the code never would work. The pipe was always closed before the byte scanner tried to read from it. Instead just read the whole output and parse that. --- drivers/storage/vbox/executor/vbox_executor.go | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/drivers/storage/vbox/executor/vbox_executor.go b/drivers/storage/vbox/executor/vbox_executor.go index 1d2f4807..4f274f8d 100644 --- a/drivers/storage/vbox/executor/vbox_executor.go +++ b/drivers/storage/vbox/executor/vbox_executor.go @@ -2,6 +2,7 @@ package executor import ( "bufio" + "bytes" "fmt" "io/ioutil" "net" @@ -49,7 +50,7 @@ func (d *driver) Supported( opts types.Store) (bool, error) { // Use dmidecode if installed - if gotil.FileExistsInPath("dmidecode") { + if gotil.FileExistsInPath(dmidecodeCmd) { out, err := exec.Command( dmidecodeCmd, "-s", "system-product-name").Output() if err == nil { @@ -69,19 +70,13 @@ func (d *driver) Supported( } // No luck with dmidecode, try dmesg - cmd := exec.Command("dmesg") - cmdReader, err := cmd.StdoutPipe() + out, err := exec.Command("dmesg").Output() if err != nil { return false, nil } - defer cmdReader.Close() - scanner := bufio.NewScanner(cmdReader) - - err = cmd.Run() - if err != nil { - return false, nil - } + rdr := bytes.NewReader(out) + scanner := bufio.NewScanner(rdr) for scanner.Scan() { if strings.Contains(scanner.Text(), "BIOS VirtualBox") { From bc4542f73fb6155a3fb2d046d934c313cd47e802 Mon Sep 17 00:00:00 2001 From: akutz Date: Wed, 9 Nov 2016 11:18:49 -0600 Subject: [PATCH 09/28] Volume Attachment Filtering Logic This patch updates the volume attachment filtering logic so that it can return both attached and unattached volumes in the same query if both bits are set in the mask. Additionally, if bit 2 is set it does not restrict the volumes being returned to only those attached to the provided instance unless the mask also includes bit 8. --- api/server/handlers/handlers_query_params.go | 4 +- api/server/router/volume/volume_routes.go | 107 +++++++++--- api/types/types_drivers_storage.go | 122 ++++++++++--- drivers/integration/docker/docker.go | 8 +- drivers/integration/docker/docker_utils.go | 4 +- drivers/storage/ebs/storage/ebs_storage.go | 10 +- drivers/storage/ebs/tests/ebs_test.go | 4 +- drivers/storage/efs/storage/efs_storage.go | 2 +- drivers/storage/efs/tests/efs_test.go | 4 +- .../storage/isilon/storage/isilon_storage.go | 8 +- drivers/storage/isilon/tests/isilon_test.go | 6 +- .../rackspace/storage/rackspace_storage.go | 10 +- .../storage/rackspace/tests/rackspace_test.go | 6 +- .../scaleio/storage/scaleio_storage.go | 10 +- drivers/storage/scaleio/tests/scaleio_test.go | 6 +- drivers/storage/vbox/storage/vbox_storage.go | 4 +- drivers/storage/vbox/tests/vbox_test.go | 6 +- drivers/storage/vfs/tests/vfs_test.go | 161 ++++++++++++++++-- 18 files changed, 380 insertions(+), 102 deletions(-) diff --git a/api/server/handlers/handlers_query_params.go b/api/server/handlers/handlers_query_params.go index 19892dac..6bbc610f 100644 --- a/api/server/handlers/handlers_query_params.go +++ b/api/server/handlers/handlers_query_params.go @@ -49,7 +49,9 @@ func (h *queryParamsHandler) Handle( if len(v[0]) == 0 { store.Set(k, true) } else { - if b, err := strconv.ParseBool(v[0]); err == nil { + if i, err := strconv.ParseInt(v[0], 10, 64); err == nil { + store.Set(k, i) + } else if b, err := strconv.ParseBool(v[0]); err == nil { store.Set(k, b) } else { store.Set(k, v[0]) diff --git a/api/server/router/volume/volume_routes.go b/api/server/router/volume/volume_routes.go index 05da1446..c14f9a5a 100644 --- a/api/server/router/volume/volume_routes.go +++ b/api/server/router/volume/volume_routes.go @@ -5,6 +5,7 @@ import ( "strings" "sync" + log "github.com/Sirupsen/logrus" "github.com/akutz/goof" "github.com/codedellemc/libstorage/api/context" @@ -127,6 +128,66 @@ func (r *router) volumesForService( http.StatusOK) } +func handleVolAttachments( + ctx types.Context, + lf log.Fields, + iid *types.InstanceID, + vol *types.Volume, + attachments types.VolumeAttachmentsTypes) bool { + + if attachments == 0 { + vol.Attachments = nil + return true + } + + if lf == nil { + lf = log.Fields{} + } + + // if only the requesting instance's attachments are requested then + // filter the volume's attachments list + if attachments.Mine() { + atts := []*types.VolumeAttachment{} + for _, a := range vol.Attachments { + alf := log.Fields{ + "attDeviceName": a.DeviceName, + "attDountPoint": a.MountPoint, + "attVolumeID": a.VolumeID, + } + if strings.EqualFold(iid.ID, a.InstanceID.ID) { + atts = append(atts, a) + ctx.WithFields(lf).WithFields(alf).Debug( + "including volume attachment") + } else { + ctx.WithFields(lf).WithFields(alf).Debug( + "omitting volume attachment") + } + } + vol.Attachments = atts + ctx.WithFields(lf).Debug("included volume attached to instance") + } + + // if the volume has no attachments and the mask indicates that + // only attached volumes should be returned then omit this volume + if len(vol.Attachments) == 0 && + attachments.Attached() && + !attachments.Unattached() { + ctx.WithFields(lf).Debug("omitting unattached volume") + return false + } + + // if the volume has attachments and the mask indicates that + // only unattached volumes should be returned then omit this volume + if len(vol.Attachments) > 0 && + !attachments.Attached() && + attachments.Unattached() { + ctx.WithFields(lf).Debug("omitting attached volume") + return false + } + + return true +} + func getFilteredVolumes( ctx types.Context, req *http.Request, @@ -147,6 +208,8 @@ func getFilteredVolumes( return nil, utils.NewMissingInstanceIDError(storSvc.Name()) } + ctx.WithField("attachments", opts.Attachments).Debug("querying volumes") + objs, err := storSvc.Driver().Volumes(ctx, opts) if err != nil { return nil, err @@ -160,34 +223,26 @@ func getFilteredVolumes( for _, obj := range objs { + lf := log.Fields{ + "attachments": opts.Attachments, + "volumeID": obj.ID, + "volumeName": obj.Name, + } + if filterOp == types.FilterEqualityMatch && filterLeft == "name" { + ctx.WithFields(lf).Debug("checking name filter") if !strings.EqualFold(obj.Name, filterRight) { + ctx.WithFields(lf).Debug("omitted volume due to name filter") continue } } - // if only the requesting instance's attachments are requested then - // filter the volume's attachments list - if opts.Attachments.Mine() { - atts := []*types.VolumeAttachment{} - for _, a := range obj.Attachments { - if strings.EqualFold(iid.ID, a.InstanceID.ID) { - atts = append(atts, a) - } - } - obj.Attachments = atts - } - - if opts.Attachments.Attached() && len(obj.Attachments) == 0 { - continue - } - - if opts.Attachments.Unattached() && len(obj.Attachments) > 0 { + if !handleVolAttachments(ctx, lf, iid, obj, opts.Attachments) { continue } if OnVolume != nil { - ctx.Debug("invoking OnVolume handler") + ctx.WithFields(lf).Debug("invoking OnVolume handler") ok, err := OnVolume(ctx, req, store, obj) if err != nil { return nil, err @@ -212,8 +267,8 @@ func (r *router) volumeInspect( attachments := store.GetAttachments() service := context.MustService(ctx) - if _, ok := context.InstanceID(ctx); !ok && - attachments.RequiresInstanceID() { + iid, iidOK := context.InstanceID(ctx) + if !iidOK && attachments.RequiresInstanceID() { return utils.NewMissingInstanceIDError(service.Name()) } @@ -239,10 +294,12 @@ func (r *router) volumeInspect( return nil, err } - volID := strings.ToLower(store.GetString("volumeID")) + volID := store.GetString("volumeID") for _, v := range vols { - if strings.ToLower(v.Name) == volID { - + if strings.EqualFold(v.Name, volID) { + if !handleVolAttachments(ctx, nil, iid, v, attachments) { + return nil, utils.NewNotFoundError(volID) + } if OnVolume != nil { ok, err := OnVolume(ctx, req, store, v) if err != nil { @@ -273,6 +330,10 @@ func (r *router) volumeInspect( return nil, err } + if !handleVolAttachments(ctx, nil, iid, v, attachments) { + return nil, utils.NewNotFoundError(v.ID) + } + if OnVolume != nil { ok, err := OnVolume(ctx, req, store, v) if err != nil { diff --git a/api/types/types_drivers_storage.go b/api/types/types_drivers_storage.go index 8aab8c5e..aa40f2c1 100644 --- a/api/types/types_drivers_storage.go +++ b/api/types/types_drivers_storage.go @@ -38,20 +38,90 @@ const ( ) const ( - // VolumeAttachmentsNone specifies no attachment information is requested. - // This is the default value and the same as omitting this parameter - // altogether. - VolumeAttachmentsNone VolumeAttachmentsTypes = 0 - - // VolumeAttachmentsFalse is an alias for VolumeAttachmentsNone. - VolumeAttachmentsFalse = VolumeAttachmentsNone - - // VolumeAttachmentsTrue is a mask of - // VolumeAttachmentsRequested | VolumeAttachmentsMine | - // VolumeAttachmentsDevices | VolumeAttachmentsAttached - VolumeAttachmentsTrue = VolumeAttachmentsRequested | - VolumeAttachmentsMine | VolumeAttachmentsDevices | + // VolAttNone is the default value. This indicates no attachment + // information is requested. + VolAttNone VolumeAttachmentsTypes = 0 + + // VolAttFalse is an alias for VolAttNone. + VolAttFalse = VolAttNone + + // VolAttReq requests attachment information for all retrieved volumes. + // + // Mask: 1 + VolAttReq = VolumeAttachmentsRequested + + // VolAttReqForInstance requests attachment information for volumes attached + // to the instance provided in the instance ID + // + // Mask: 1 | 2 + VolAttReqForInstance = VolAttReq | VolumeAttachmentsMine + + // VolAttReqWithDevMapForInstance requests attachment information for + // volumes attached to the instance provided in the instance ID and perform + // device mappings where possible. + // + // Mask: 1 | 2 | 4 + VolAttReqWithDevMapForInstance = VolAttReqForInstance | + VolumeAttachmentsDevices + + // VolAttReqOnlyAttachedVols requests attachment information for all + // retrieved volumes and return only volumes that are attached to some + // instance. + // + // Mask: 1 | 8 + VolAttReqOnlyAttachedVols = VolAttReq | VolumeAttachmentsAttached + + // VolAttReqOnlyUnattachedVols requests attachment information for + // all retrieved volumes and return only volumes that are not attached to + // any instance. + // + // Mask: 1 | 16 + VolAttReqOnlyUnattachedVols = VolAttReq | VolumeAttachmentsUnattached + + // VolAttReqOnlyVolsAttachedToInstance requests attachment + // information for all retrieved volumes and return only volumes that + // attached to the instance provided in the instance ID. + // + // Mask: 1 | 2 | 8 + VolAttReqOnlyVolsAttachedToInstance = VolAttReqForInstance | VolumeAttachmentsAttached + + // VolAttReqWithDevMapOnlyVolsAttachedToInstance requests attachment + // information for all retrieved volumes and return only volumes that + // attached to the instance provided in the instance ID and perform device + // mappings where possible. + // + // Mask: 1 | 2 | 4 | 8 + VolAttReqWithDevMapOnlyVolsAttachedToInstance = VolumeAttachmentsDevices | + VolAttReqOnlyVolsAttachedToInstance + + // VolAttReqTrue is an alias for + // VolAttReqWithDevMapOnlyVolsAttachedToInstance. + VolAttReqTrue = VolAttReqWithDevMapOnlyVolsAttachedToInstance + + // VolumeAttachmentsTrue is an alias for VolAttReqTrue. + VolumeAttachmentsTrue = VolAttReqTrue + + // VolAttReqOnlyVolsAttachedToInstanceOrUnattachedVols requests attachment + // information for all retrieved volumes and return only volumes that + // attached to the instance provided in the instance ID or are not attached + // to any instance at all. tl;dr - Attached To Me or Available + // + // Mask: 1 | 2 | 8 | 16 + VolAttReqOnlyVolsAttachedToInstanceOrUnattachedVols = 0 | + VolAttReqOnlyVolsAttachedToInstance | + VolumeAttachmentsUnattached + + // VolAttReqWithDevMapOnlyVolsAttachedToInstanceOrUnattachedVols requests + // attachment information for all retrieved volumes and return only volumes + // that attached to the instance provided in the instance ID or are not + // attached to any instance at all and perform device mappings where + // possible. tl;dr - Attached To Me With Device Mappings or Available + // + // Mask: 1 | 2 | 4 | 8 | 16 + VolAttReqWithDevMapOnlyVolsAttachedToInstanceOrUnattachedVols = 0 | + VolumeAttachmentsDevices | + VolAttReqOnlyVolsAttachedToInstanceOrUnattachedVols ) // ParseVolumeAttachmentTypes parses a value into a VolumeAttachmentsTypes @@ -60,11 +130,6 @@ func ParseVolumeAttachmentTypes(v interface{}) VolumeAttachmentsTypes { switch tv := v.(type) { case VolumeAttachmentsTypes: return tv - case bool: - if tv { - return VolumeAttachmentsTrue - } - return VolumeAttachmentsRequested case int: return VolumeAttachmentsTypes(tv) case uint: @@ -92,8 +157,13 @@ func ParseVolumeAttachmentTypes(v interface{}) VolumeAttachmentsTypes { if b, err := strconv.ParseBool(tv); err == nil { return ParseVolumeAttachmentTypes(b) } + case bool: + if tv { + return VolumeAttachmentsTrue + } + return VolumeAttachmentsRequested } - return VolumeAttachmentsNone + return VolAttNone } // RequiresInstanceID returns a flag that indicates whether the attachment @@ -104,7 +174,7 @@ func (v VolumeAttachmentsTypes) RequiresInstanceID() bool { // Requested returns a flag that indicates attachment information is requested. func (v VolumeAttachmentsTypes) Requested() bool { - return v&VolumeAttachmentsRequested > 0 + return v.bitSet(VolumeAttachmentsRequested) } // Mine returns a flag that indicates attachment information should @@ -112,7 +182,7 @@ func (v VolumeAttachmentsTypes) Requested() bool { // instance ID request header. If this bit is set then the instance ID // header is required. func (v VolumeAttachmentsTypes) Mine() bool { - return v&VolumeAttachmentsMine > 0 + return v.bitSet(VolumeAttachmentsMine) } // Devices returns a flag that indicates an attempt should made to map devices @@ -120,19 +190,23 @@ func (v VolumeAttachmentsTypes) Mine() bool { // attachment information. If this bit is set then the instance ID and // local device headers are required. func (v VolumeAttachmentsTypes) Devices() bool { - return v&VolumeAttachmentsDevices > 0 + return v.bitSet(VolumeAttachmentsDevices) } // Attached returns a flag that indicates only volumes that are attached should // be returned. func (v VolumeAttachmentsTypes) Attached() bool { - return v&VolumeAttachmentsAttached > 0 + return v.bitSet(VolumeAttachmentsAttached) } // Unattached returns a flag that indicates only volumes that are unattached // should be returned. func (v VolumeAttachmentsTypes) Unattached() bool { - return v&VolumeAttachmentsUnattached > 0 + return v.bitSet(VolumeAttachmentsUnattached) +} + +func (v VolumeAttachmentsTypes) bitSet(b VolumeAttachmentsTypes) bool { + return v&b == b } // VolumesOpts are options when inspecting a volume. diff --git a/drivers/integration/docker/docker.go b/drivers/integration/docker/docker.go index 6f2fe352..dabc4b95 100644 --- a/drivers/integration/docker/docker.go +++ b/drivers/integration/docker/docker.go @@ -168,7 +168,7 @@ func (d *driver) Mount( "opts": opts}).Info("mounting volume") vol, err := d.volumeInspectByIDOrName( - ctx, volumeID, volumeName, types.VolumeAttachmentsTrue, opts.Opts) + ctx, volumeID, volumeName, types.VolAttReqTrue, opts.Opts) if isErrNotFound(err) && d.volumeCreateImplicit() { var err error if vol, err = d.Create(ctx, volumeName, &types.VolumeCreateOpts{ @@ -223,7 +223,7 @@ func (d *driver) Mount( } vol, err = d.volumeInspectByIDOrName( - ctx, vol.ID, "", types.VolumeAttachmentsTrue, opts.Opts) + ctx, vol.ID, "", types.VolAttReqTrue, opts.Opts) if err != nil { return "", nil, err } @@ -322,7 +322,7 @@ func (d *driver) Unmount( } vol, err := d.volumeInspectByIDOrName( - ctx, volumeID, volumeName, types.VolumeAttachmentsTrue, opts) + ctx, volumeID, volumeName, types.VolAttReqTrue, opts) if err != nil { return err } @@ -400,7 +400,7 @@ func (d *driver) Path( "opts": opts}).Info("getting path to volume") vol, err := d.volumeInspectByIDOrName( - ctx, volumeID, volumeName, types.VolumeAttachmentsTrue, opts) + ctx, volumeID, volumeName, types.VolAttReqTrue, opts) if err != nil { return "", err } else if vol == nil { diff --git a/drivers/integration/docker/docker_utils.go b/drivers/integration/docker/docker_utils.go index 0118d827..b497e8d2 100644 --- a/drivers/integration/docker/docker_utils.go +++ b/drivers/integration/docker/docker_utils.go @@ -50,7 +50,7 @@ func (d *driver) volumeInspectByIDOrName( if volumeID != "" { var err error obj, err = d.volumeInspectByID( - ctx, volumeID, types.VolumeAttachmentsTrue, opts) + ctx, volumeID, types.VolAttReqTrue, opts) if err != nil { return nil, err } @@ -64,7 +64,7 @@ func (d *driver) volumeInspectByIDOrName( if strings.EqualFold(volumeName, o.Name) { if attachments.Requested() { obj, err = d.volumeInspectByID( - ctx, o.ID, types.VolumeAttachmentsTrue, opts) + ctx, o.ID, types.VolAttReqTrue, opts) if err != nil { return nil, err } diff --git a/drivers/storage/ebs/storage/ebs_storage.go b/drivers/storage/ebs/storage/ebs_storage.go index 06b05795..d4414f57 100644 --- a/drivers/storage/ebs/storage/ebs_storage.go +++ b/drivers/storage/ebs/storage/ebs_storage.go @@ -331,7 +331,7 @@ func (d *driver) VolumeCreate(ctx types.Context, volumeName string, } // Return the volume created return d.VolumeInspect(ctx, *vol.VolumeId, &types.VolumeInspectOpts{ - Attachments: types.VolumeAttachmentsTrue, + Attachments: types.VolAttReqTrue, }) } @@ -582,7 +582,7 @@ func (d *driver) VolumeAttach( return nil, "", goof.WithError("error getting volume", err) } volumes, convErr := d.toTypesVolume( - ctx, ec2vols, types.VolumeAttachmentsTrue) + ctx, ec2vols, types.VolAttReqTrue) if convErr != nil { return nil, "", goof.WithError( "error converting to types.Volume", convErr) @@ -634,7 +634,7 @@ func (d *driver) VolumeAttach( // Check if successful attach attachedVol, err := d.VolumeInspect( ctx, volumeID, &types.VolumeInspectOpts{ - Attachments: types.VolumeAttachmentsTrue, + Attachments: types.VolAttReqTrue, Opts: opts.Opts, }) if err != nil { @@ -659,7 +659,7 @@ func (d *driver) VolumeDetach( return nil, goof.WithError("error getting volume", err) } volumes, convErr := d.toTypesVolume( - ctx, ec2vols, types.VolumeAttachmentsTrue) + ctx, ec2vols, types.VolAttReqTrue) if convErr != nil { return nil, goof.WithError("error converting to types.Volume", convErr) } @@ -696,7 +696,7 @@ func (d *driver) VolumeDetach( // check if successful detach detachedVol, err := d.VolumeInspect( ctx, volumeID, &types.VolumeInspectOpts{ - Attachments: types.VolumeAttachmentsTrue, + Attachments: types.VolAttReqTrue, Opts: opts.Opts, }) if err != nil { diff --git a/drivers/storage/ebs/tests/ebs_test.go b/drivers/storage/ebs/tests/ebs_test.go index 7d477f6b..07dd8ccf 100644 --- a/drivers/storage/ebs/tests/ebs_test.go +++ b/drivers/storage/ebs/tests/ebs_test.go @@ -464,7 +464,7 @@ func volumeInspectAttached( log.WithField("volumeID", volumeID).Info("inspecting volume") reply, err := client.API().VolumeInspect( nil, ebs.Name, volumeID, - types.VolumeAttachmentsTrue) + types.VolAttReqTrue) assert.NoError(t, err) if err != nil { @@ -482,7 +482,7 @@ func volumeInspectDetached( log.WithField("volumeID", volumeID).Info("inspecting volume") reply, err := client.API().VolumeInspect( nil, ebs.Name, volumeID, - types.VolumeAttachmentsTrue) + types.VolAttReqTrue) assert.NoError(t, err) if err != nil { diff --git a/drivers/storage/efs/storage/efs_storage.go b/drivers/storage/efs/storage/efs_storage.go index fef324dc..db579264 100644 --- a/drivers/storage/efs/storage/efs_storage.go +++ b/drivers/storage/efs/storage/efs_storage.go @@ -389,7 +389,7 @@ func (d *driver) VolumeAttach( opts *types.VolumeAttachOpts) (*types.Volume, string, error) { vol, err := d.VolumeInspect(ctx, volumeID, - &types.VolumeInspectOpts{Attachments: types.VolumeAttachmentsTrue}) + &types.VolumeInspectOpts{Attachments: types.VolAttReqTrue}) if err != nil { return nil, "", err } diff --git a/drivers/storage/efs/tests/efs_test.go b/drivers/storage/efs/tests/efs_test.go index e7268f55..43cc20e2 100644 --- a/drivers/storage/efs/tests/efs_test.go +++ b/drivers/storage/efs/tests/efs_test.go @@ -246,7 +246,7 @@ func volumeInspectAttached( log.WithField("volumeID", volumeID).Info("inspecting volume") reply, err := client.API().VolumeInspect( nil, efs.Name, volumeID, - types.VolumeAttachmentsTrue) + types.VolAttReqTrue) assert.NoError(t, err) if err != nil { @@ -265,7 +265,7 @@ func volumeInspectDetached( log.WithField("volumeID", volumeID).Info("inspecting volume") reply, err := client.API().VolumeInspect( nil, efs.Name, volumeID, - types.VolumeAttachmentsTrue) + types.VolAttReqTrue) assert.NoError(t, err) if err != nil { diff --git a/drivers/storage/isilon/storage/isilon_storage.go b/drivers/storage/isilon/storage/isilon_storage.go index 1145dfb0..7dcef1c5 100644 --- a/drivers/storage/isilon/storage/isilon_storage.go +++ b/drivers/storage/isilon/storage/isilon_storage.go @@ -240,7 +240,7 @@ func (d *driver) VolumeCreate(ctx types.Context, volumeName string, opts *types.VolumeCreateOpts) (*types.Volume, error) { vol, err := d.VolumeInspect(ctx, volumeName, - &types.VolumeInspectOpts{Attachments: types.VolumeAttachmentsTrue}) + &types.VolumeInspectOpts{Attachments: types.VolAttReqTrue}) if err != nil { return nil, err } @@ -329,7 +329,7 @@ func (d *driver) VolumeAttach( // ensure the volume exists and is exported vol, err := d.VolumeInspect(ctx, volumeID, - &types.VolumeInspectOpts{Attachments: types.VolumeAttachmentsTrue}) + &types.VolumeInspectOpts{Attachments: types.VolAttReqTrue}) if err != nil { return nil, "", err } @@ -390,7 +390,7 @@ func (d *driver) VolumeAttach( } vol, err = d.VolumeInspect(ctx, volumeID, - &types.VolumeInspectOpts{Attachments: types.VolumeAttachmentsTrue}) + &types.VolumeInspectOpts{Attachments: types.VolAttReqTrue}) if err != nil { return nil, "", err } @@ -452,7 +452,7 @@ func (d *driver) VolumeDetach( } return d.VolumeInspect(ctx, volumeID, &types.VolumeInspectOpts{ - Attachments: types.VolumeAttachmentsTrue, + Attachments: types.VolAttReqTrue, }) } diff --git a/drivers/storage/isilon/tests/isilon_test.go b/drivers/storage/isilon/tests/isilon_test.go index 806a9b02..128b29e1 100644 --- a/drivers/storage/isilon/tests/isilon_test.go +++ b/drivers/storage/isilon/tests/isilon_test.go @@ -230,7 +230,7 @@ func volumeInspectAttached( log.WithField("volumeID", volumeID).Info("inspecting volume") reply, err := client.API().VolumeInspect( - nil, isilon.Name, volumeID, types.VolumeAttachmentsTrue) + nil, isilon.Name, volumeID, types.VolAttReqTrue) assert.NoError(t, err) if err != nil { @@ -248,7 +248,7 @@ func volumeInspectAttachedFail( log.WithField("volumeID", volumeID).Info("inspecting volume") reply, err := client.API().VolumeInspect( - nil, isilon.Name, volumeID, types.VolumeAttachmentsTrue) + nil, isilon.Name, volumeID, types.VolAttReqTrue) assert.NoError(t, err) if err != nil { @@ -265,7 +265,7 @@ func volumeInspectDetached( log.WithField("volumeID", volumeID).Info("inspecting volume") reply, err := client.API().VolumeInspect( - nil, isilon.Name, volumeID, types.VolumeAttachmentsTrue) + nil, isilon.Name, volumeID, types.VolAttReqTrue) assert.NoError(t, err) if err != nil { diff --git a/drivers/storage/rackspace/storage/rackspace_storage.go b/drivers/storage/rackspace/storage/rackspace_storage.go index f4bd4250..676c67a0 100644 --- a/drivers/storage/rackspace/storage/rackspace_storage.go +++ b/drivers/storage/rackspace/storage/rackspace_storage.go @@ -138,7 +138,7 @@ func (d *driver) Volumes( ctx types.Context, opts *types.VolumesOpts) ([]*types.Volume, error) { // always return attachments to align against other drivers for now - return d.getVolume(ctx, "", "", types.VolumeAttachmentsTrue) + return d.getVolume(ctx, "", "", types.VolAttReqTrue) } // // VolumeInspect inspects a single volume. @@ -311,7 +311,7 @@ func (d *driver) VolumeDetach( if volumeID == "" { return nil, goof.WithFields(fields, "volumeId is required for VolumeDetach") } - vols, err := d.getVolume(ctx, volumeID, "", types.VolumeAttachmentsTrue) + vols, err := d.getVolume(ctx, volumeID, "", types.VolAttReqTrue) if err != nil { return nil, err } @@ -586,7 +586,7 @@ func (d *driver) createVolume( "error waiting for volume creation to complete", err) } log.WithFields(fields).Debug("created volume") - return translateVolume(resp, types.VolumeAttachmentsTrue), nil + return translateVolume(resp, types.VolAttReqTrue), nil } //Reformats from volumes.Volume to types.Volume credit to github.com/MatMaul @@ -660,7 +660,7 @@ func (d *driver) volumeAttached(ctx types.Context, } volume, err := d.VolumeInspect( ctx, volumeID, &types.VolumeInspectOpts{ - Attachments: types.VolumeAttachmentsTrue}) + Attachments: types.VolAttReqTrue}) if err != nil { return true, goof.WithFieldsE(fields, "error getting volume when waiting", err) } @@ -711,7 +711,7 @@ func (d *driver) waitVolumeAttachStatus( for { volume, err := d.VolumeInspect( ctx, volumeID, - &types.VolumeInspectOpts{Attachments: types.VolumeAttachmentsTrue}) + &types.VolumeInspectOpts{Attachments: types.VolAttReqTrue}) if err != nil { return nil, goof.WithFieldsE(fields, "error getting volume when waiting", err) } diff --git a/drivers/storage/rackspace/tests/rackspace_test.go b/drivers/storage/rackspace/tests/rackspace_test.go index 8f4283cd..81e1ccf9 100644 --- a/drivers/storage/rackspace/tests/rackspace_test.go +++ b/drivers/storage/rackspace/tests/rackspace_test.go @@ -242,7 +242,7 @@ func volumeInspectAttached( log.WithField("volumeID", volumeID).Info("inspecting volume") reply, err := client.API().VolumeInspect( - nil, rackspace.Name, volumeID, types.VolumeAttachmentsTrue) + nil, rackspace.Name, volumeID, types.VolAttReqTrue) assert.NoError(t, err) if err != nil { @@ -259,7 +259,7 @@ func volumeInspectAttachedFail( log.WithField("volumeID", volumeID).Info("inspecting volume") reply, err := client.API().VolumeInspect( - nil, rackspace.Name, volumeID, types.VolumeAttachmentsTrue) + nil, rackspace.Name, volumeID, types.VolAttReqTrue) assert.NoError(t, err) if err != nil { @@ -276,7 +276,7 @@ func volumeInspectDetached( log.WithField("volumeID", volumeID).Info("inspecting volume") reply, err := client.API().VolumeInspect( - nil, rackspace.Name, volumeID, types.VolumeAttachmentsTrue) + nil, rackspace.Name, volumeID, types.VolAttReqTrue) assert.NoError(t, err) if err != nil { diff --git a/drivers/storage/scaleio/storage/scaleio_storage.go b/drivers/storage/scaleio/storage/scaleio_storage.go index 3053b1c6..477d790f 100644 --- a/drivers/storage/scaleio/storage/scaleio_storage.go +++ b/drivers/storage/scaleio/storage/scaleio_storage.go @@ -374,7 +374,7 @@ func (d *driver) VolumeCreate(ctx types.Context, volumeName string, } return d.VolumeInspect(ctx, vol.ID, &types.VolumeInspectOpts{ - Attachments: types.VolumeAttachmentsTrue, + Attachments: types.VolAttReqTrue, }) } @@ -406,7 +406,7 @@ func (d *driver) VolumeCreateFromSnapshot( } volumeInspectOpts := &types.VolumeInspectOpts{ - Attachments: types.VolumeAttachmentsTrue, + Attachments: types.VolAttReqTrue, Opts: opts.Opts, } @@ -478,7 +478,7 @@ func (d *driver) VolumeAttach( vol, err := d.VolumeInspect( ctx, volumeID, &types.VolumeInspectOpts{ - Attachments: types.VolumeAttachmentsTrue, + Attachments: types.VolAttReqTrue, }) if err != nil { return nil, "", goof.WithError("error getting volume", err) @@ -505,7 +505,7 @@ func (d *driver) VolumeAttach( attachedVol, err := d.VolumeInspect( ctx, volumeID, &types.VolumeInspectOpts{ - Attachments: types.VolumeAttachmentsTrue, + Attachments: types.VolAttReqTrue, Opts: opts.Opts, }) if err != nil { @@ -551,7 +551,7 @@ func (d *driver) VolumeDetach( } vol, err := d.VolumeInspect(ctx, volumeID, &types.VolumeInspectOpts{ - Attachments: types.VolumeAttachmentsTrue, + Attachments: types.VolAttReqTrue, }) if err != nil { return nil, err diff --git a/drivers/storage/scaleio/tests/scaleio_test.go b/drivers/storage/scaleio/tests/scaleio_test.go index e93099e1..829ae9bd 100644 --- a/drivers/storage/scaleio/tests/scaleio_test.go +++ b/drivers/storage/scaleio/tests/scaleio_test.go @@ -222,7 +222,7 @@ func volumeInspectAttached( log.WithField("volumeID", volumeID).Info("inspecting volume") reply, err := client.API().VolumeInspect( - nil, sio.Name, volumeID, types.VolumeAttachmentsTrue) + nil, sio.Name, volumeID, types.VolAttReqTrue) assert.NoError(t, err) if err != nil { @@ -239,7 +239,7 @@ func volumeInspectAttachedFail( log.WithField("volumeID", volumeID).Info("inspecting volume") reply, err := client.API().VolumeInspect( - nil, sio.Name, volumeID, types.VolumeAttachmentsTrue) + nil, sio.Name, volumeID, types.VolAttReqTrue) assert.NoError(t, err) if err != nil { @@ -256,7 +256,7 @@ func volumeInspectDetached( log.WithField("volumeID", volumeID).Info("inspecting volume") reply, err := client.API().VolumeInspect( - nil, sio.Name, volumeID, types.VolumeAttachmentsTrue) + nil, sio.Name, volumeID, types.VolAttReqTrue) assert.NoError(t, err) if err != nil { diff --git a/drivers/storage/vbox/storage/vbox_storage.go b/drivers/storage/vbox/storage/vbox_storage.go index 1860ca9b..ea480db7 100644 --- a/drivers/storage/vbox/storage/vbox_storage.go +++ b/drivers/storage/vbox/storage/vbox_storage.go @@ -356,7 +356,7 @@ func (d *driver) VolumeAttach( ) } - volumes, err = d.getVolume(ctx, volumeID, "", types.VolumeAttachmentsTrue) + volumes, err = d.getVolume(ctx, volumeID, "", types.VolAttReqTrue) if err != nil { return nil, "", err } @@ -403,7 +403,7 @@ func (d *driver) VolumeDetach( ctx.Info("detached volume", volumeID) return d.VolumeInspect( ctx, volumeID, &types.VolumeInspectOpts{ - Attachments: types.VolumeAttachmentsTrue}) + Attachments: types.VolAttReqTrue}) } func (d *driver) VolumeDetachAll( diff --git a/drivers/storage/vbox/tests/vbox_test.go b/drivers/storage/vbox/tests/vbox_test.go index 87e658fa..7a107f54 100644 --- a/drivers/storage/vbox/tests/vbox_test.go +++ b/drivers/storage/vbox/tests/vbox_test.go @@ -221,7 +221,7 @@ func volumeInspectAttached( log.WithField("volumeID", volumeID).Info("inspecting volume") reply, err := client.API().VolumeInspect( - nil, vbox.Name, volumeID, types.VolumeAttachmentsTrue) + nil, vbox.Name, volumeID, types.VolAttReqTrue) assert.NoError(t, err) if err != nil { @@ -238,7 +238,7 @@ func volumeInspectAttachedFail( log.WithField("volumeID", volumeID).Info("inspecting volume") reply, err := client.API().VolumeInspect( - nil, vbox.Name, volumeID, types.VolumeAttachmentsTrue) + nil, vbox.Name, volumeID, types.VolAttReqTrue) assert.NoError(t, err) if err != nil { @@ -255,7 +255,7 @@ func volumeInspectDetached( log.WithField("volumeID", volumeID).Info("inspecting volume") reply, err := client.API().VolumeInspect( - nil, vbox.Name, volumeID, types.VolumeAttachmentsTrue) + nil, vbox.Name, volumeID, types.VolAttReqTrue) assert.NoError(t, err) if err != nil { diff --git a/drivers/storage/vfs/tests/vfs_test.go b/drivers/storage/vfs/tests/vfs_test.go index 0cf27976..6cb3574c 100644 --- a/drivers/storage/vfs/tests/vfs_test.go +++ b/drivers/storage/vfs/tests/vfs_test.go @@ -122,7 +122,7 @@ func TestStorageDriverVolumes(t *testing.T) { context.Background().WithValue( context.ServiceKey, vfs.Name), &types.VolumesOpts{ - Attachments: types.VolumeAttachmentsTrue, + Attachments: types.VolAttReqTrue, Opts: utils.NewStore()}) assert.NoError(t, err) assert.Len(t, vols, 2) @@ -132,11 +132,12 @@ func TestStorageDriverVolumes(t *testing.T) { func TestVolumes(t *testing.T) { tc, _, vols, _ := newTestConfigAll(t) tf := func(config gofig.Config, client types.Client, t *testing.T) { - reply, err := client.API().Volumes(nil, 0) + reply, err := client.API().Volumes(nil, types.VolAttNone) if err != nil { t.Fatal(err) } for volumeID, volume := range vols { + volume.Attachments = nil assert.NotNil(t, reply["vfs"][volumeID]) assert.EqualValues(t, volume, reply["vfs"][volumeID]) } @@ -145,10 +146,10 @@ func TestVolumes(t *testing.T) { apitests.RunWithClientType(t, types.ControllerClient, vfs.Name, tc, tf) } -func TestVolumesWithAttachments(t *testing.T) { +func TestVolumesWithAttachmentsTrue(t *testing.T) { tc, _, vols, _ := newTestConfigAll(t) tf := func(config gofig.Config, client types.Client, t *testing.T) { - reply, err := client.API().Volumes(nil, types.VolumeAttachmentsTrue) + reply, err := client.API().Volumes(nil, types.VolAttReqTrue) if err != nil { t.Fatal(err) } @@ -162,11 +163,149 @@ func TestVolumesWithAttachments(t *testing.T) { apitests.Run(t, vfs.Name, tc, tf) } +func TestVolumesWithAttachmentsRequested(t *testing.T) { + tc, _, vols, _ := newTestConfigAll(t) + tf := func(config gofig.Config, client types.Client, t *testing.T) { + reply, err := client.API().Volumes(nil, types.VolAttReq) + if err != nil { + t.Fatal(err) + } + + assert.NotNil(t, reply["vfs"]["vfs-000"]) + assert.NotNil(t, reply["vfs"]["vfs-001"]) + assert.NotNil(t, reply["vfs"]["vfs-002"]) + assert.EqualValues(t, vols["vfs-000"], reply["vfs"]["vfs-000"]) + assert.EqualValues(t, vols["vfs-001"], reply["vfs"]["vfs-001"]) + assert.Len(t, reply["vfs"]["vfs-002"].Attachments, 0) + } + apitests.Run(t, vfs.Name, tc, tf) +} + +func TestVolumesWithAttachmentsNone(t *testing.T) { + tc, _, _, _ := newTestConfigAll(t) + tf := func(config gofig.Config, client types.Client, t *testing.T) { + reply, err := client.API().Volumes(nil, types.VolAttNone) + if err != nil { + t.Fatal(err) + } + + assert.NotNil(t, reply["vfs"]["vfs-000"]) + assert.NotNil(t, reply["vfs"]["vfs-001"]) + assert.NotNil(t, reply["vfs"]["vfs-002"]) + assert.Len(t, reply["vfs"]["vfs-000"].Attachments, 0) + assert.Len(t, reply["vfs"]["vfs-001"].Attachments, 0) + assert.Len(t, reply["vfs"]["vfs-002"].Attachments, 0) + } + apitests.Run(t, vfs.Name, tc, tf) +} + +func TestVolumesWithAttachmentsAttached(t *testing.T) { + tc, _, vols, _ := newTestConfigAll(t) + tf := func(config gofig.Config, client types.Client, t *testing.T) { + reply, err := client.API().Volumes(nil, types.VolAttReqOnlyAttachedVols) + if err != nil { + t.Fatal(err) + } + + assert.NotNil(t, reply["vfs"]["vfs-000"]) + assert.NotNil(t, reply["vfs"]["vfs-001"]) + assert.Nil(t, reply["vfs"]["vfs-002"]) + assert.EqualValues(t, vols["vfs-000"], reply["vfs"]["vfs-000"]) + assert.EqualValues(t, vols["vfs-001"], reply["vfs"]["vfs-001"]) + } + apitests.Run(t, vfs.Name, tc, tf) +} + +func TestVolumesWithAttachmentsUnattached(t *testing.T) { + tc, _, _, _ := newTestConfigAll(t) + tf := func(config gofig.Config, client types.Client, t *testing.T) { + reply, err := client.API().Volumes(nil, + types.VolAttReqOnlyUnattachedVols) + if err != nil { + t.Fatal(err) + } + + assert.Nil(t, reply["vfs"]["vfs-000"]) + assert.Nil(t, reply["vfs"]["vfs-001"]) + assert.NotNil(t, reply["vfs"]["vfs-002"]) + assert.Len(t, reply["vfs"]["vfs-002"].Attachments, 0) + } + apitests.Run(t, vfs.Name, tc, tf) +} + +func TestVolumesWithAttachmentsAttachedAndUnattached(t *testing.T) { + tc, _, vols, _ := newTestConfigAll(t) + tf := func(config gofig.Config, client types.Client, t *testing.T) { + reply, err := client.API().Volumes(nil, + types.VolAttReqOnlyAttachedVols|types.VolAttReqOnlyUnattachedVols) + if err != nil { + t.Fatal(err) + } + + assert.NotNil(t, reply["vfs"]["vfs-000"]) + assert.NotNil(t, reply["vfs"]["vfs-001"]) + assert.NotNil(t, reply["vfs"]["vfs-002"]) + assert.EqualValues(t, vols["vfs-000"], reply["vfs"]["vfs-000"]) + assert.EqualValues(t, vols["vfs-001"], reply["vfs"]["vfs-001"]) + } + apitests.Run(t, vfs.Name, tc, tf) +} + +func TestVolumesWithAttachmentsMineWithNotMyInstanceID( + t *testing.T) { + tc, _, _, _ := newTestConfigAll(t) + tf := func(config gofig.Config, client types.Client, t *testing.T) { + + ctx := context.Background() + iidm := types.InstanceIDMap{ + "vfs": &types.InstanceID{ID: "none", Driver: "vfs"}, + } + ctx = ctx.WithValue(context.AllInstanceIDsKey, iidm) + + reply, err := client.API().Volumes(ctx, types.VolAttReqForInstance) + if err != nil { + t.Fatal(err) + } + + assert.NotNil(t, reply["vfs"]["vfs-000"]) + assert.NotNil(t, reply["vfs"]["vfs-001"]) + assert.NotNil(t, reply["vfs"]["vfs-002"]) + assert.Len(t, reply["vfs"]["vfs-000"].Attachments, 0) + assert.Len(t, reply["vfs"]["vfs-001"].Attachments, 0) + assert.Len(t, reply["vfs"]["vfs-002"].Attachments, 0) + } + apitests.RunWithClientType(t, types.ControllerClient, vfs.Name, tc, tf) +} + +func TestVolumesWithAttachmentsAttachedAndMineWithNotMyInstanceID( + t *testing.T) { + tc, _, _, _ := newTestConfigAll(t) + tf := func(config gofig.Config, client types.Client, t *testing.T) { + + ctx := context.Background() + iidm := types.InstanceIDMap{ + "vfs": &types.InstanceID{ID: "none", Driver: "vfs"}, + } + ctx = ctx.WithValue(context.AllInstanceIDsKey, iidm) + + reply, err := client.API().Volumes(ctx, + types.VolAttReqOnlyVolsAttachedToInstance) + if err != nil { + t.Fatal(err) + } + + assert.Nil(t, reply["vfs"]["vfs-000"]) + assert.Nil(t, reply["vfs"]["vfs-001"]) + assert.Nil(t, reply["vfs"]["vfs-002"]) + } + apitests.RunWithClientType(t, types.ControllerClient, vfs.Name, tc, tf) +} + func TestVolumesWithAttachmentsWithControllerClient(t *testing.T) { tc, _, _, _ := newTestConfigAll(t) tf := func(config gofig.Config, client types.Client, t *testing.T) { - _, err := client.API().Volumes(nil, types.VolumeAttachmentsTrue) + _, err := client.API().Volumes(nil, types.VolAttReqTrue) assert.Error(t, err) assert.Equal(t, "batch processing error", err.Error()) } @@ -182,6 +321,7 @@ func TestVolumesByService(t *testing.T) { t.Fatal(err) } for volumeID, volume := range vols { + volume.Attachments = nil assert.NotNil(t, reply[volumeID]) assert.EqualValues(t, volume, reply[volumeID]) } @@ -193,7 +333,7 @@ func TestVolumesByServiceWithAttachments(t *testing.T) { tc, _, vols, _ := newTestConfigAll(t) tf := func(config gofig.Config, client types.Client, t *testing.T) { reply, err := client.API().VolumesByService( - nil, "vfs", types.VolumeAttachmentsTrue) + nil, "vfs", types.VolAttReqTrue) if err != nil { t.Fatal(err) } @@ -213,6 +353,7 @@ func TestVolumeInspect(t *testing.T) { if err != nil { t.Fatal(err) } + vols[reply.ID].Attachments = nil assert.NotNil(t, reply) assert.EqualValues(t, vols[reply.ID], reply) } @@ -223,7 +364,7 @@ func TestVolumeInspectWithAttachments(t *testing.T) { tc, _, vols, _ := newTestConfigAll(t) tf := func(config gofig.Config, client types.Client, t *testing.T) { reply, err := client.API().VolumeInspect( - nil, "vfs", "vfs-000", types.VolumeAttachmentsTrue) + nil, "vfs", "vfs-000", types.VolAttReqTrue) if err != nil { t.Fatal(err) } @@ -407,7 +548,7 @@ func TestVolumeCreateFromSnapshot(t *testing.T) { tf := func(config gofig.Config, client types.Client, t *testing.T) { ogVol, err := client.API().VolumeInspect( - nil, "vfs", "vfs-000", types.VolumeAttachmentsTrue) + nil, "vfs", "vfs-000", types.VolAttReqTrue) assert.NoError(t, err) volumeName := "Volume 003" @@ -535,7 +676,7 @@ func TestVolumeDetachAllForService(t *testing.T) { assert.EqualValues(t, vols, reply) reply, err = client.API().VolumesByService( - nil, vfs.Name, types.VolumeAttachmentsTrue) + nil, vfs.Name, types.VolAttReqTrue) assert.NoError(t, err) assert.Equal(t, 0, len(reply)) @@ -573,7 +714,7 @@ func TestVolumeDetachAll(t *testing.T) { assert.Equal(t, 3, len(reply[vfs.Name])) assert.EqualValues(t, vols, reply[vfs.Name]) - reply, err = client.API().Volumes(nil, types.VolumeAttachmentsTrue) + reply, err = client.API().Volumes(nil, types.VolAttReqTrue) assert.NoError(t, err) assert.Equal(t, 1, len(reply)) assert.Equal(t, 0, len(reply[vfs.Name])) From 8075ff1bdb428165463a44207873f9147ddfba35 Mon Sep 17 00:00:00 2001 From: akutz Date: Thu, 10 Nov 2016 10:59:34 -0600 Subject: [PATCH 10/28] Release Candidate 0.3.3-rc4 This patch marks release candidate 0.3.3-rc4 --- .docs/about/release-notes.md | 1 + VERSION | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/.docs/about/release-notes.md b/.docs/about/release-notes.md index 740d7856..721e2b93 100644 --- a/.docs/about/release-notes.md +++ b/.docs/about/release-notes.md @@ -12,6 +12,7 @@ attachment information about one or more volumes. ### Bug Fixes * AWS Config Support ([#314](https://github.com/codedellemc/libstorage/pull/314)) +* VirtualBox Executor Fix ([#325](https://github.com/codedellemc/libstorage/pull/325)) ## Version 0.3.2 (2016/10/18) This release updates the project to reflect its new location at diff --git a/VERSION b/VERSION index ffdef902..fad33df1 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -0.3.3-rc3 +0.3.3-rc4 From 488697e476028d2122b0b10451b60999eb0cee02 Mon Sep 17 00:00:00 2001 From: akutz Date: Fri, 11 Nov 2016 13:44:48 -0600 Subject: [PATCH 11/28] Fix for Absent Attachment State This patch fixes the missing attachment state when listing volumes. --- api/server/router/volume/volume_routes.go | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/api/server/router/volume/volume_routes.go b/api/server/router/volume/volume_routes.go index 5a99e1f1..faea4776 100644 --- a/api/server/router/volume/volume_routes.go +++ b/api/server/router/volume/volume_routes.go @@ -185,6 +185,20 @@ func handleVolAttachments( return false } + // determine a volume's attachment state + if len(vol.Attachments) == 0 { + vol.AttachmentState = types.VolumeAvailable + } else if iid != nil { + vol.AttachmentState = types.VolumeUnavailable + for _, a := range vol.Attachments { + if a.InstanceID != nil && + strings.EqualFold(iid.ID, a.InstanceID.ID) { + vol.AttachmentState = types.VolumeAttached + break + } + } + } + return true } From 387b4454f4b4b4b1eacadc23bda43235a2a42ac7 Mon Sep 17 00:00:00 2001 From: akutz Date: Fri, 11 Nov 2016 13:55:31 -0600 Subject: [PATCH 12/28] EBS Get Volumes Attachment States Fix This patch fixes an issue where the EBS storage driver always assumed that the local devices information should be available when inspecting a volume or that a volume's attachment information should always be fetched. --- drivers/storage/ebs/storage/ebs_storage.go | 63 +++++++++++++--------- 1 file changed, 38 insertions(+), 25 deletions(-) diff --git a/drivers/storage/ebs/storage/ebs_storage.go b/drivers/storage/ebs/storage/ebs_storage.go index c585eb7e..8ce3328e 100644 --- a/drivers/storage/ebs/storage/ebs_storage.go +++ b/drivers/storage/ebs/storage/ebs_storage.go @@ -923,39 +923,51 @@ func (d *driver) toTypesVolume( ctx types.Context, ec2vols []*awsec2.Volume, attachments types.VolumeAttachmentsTypes) ([]*types.Volume, error) { - // Get local devices map from context - ld, ldOK := context.LocalDevices(ctx) - if !ldOK { - return nil, errGetLocDevs + + var ( + ld *types.LocalDevices + ldOK bool + ) + + if attachments.Devices() { + // Get local devices map from context + if ld, ldOK = context.LocalDevices(ctx); !ldOK { + return nil, errGetLocDevs + } } var volumesSD []*types.Volume for _, volume := range ec2vols { + var attachmentsSD []*types.VolumeAttachment - // Leave attachment's device name blank if attachments is false - for _, attachment := range volume.Attachments { - deviceName := "" - if attachments.Devices() { - // Compensate for kernel volume mapping i.e. change "/dev/sda" - // to "/dev/xvda" - deviceName = strings.Replace( - *attachment.Device, "sd", ebsUtils.NextDeviceInfo.Prefix, 1) - // Keep device name if it is found in local devices - if _, ok := ld.DeviceMap[deviceName]; !ok { - deviceName = "" + if attachments.Requested() { + // Leave attachment's device name blank if attachments is false + for _, attachment := range volume.Attachments { + deviceName := "" + if attachments.Devices() { + // Compensate for kernel volume mapping i.e. change + // "/dev/sda" to "/dev/xvda" + deviceName = strings.Replace( + *attachment.Device, "sd", + ebsUtils.NextDeviceInfo.Prefix, 1) + // Keep device name if it is found in local devices + if _, ok := ld.DeviceMap[deviceName]; !ok { + deviceName = "" + } } + attachmentSD := &types.VolumeAttachment{ + VolumeID: *attachment.VolumeId, + InstanceID: &types.InstanceID{ + ID: *attachment.InstanceId, + Driver: d.Name(), + }, + DeviceName: deviceName, + Status: *attachment.State, + } + attachmentsSD = append(attachmentsSD, attachmentSD) } - attachmentSD := &types.VolumeAttachment{ - VolumeID: *attachment.VolumeId, - InstanceID: &types.InstanceID{ - ID: *attachment.InstanceId, - Driver: d.Name(), - }, - DeviceName: deviceName, - Status: *attachment.State, - } - attachmentsSD = append(attachmentsSD, attachmentSD) } + name := d.getName(volume.Tags) volumeSD := &types.Volume{ Name: name, @@ -967,6 +979,7 @@ func (d *driver) toTypesVolume( Size: *volume.Size, Attachments: attachmentsSD, } + // Some volume types have no IOPS, so we get nil in volume.Iops if volume.Iops != nil { volumeSD.IOPS = *volume.Iops From 6f5e1477c83cdc84fb79628fd7af0b498987b85c Mon Sep 17 00:00:00 2001 From: akutz Date: Fri, 11 Nov 2016 14:02:58 -0600 Subject: [PATCH 13/28] EFS Get Volumes Attachment States Fix This patch fixes an issue where the EFS storage driver always assumed that the local devices information should be available when inspecting a volume or that a volume's attachment information should always be fetched. --- drivers/storage/efs/storage/efs_storage.go | 113 +++++++++++++-------- 1 file changed, 70 insertions(+), 43 deletions(-) diff --git a/drivers/storage/efs/storage/efs_storage.go b/drivers/storage/efs/storage/efs_storage.go index 4bcb5b53..78f6cd3e 100644 --- a/drivers/storage/efs/storage/efs_storage.go +++ b/drivers/storage/efs/storage/efs_storage.go @@ -154,7 +154,8 @@ func (d *driver) Volumes( var atts []*types.VolumeAttachment if opts.Attachments.Requested() { - atts, err = d.getVolumeAttachments(ctx, *fileSystem.FileSystemId) + atts, err = d.getVolumeAttachments( + ctx, *fileSystem.FileSystemId, opts.Attachments) if err != nil { return nil, err } @@ -174,52 +175,53 @@ func (d *driver) VolumeInspect( volumeID string, opts *types.VolumeInspectOpts) (*types.Volume, error) { - resp, err := d.efsClient().DescribeFileSystems(&awsefs.DescribeFileSystemsInput{ - FileSystemId: aws.String(volumeID), - }) + resp, err := d.efsClient().DescribeFileSystems( + &awsefs.DescribeFileSystemsInput{FileSystemId: aws.String(volumeID)}) if err != nil { return nil, err } - if len(resp.FileSystems) > 0 { - fileSystem := resp.FileSystems[0] - // Only volumes in "available" state - if *fileSystem.LifeCycleState != awsefs.LifeCycleStateAvailable { - return nil, nil - } + if len(resp.FileSystems) == 0 { + return nil, types.ErrNotFound{} + } - // Name is optional via tag so make sure it exists - var fileSystemName string - if fileSystem.Name != nil { - fileSystemName = *fileSystem.Name - } else { - ctx.WithFields(log.Fields{ - "filesystemid": *fileSystem.FileSystemId, - }).Warn("missing EFS filesystem name") - } + fileSystem := resp.FileSystems[0] - volume := &types.Volume{ - Name: d.getPrintableName(fileSystemName), - ID: *fileSystem.FileSystemId, - Size: *fileSystem.SizeInBytes.Value, - Attachments: nil, - } + // Only volumes in "available" state + if *fileSystem.LifeCycleState != awsefs.LifeCycleStateAvailable { + return nil, nil + } - var atts []*types.VolumeAttachment + // Name is optional via tag so make sure it exists + var fileSystemName string + if fileSystem.Name != nil { + fileSystemName = *fileSystem.Name + } else { + ctx.WithFields(log.Fields{ + "filesystemid": *fileSystem.FileSystemId, + }).Warn("missing EFS filesystem name") + } - if opts.Attachments.Requested() { - atts, err = d.getVolumeAttachments(ctx, *fileSystem.FileSystemId) - if err != nil { - return nil, err - } - } - if len(atts) > 0 { - volume.Attachments = atts - } - return volume, nil + volume := &types.Volume{ + Name: d.getPrintableName(fileSystemName), + ID: *fileSystem.FileSystemId, + Size: *fileSystem.SizeInBytes.Value, + Attachments: nil, } - return nil, types.ErrNotFound{} + var atts []*types.VolumeAttachment + + if opts.Attachments.Requested() { + atts, err = d.getVolumeAttachments( + ctx, *fileSystem.FileSystemId, opts.Attachments) + if err != nil { + return nil, err + } + } + if len(atts) > 0 { + volume.Attachments = atts + } + return volume, nil } // VolumeCreate creates a new volume. @@ -534,12 +536,22 @@ func (d *driver) getFullVolumeName(name string) string { return d.tag() + tagDelimiter + name } -func (d *driver) getVolumeAttachments(ctx types.Context, volumeID string) ( +var errGetLocDevs = goof.New("error getting local devices from context") + +func (d *driver) getVolumeAttachments( + ctx types.Context, + volumeID string, + attachments types.VolumeAttachmentsTypes) ( []*types.VolumeAttachment, error) { + if !attachments.Requested() { + return nil, nil + } + if volumeID == "" { return nil, goof.New("missing volume ID") } + resp, err := d.efsClient().DescribeMountTargets( &awsefs.DescribeMountTargetsInput{ FileSystemId: aws.String(volumeID), @@ -548,12 +560,24 @@ func (d *driver) getVolumeAttachments(ctx types.Context, volumeID string) ( return nil, err } - ld, ldOK := context.LocalDevices(ctx) + var ( + ld *types.LocalDevices + ldOK bool + ) + + if attachments.Devices() { + // Get local devices map from context + if ld, ldOK = context.LocalDevices(ctx); !ldOK { + return nil, errGetLocDevs + } + } var atts []*types.VolumeAttachment for _, mountTarget := range resp.MountTargets { - var dev string - var status string + var ( + dev string + status string + ) if ldOK { // TODO(kasisnu): Check lifecycle state and build the path better dev = *mountTarget.IpAddress + ":" + "/" @@ -566,8 +590,11 @@ func (d *driver) getVolumeAttachments(ctx types.Context, volumeID string) ( status = "Exported" } attachmentSD := &types.VolumeAttachment{ - VolumeID: *mountTarget.FileSystemId, - InstanceID: &types.InstanceID{ID: *mountTarget.SubnetId, Driver: d.Name()}, + VolumeID: *mountTarget.FileSystemId, + InstanceID: &types.InstanceID{ + ID: *mountTarget.SubnetId, + Driver: d.Name(), + }, DeviceName: dev, Status: status, } From 334b3b61d3a49a022271ad41ebbc74d01eb319db Mon Sep 17 00:00:00 2001 From: Travis Rhoden Date: Fri, 11 Nov 2016 14:59:26 -0700 Subject: [PATCH 14/28] VirtualBox Get Volumes Attachment States Fix The vbox driver was always returning attachment information, regardless of whether it was requested. Additionally, it was always trying to map volumes to devices. Change the behavior by only doing the necessary steps based on the attachment types requested. --- drivers/storage/vbox/storage/vbox_storage.go | 50 ++++++++++++-------- 1 file changed, 30 insertions(+), 20 deletions(-) diff --git a/drivers/storage/vbox/storage/vbox_storage.go b/drivers/storage/vbox/storage/vbox_storage.go index 80de3c42..f9b45686 100644 --- a/drivers/storage/vbox/storage/vbox_storage.go +++ b/drivers/storage/vbox/storage/vbox_storage.go @@ -234,7 +234,7 @@ func (d *driver) VolumeCreate(ctx types.Context, volumeName string, size := *opts.Size * 1024 * 1024 * 1024 - vol, err := d.getVolume(ctx, "", volumeName, 0) + vol, err := d.getVolume(ctx, "", volumeName, types.VolAttFalse) if err != nil { return nil, err } @@ -328,7 +328,7 @@ func (d *driver) VolumeAttach( } // review volume with attachments to any host - volumes, err := d.getVolume(ctx, volumeID, "", 0) + volumes, err := d.getVolume(ctx, volumeID, "", types.VolAttReq) if err != nil { return nil, "", err } @@ -385,7 +385,7 @@ func (d *driver) VolumeDetach( return nil, goof.New("missing volume id") } - volumes, err := d.getVolume(ctx, volumeID, "", 0) + volumes, err := d.getVolume(ctx, volumeID, "", types.VolAttFalse) if err != nil { return nil, err } @@ -394,6 +394,8 @@ func (d *driver) VolumeDetach( return nil, goof.New("no volume returned") } + // TODO: Check if volumes[[0].Attachments > 0? + if err := d.detachVolume(ctx, volumeID, ""); err != nil { return nil, goof.WithFieldsE( log.Fields{ @@ -495,7 +497,7 @@ func (d *driver) getVolume( } var mapDN map[string]string - if attachments.Requested() { + if attachments.Devices() { volumeMapping, err := d.getVolumeMapping(ctx) if err != nil { return nil, err @@ -510,25 +512,33 @@ func (d *driver) getVolume( var volumesSD []*types.Volume for _, v := range volumes { - var attachmentsSD []*types.VolumeAttachment - for _, mid := range v.MachineIDs { - dn, _ := mapDN[v.ID] - attachmentSD := &types.VolumeAttachment{ - VolumeID: v.ID, - InstanceID: &types.InstanceID{ID: mid, Driver: vbox.Name}, - DeviceName: dn, - Status: v.Location, - } - attachmentsSD = append(attachmentsSD, attachmentSD) + volumeSD := &types.Volume{ + Name: v.Name, + ID: v.ID, + Size: int64(v.LogicalSize / 1024 / 1024 / 1024), + Status: v.Location, + Type: string(v.DeviceType), } - volumeSD := &types.Volume{ - Name: v.Name, - ID: v.ID, - Size: int64(v.LogicalSize / 1024 / 1024 / 1024), - Status: v.Location, - Attachments: attachmentsSD, + if attachments.Requested() { + var attachmentsSD []*types.VolumeAttachment + for _, mid := range v.MachineIDs { + attachmentSD := &types.VolumeAttachment{ + VolumeID: v.ID, + InstanceID: &types.InstanceID{ + ID: mid, + Driver: vbox.Name, + }, + } + if attachments.Devices() && mapDN != nil { + dn, _ := mapDN[v.ID] + attachmentSD.DeviceName = dn + } + attachmentsSD = append(attachmentsSD, attachmentSD) + } + volumeSD.Attachments = attachmentsSD } + volumesSD = append(volumesSD, volumeSD) } From 610ddd3c60a0ff2535f7872b81d35c083a6bfc0a Mon Sep 17 00:00:00 2001 From: akutz Date: Fri, 11 Nov 2016 14:02:58 -0600 Subject: [PATCH 15/28] Isilon Get Volumes Attachment States Fix This patch fixes an issue where the Isilon storage driver always assumed that the local devices information should be available when inspecting a volume or that a volume's attachment information should always be fetched. --- .../storage/isilon/storage/isilon_storage.go | 62 ++++++++++++------- 1 file changed, 40 insertions(+), 22 deletions(-) diff --git a/drivers/storage/isilon/storage/isilon_storage.go b/drivers/storage/isilon/storage/isilon_storage.go index 2baaca7f..a89bf3e8 100644 --- a/drivers/storage/isilon/storage/isilon_storage.go +++ b/drivers/storage/isilon/storage/isilon_storage.go @@ -167,23 +167,37 @@ func (d *driver) NextDeviceInfo( return nil, nil } -func (d *driver) getVolumeAttachments(ctx types.Context) ( +func (d *driver) getVolumeAttachments( + ctx types.Context, + attachments types.VolumeAttachmentsTypes) ( []*types.VolumeAttachment, error) { + if !attachments.Requested() { + return nil, nil + } + exports, err := d.getVolumeExports(ctx) if err != nil { return nil, err } iid, iidOK := context.InstanceID(ctx) - ld, ldOK := context.LocalDevices(ctx) + var ( + ld *types.LocalDevices + ldOK bool + ) + if attachments.Devices() { + ld, ldOK = context.LocalDevices(ctx) + } var atts []*types.VolumeAttachment for _, export := range exports { - var dev string - var status string + var ( + dev string + status string + ) for _, c := range export.Clients { - if iidOK && ldOK && c == iid.ID { + if iidOK && ldOK && strings.EqualFold(c, iid.ID) { dev = d.nfsMountPath(export.ExportPath) if _, ok := ld.DeviceMap[dev]; ok { status = "Exported and Mounted" @@ -542,21 +556,23 @@ func (d *driver) getVolume( return nil, nil } - var atts []*types.VolumeAttachment + var ( + err error + atts []*types.VolumeAttachment + attMap map[string][]*types.VolumeAttachment + ) + if attachments.Requested() { - var err error - atts, err = d.getVolumeAttachments(ctx) - if err != nil { + if atts, err = d.getVolumeAttachments(ctx, attachments); err != nil { return nil, err } - } - - attMap := make(map[string][]*types.VolumeAttachment) - for _, att := range atts { - if attMap[att.VolumeID] == nil { - attMap[att.VolumeID] = make([]*types.VolumeAttachment, 0) + attMap = map[string][]*types.VolumeAttachment{} + for _, att := range atts { + if attMap[att.VolumeID] == nil { + attMap[att.VolumeID] = []*types.VolumeAttachment{} + } + attMap[att.VolumeID] = append(attMap[att.VolumeID], att) } - attMap[att.VolumeID] = append(attMap[att.VolumeID], att) } var volumesSD []*types.Volume @@ -565,13 +581,15 @@ func (d *driver) getVolume( if err != nil { return nil, err } - - vatts, _ := attMap[volume.Name] volumeSD := &types.Volume{ - Name: volume.Name, - ID: volume.Name, - Size: volSize, - Attachments: vatts, + Name: volume.Name, + ID: volume.Name, + Size: volSize, + } + if attachments.Requested() { + if vatts, ok := attMap[volume.Name]; ok { + volumeSD.Attachments = vatts + } } volumesSD = append(volumesSD, volumeSD) } From 6c610b5305adc01fe8421b99974c571948529239 Mon Sep 17 00:00:00 2001 From: akutz Date: Fri, 11 Nov 2016 14:02:58 -0600 Subject: [PATCH 16/28] ScaleIO Get Volumes Attachment States Fix This patch fixes an issue where the ScaleIO storage driver always assumed that the local devices information should be available when inspecting a volume or that a volume's attachment information should always be fetched. --- .../scaleio/storage/scaleio_storage.go | 36 +++++++++---------- 1 file changed, 18 insertions(+), 18 deletions(-) diff --git a/drivers/storage/scaleio/storage/scaleio_storage.go b/drivers/storage/scaleio/storage/scaleio_storage.go index 21f08f7b..32c9fd05 100644 --- a/drivers/storage/scaleio/storage/scaleio_storage.go +++ b/drivers/storage/scaleio/storage/scaleio_storage.go @@ -164,7 +164,7 @@ func (d *driver) Volumes( opts *types.VolumesOpts) ([]*types.Volume, error) { sdcMappedVolumes := make(map[string]string) - if opts.Attachments.Requested() { + if opts.Attachments.Devices() { if ld, ok := context.LocalDevices(ctx); ok { sdcMappedVolumes = ld.DeviceMap } @@ -195,8 +195,8 @@ func (d *driver) Volumes( return "" } - if protectionDomain, ok := mapProtectionDomainName[pool.ProtectionDomainID]; ok { - return protectionDomain.Name + if pd, ok := mapProtectionDomainName[pool.ProtectionDomainID]; ok { + return pd.Name } return "" } @@ -209,22 +209,22 @@ func (d *driver) Volumes( var volumesSD []*types.Volume for _, volume := range volumes { var attachmentsSD []*types.VolumeAttachment - for _, attachment := range volume.MappedSdcInfo { - var deviceName string - if _, exists := sdcMappedVolumes[volume.ID]; exists { - deviceName = sdcMappedVolumes[volume.ID] - } - instanceID := &types.InstanceID{ - ID: attachment.SdcID, - Driver: d.Name(), + if opts.Attachments.Requested() { + for _, attachment := range volume.MappedSdcInfo { + instanceID := &types.InstanceID{ + ID: attachment.SdcID, + Driver: d.Name(), + } + attachmentSD := &types.VolumeAttachment{ + VolumeID: volume.ID, + InstanceID: instanceID, + Status: "", + } + if devName, ok := sdcMappedVolumes[volume.ID]; ok { + attachmentSD.DeviceName = devName + } + attachmentsSD = append(attachmentsSD, attachmentSD) } - attachmentSD := &types.VolumeAttachment{ - VolumeID: volume.ID, - InstanceID: instanceID, - DeviceName: deviceName, - Status: "", - } - attachmentsSD = append(attachmentsSD, attachmentSD) } var IOPS int64 From 5428d7a1aef45bc2ea3dc2256b2b4932b92cb155 Mon Sep 17 00:00:00 2001 From: akutz Date: Sat, 12 Nov 2016 14:16:04 -0600 Subject: [PATCH 17/28] Rackspace Get Volumes Attachment States Fix This patch fixes an issue where the Rackspace storage driver always assumed that the local devices information should be available when inspecting a volume or that a volume's attachment information should always be fetched. --- .../rackspace/storage/rackspace_storage.go | 34 +++++++------------ 1 file changed, 12 insertions(+), 22 deletions(-) diff --git a/drivers/storage/rackspace/storage/rackspace_storage.go b/drivers/storage/rackspace/storage/rackspace_storage.go index cb4154cc..1c56294f 100644 --- a/drivers/storage/rackspace/storage/rackspace_storage.go +++ b/drivers/storage/rackspace/storage/rackspace_storage.go @@ -138,7 +138,7 @@ func (d *driver) Volumes( ctx types.Context, opts *types.VolumesOpts) ([]*types.Volume, error) { // always return attachments to align against other drivers for now - return d.getVolume(ctx, "", "", types.VolAttReqTrue) + return d.getVolume(ctx, "", "", opts.Attachments) } // // VolumeInspect inspects a single volume. @@ -592,31 +592,21 @@ func (d *driver) createVolume( //Reformats from volumes.Volume to types.Volume credit to github.com/MatMaul func translateVolume( volume *volumes.Volume, - includeAttachments types.VolumeAttachmentsTypes) *types.Volume { + attachments types.VolumeAttachmentsTypes) *types.Volume { - var attachments []*types.VolumeAttachment - if includeAttachments.Requested() { - for _, attachment := range volume.Attachments { - libstorageAttachment := &types.VolumeAttachment{ - VolumeID: attachment["volume_id"].(string), + var atts []*types.VolumeAttachment + if attachments.Requested() { + for _, att := range volume.Attachments { + lsAtt := &types.VolumeAttachment{ + VolumeID: att["volume_id"].(string), InstanceID: &types.InstanceID{ - ID: attachment["server_id"].(string), + ID: att["server_id"].(string), Driver: rackspace.Name}, - DeviceName: attachment["device"].(string), - Status: "", } - attachments = append(attachments, libstorageAttachment) - } - } else { - for _, attachment := range volume.Attachments { - libstorageAttachment := &types.VolumeAttachment{ - VolumeID: attachment["volume_id"].(string), - InstanceID: &types.InstanceID{ID: attachment["server_id"].(string), Driver: rackspace.Name}, - DeviceName: "", - Status: "", + if attachments.Devices() { + lsAtt.DeviceName = att["device"].(string) } - attachments = append(attachments, libstorageAttachment) - break + atts = append(atts, lsAtt) } } @@ -628,7 +618,7 @@ func translateVolume( Type: volume.VolumeType, IOPS: 0, Size: int64(volume.Size), - Attachments: attachments, + Attachments: atts, } } From 9dd1b3711f8d322eb448e766964d42a0dd350d96 Mon Sep 17 00:00:00 2001 From: akutz Date: Sat, 12 Nov 2016 14:59:52 -0600 Subject: [PATCH 18/28] Release Candidate 0.3.3-rc5 This patch marks release candidate 0.3.3-rc5. --- .docs/about/release-notes.md | 3 ++- VERSION | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/.docs/about/release-notes.md b/.docs/about/release-notes.md index 721e2b93..679af41b 100644 --- a/.docs/about/release-notes.md +++ b/.docs/about/release-notes.md @@ -8,7 +8,8 @@ This release includes some minor fixes as well as a new way to query attachment information about one or more volumes. ### Enhancements -* Enhanced attachment querying ([#313](https://github.com/codedellemc/libstorage/pull/313), [#316](https://github.com/codedellemc/libstorage/pull/316), [#319](https://github.com/codedellemc/libstorage/pull/319)) +* Enhanced attachment querying ([#313](https://github.com/codedellemc/libstorage/pull/313), [#316](https://github.com/codedellemc/libstorage/pull/316), [#319](https://github.com/codedellemc/libstorage/pull/319), [#330](https://github.com/codedellemc/libstorage/pull/330), [#331](https://github.com/codedellemc/libstorage/pull/331), [#332](https://github.com/codedellemc/libstorage/pull/332), [#334](https://github.com/codedellemc/libstorage/pull/334), +[#335](https://github.com/codedellemc/libstorage/pull/335), [#336](https://github.com/codedellemc/libstorage/pull/336)) ### Bug Fixes * AWS Config Support ([#314](https://github.com/codedellemc/libstorage/pull/314)) diff --git a/VERSION b/VERSION index fad33df1..acbf45c8 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -0.3.3-rc4 +0.3.3-rc5 From 0178d26c1a8b1434109bbad2a0b645592071e8a3 Mon Sep 17 00:00:00 2001 From: Travis Rhoden Date: Mon, 14 Nov 2016 14:19:36 -0700 Subject: [PATCH 19/28] Only set Vol AttachmentState if not set by driver The framework can automatically set a volume's attachment state, but a driver can also explicitly set it to "unknown" -- something that only a driver can determine. That driver-set value was always being overriden by the framework. This patch makes it so that the framework only determines attachment state if a driver has not already set a value. --- api/server/router/volume/volume_routes.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/api/server/router/volume/volume_routes.go b/api/server/router/volume/volume_routes.go index faea4776..a85467cd 100644 --- a/api/server/router/volume/volume_routes.go +++ b/api/server/router/volume/volume_routes.go @@ -185,7 +185,11 @@ func handleVolAttachments( return false } - // determine a volume's attachment state + // determine a volume's attachment state if it hasn't been set already + if vol.AttachmentState > 0 { + return true + } + if len(vol.Attachments) == 0 { vol.AttachmentState = types.VolumeAvailable } else if iid != nil { From 80750496e644f8bbd32786439a3947d89d179adb Mon Sep 17 00:00:00 2001 From: akutz Date: Mon, 14 Nov 2016 16:47:57 -0600 Subject: [PATCH 20/28] Fix for Nil Exception w Attachments This patch fixes some logic to do with the Docker integration driver and attachments as well as a nil exception when responding to a Detach request and attachment queries. --- api/server/router/volume/volume_routes.go | 18 +++++++++++++++--- drivers/integration/docker/docker.go | 7 +++++-- drivers/integration/docker/docker_utils.go | 18 ++++++------------ 3 files changed, 26 insertions(+), 17 deletions(-) diff --git a/api/server/router/volume/volume_routes.go b/api/server/router/volume/volume_routes.go index a85467cd..a22eca9a 100644 --- a/api/server/router/volume/volume_routes.go +++ b/api/server/router/volume/volume_routes.go @@ -587,7 +587,11 @@ func (r *router) volumeDetach( return nil, err } - if v != nil && OnVolume != nil { + if v == nil { + return nil, nil + } + + if OnVolume != nil { ok, err := OnVolume(ctx, req, store, v) if err != nil { return nil, err @@ -677,7 +681,11 @@ func (r *router) volumeDetachAll( return nil, err } - if v != nil && OnVolume != nil { + if v == nil { + continue + } + + if OnVolume != nil { ok, err := OnVolume(ctx, req, store, v) if err != nil { return nil, err @@ -761,7 +769,11 @@ func (r *router) volumeDetachAllForService( return nil, err } - if v != nil && OnVolume != nil { + if v == nil { + continue + } + + if OnVolume != nil { ok, err := OnVolume(ctx, req, store, v) if err != nil { return nil, err diff --git a/drivers/integration/docker/docker.go b/drivers/integration/docker/docker.go index 37978144..13cec364 100644 --- a/drivers/integration/docker/docker.go +++ b/drivers/integration/docker/docker.go @@ -169,7 +169,9 @@ func (d *driver) Mount( "opts": opts}).Info("mounting volume") vol, err := d.volumeInspectByIDOrName( - ctx, volumeID, volumeName, types.VolAttReqTrue, opts.Opts) + ctx, volumeID, volumeName, + types.VolAttReqWithDevMapOnlyVolsAttachedToInstanceOrUnattachedVols, + opts.Opts) if isErrNotFound(err) && d.volumeCreateImplicit() { var err error if vol, err = d.Create(ctx, volumeName, &types.VolumeCreateOpts{ @@ -323,7 +325,8 @@ func (d *driver) Unmount( } vol, err := d.volumeInspectByIDOrName( - ctx, volumeID, volumeName, types.VolAttReqTrue, opts) + ctx, volumeID, volumeName, + types.VolAttReqOnlyVolsAttachedToInstance, opts) if err != nil { return err } diff --git a/drivers/integration/docker/docker_utils.go b/drivers/integration/docker/docker_utils.go index b497e8d2..d86437a0 100644 --- a/drivers/integration/docker/docker_utils.go +++ b/drivers/integration/docker/docker_utils.go @@ -49,27 +49,21 @@ func (d *driver) volumeInspectByIDOrName( var obj *types.Volume if volumeID != "" { var err error - obj, err = d.volumeInspectByID( - ctx, volumeID, types.VolAttReqTrue, opts) + obj, err = d.volumeInspectByID(ctx, volumeID, attachments, opts) if err != nil { return nil, err } } else { - objs, err := client.Storage().Volumes(ctx, &types.VolumesOpts{ - Attachments: 0}) + objs, err := client.Storage().Volumes( + ctx, &types.VolumesOpts{Attachments: 0}) if err != nil { return nil, err } for _, o := range objs { if strings.EqualFold(volumeName, o.Name) { - if attachments.Requested() { - obj, err = d.volumeInspectByID( - ctx, o.ID, types.VolAttReqTrue, opts) - if err != nil { - return nil, err - } - } else { - obj = o + obj, err = d.volumeInspectByID(ctx, o.ID, attachments, opts) + if err != nil { + return nil, err } break } From 6fc5cd28bd2e1cffc6a9dec9822550120e475fc5 Mon Sep 17 00:00:00 2001 From: akutz Date: Wed, 16 Nov 2016 13:43:14 -0600 Subject: [PATCH 21/28] Update Handling of Vols w Unknown Att State This patch updates the way volumes with an unknown attachment state are handled. If a driver sets a volume's attachment state to unknown and only available or unavailable volumes are requested, a volume with an unknown attachment state will not be returned. --- .gitignore | 1 + api/server/router/volume/volume_routes.go | 74 +++++++++++++--------- api/types/types_model.go | 19 ++++++ drivers/storage/vfs/storage/vfs_storage.go | 6 ++ drivers/storage/vfs/tests/vfs_test.go | 4 +- 5 files changed, 72 insertions(+), 32 deletions(-) diff --git a/.gitignore b/.gitignore index 884f14b4..ea18cd18 100644 --- a/.gitignore +++ b/.gitignore @@ -6,6 +6,7 @@ *.a *.d *.out +got libstorage.paw .site/ site/ diff --git a/api/server/router/volume/volume_routes.go b/api/server/router/volume/volume_routes.go index a85467cd..6e47022c 100644 --- a/api/server/router/volume/volume_routes.go +++ b/api/server/router/volume/volume_routes.go @@ -144,6 +144,38 @@ func handleVolAttachments( lf = log.Fields{} } + f := func(s types.VolumeAttachmentStates) bool { + lf["attachmentState"] = s + // if the volume has no attachments and the mask indicates that + // only attached volumes should be returned then omit this volume + if s == types.VolumeAvailable && + attachments.Attached() && + !attachments.Unattached() { + ctx.WithFields(lf).Debug("omitting unattached volume") + return false + } + // if the volume has attachments and the mask indicates that + // only unattached volumes should be returned then omit this volume + if (s == types.VolumeAttached || s == types.VolumeUnavailable) && + !attachments.Attached() && + attachments.Unattached() { + ctx.WithFields(lf).Debug("omitting attached volume") + return false + } + ctx.WithFields(lf).Debug("including volume") + return true + } + + // if the attachment state has already been set by the driver then + // use it to determine whether the volume should be omitted + if vol.AttachmentState > 0 { + ctx.WithFields(lf).Debug( + "deferring to driver-specified attachment state") + return f(vol.AttachmentState) + } + + ctx.WithFields(lf).Debug("manually calculating attachment state") + // if only the requesting instance's attachments are requested then // filter the volume's attachments list if attachments.Mine() { @@ -167,43 +199,25 @@ func handleVolAttachments( ctx.WithFields(lf).Debug("included volume attached to instance") } - // if the volume has no attachments and the mask indicates that - // only attached volumes should be returned then omit this volume - if len(vol.Attachments) == 0 && - attachments.Attached() && - !attachments.Unattached() { - ctx.WithFields(lf).Debug("omitting unattached volume") - return false - } - - // if the volume has attachments and the mask indicates that - // only unattached volumes should be returned then omit this volume - if len(vol.Attachments) > 0 && - !attachments.Attached() && - attachments.Unattached() { - ctx.WithFields(lf).Debug("omitting attached volume") - return false - } - - // determine a volume's attachment state if it hasn't been set already - if vol.AttachmentState > 0 { - return true - } - + // determine a volume's attachment state if len(vol.Attachments) == 0 { vol.AttachmentState = types.VolumeAvailable - } else if iid != nil { + } else { vol.AttachmentState = types.VolumeUnavailable - for _, a := range vol.Attachments { - if a.InstanceID != nil && - strings.EqualFold(iid.ID, a.InstanceID.ID) { - vol.AttachmentState = types.VolumeAttached - break + if iid != nil { + for _, a := range vol.Attachments { + if a.InstanceID != nil && + strings.EqualFold(iid.ID, a.InstanceID.ID) { + vol.AttachmentState = types.VolumeAttached + break + } } } } - return true + // use the ascertained attachment state to determine whether or not the + // volume should be omitted + return f(vol.AttachmentState) } func getFilteredVolumes( diff --git a/api/types/types_model.go b/api/types/types_model.go index a2ffc598..dca3bb08 100644 --- a/api/types/types_model.go +++ b/api/types/types_model.go @@ -1,5 +1,7 @@ package types +import "strconv" + // StorageType is the type of storage a driver provides. type StorageType string @@ -146,6 +148,23 @@ const ( VolumeUnavailable VolumeAttachmentStates = 4 ) +// String returns the string represntation of a VolumeAttachmentStates value. +func (s VolumeAttachmentStates) String() string { + switch s { + case 0: + return "0" + case VolumeAttachmentStateUnknown: + return "unknown" + case VolumeAttached: + return "attached" + case VolumeAvailable: + return "available" + case VolumeUnavailable: + return "unavailable" + } + return strconv.Itoa(int(s)) +} + // Volume provides information about a storage volume. type Volume struct { // Attachments is information about the instances to which the volume diff --git a/drivers/storage/vfs/storage/vfs_storage.go b/drivers/storage/vfs/storage/vfs_storage.go index 513034eb..55417b00 100644 --- a/drivers/storage/vfs/storage/vfs_storage.go +++ b/drivers/storage/vfs/storage/vfs_storage.go @@ -150,6 +150,9 @@ func (d *driver) Volumes( if err != nil { return nil, err } + if opts.Attachments > 0 { + v.AttachmentState = 0 + } volumes = append(volumes, v) } @@ -167,6 +170,9 @@ func (d *driver) VolumeInspect( if err != nil { return nil, err } + if opts.Attachments > 0 { + v.AttachmentState = 0 + } return v, nil } diff --git a/drivers/storage/vfs/tests/vfs_test.go b/drivers/storage/vfs/tests/vfs_test.go index a304a17d..43cab2c2 100644 --- a/drivers/storage/vfs/tests/vfs_test.go +++ b/drivers/storage/vfs/tests/vfs_test.go @@ -997,7 +997,7 @@ const volJSON = `{ }, "status": "attached" }], - "attachmentState": 2, + "attachmentState": 2, "fields": { "owner": "root@example.com", "priority": "2" @@ -1011,7 +1011,7 @@ const volNoAttachJSON = `{ "size": 10240, "id": "vfs-%03[1]d", "type": "myType", - "attachmentState": 3, + "attachmentState": 3, "fields": { "owner": "root@example.com", "priority": "2" From 2980d9286bed8c361f248441ca5d15b4226318c6 Mon Sep 17 00:00:00 2001 From: akutz Date: Wed, 16 Nov 2016 16:39:29 -0600 Subject: [PATCH 22/28] Added Example for EC2 Vagrantfile This patch adds an example for bringing a libStorage environment online with REX-Ray using EC2, EBS, and EFS. --- .gitignore | 4 +- examples/ec2/vagrant/README.md | 100 ++++++ examples/ec2/vagrant/Vagrantfile | 590 +++++++++++++++++++++++++++++++ examples/ec2/vagrant/client0.sh | 23 ++ examples/ec2/vagrant/client1.sh | 15 + examples/ec2/vagrant/server.sh | 4 + 6 files changed, 735 insertions(+), 1 deletion(-) create mode 100644 examples/ec2/vagrant/README.md create mode 100644 examples/ec2/vagrant/Vagrantfile create mode 100644 examples/ec2/vagrant/client0.sh create mode 100644 examples/ec2/vagrant/client1.sh create mode 100644 examples/ec2/vagrant/server.sh diff --git a/.gitignore b/.gitignore index ea18cd18..db922810 100644 --- a/.gitignore +++ b/.gitignore @@ -23,7 +23,9 @@ cli/semaphores/signal cli/semaphores/unlink api/api_generated.go api/server/executors/executors_generated.go -drivers/storage/ebs/tests/.vagrant +drivers/storage/ebs/tests/.vagrant/ +examples/ec2/vagrant/.vagrant/ + # Created by https://www.gitignore.io diff --git a/examples/ec2/vagrant/README.md b/examples/ec2/vagrant/README.md new file mode 100644 index 00000000..e8741acd --- /dev/null +++ b/examples/ec2/vagrant/README.md @@ -0,0 +1,100 @@ +# EC2 Vagrantfile +This directory includes a Vagrantfile that can be used to bring a libStorage +server/client installation online using REX-Ray and EC2 instances. For example: + +``` +vagrant up --provider=aws --no-parallel +``` + +The following sections outline dependencies, settings, and different execution +scenarios that may be required or useful for using the Vagrantfile. + +### Dependencies +The following dependencies are required in order to bring the EC2 Vagrant +enviornment online: + + * [Vagrant](https://www.vagrantup.com/) 1.8.5+ + * [vagrant-aws](https://github.com/mitchellh/vagrant-aws) + * [vagrant-hostmanager](https://github.com/devopsgroup-io/vagrant-hostmanager) + +Once Vagrant is installed the required plug-ins may be installed with the +following commands: + +```bash +vagrant plugin install vagrant-aws +vagrant plugin install vagrant-hostmanager +``` + +### Settings +The following environment variables may be used to configure the `Vagrantfile`. + +Environment Variable | Description | Required | Default +---------------------|-------------|:--------:|-------- +`AWS_AKEY` | The AWS access key ID. | ✓ | +`AWS_SKEY` | The AWS secret key. | ✓ | +`AWS_KPNM` | The name of the AWS key pair for instances. | ✓ | +`AWS_AMI` | The AMI to use. Defaults to Amazon Linux PV x64 ([AMIs by region](https://aws.amazon.com/amazon-linux-ami/)) | | ami-de347abe +`AWS_REGN` | The AWS region. | | us-west-1 +`AWS_ZONE` | The AWS availability zone. | | a +`AWS_SSHK` | Local SSH key used to access AWS instances. | ✓ | + +### Nodes +The `Vagrantfile` which deploys a libStorage server and two clients onto EC2 +instances named: + + * libstorage-server + * libstorage-client0 + * libstorage-client1 + +### Test Plan Boot Order +The order in which Vagrant brings the nodes online can vary: + +```bash +vagrant up --provider=aws +``` + +The above command will bring all three nodes -- the server and both clients -- +online in parallel since the Vagrant AWS plug-in is configured to support +the parallel option. However, the Vagrantfile will fail if the server +is not brought online first. That is why when bringing the Vagrant nodes online +all at once, the following command should be used: + +```bash +vagrant up --provider=aws --no-parallel +``` + +The above command will bring the nodes online in the following order: + + 1. libstorage-server + 2. libstorage-client0 + 3. libstorage-client1 + +However, another option for starting the environment is to bring up the server +first and then the clients in parallel. Doing so can duplicate a scenario +where two clients are contending for storage resources: + +```bash +vagrant up --provider=aws libstorage-server +vagrant up --provider=aws '/.*client.*/' +``` + +### Scripts +This package includes several test scripts that act as simple ways to execute +random pieces of logic for a node after it is brought online: + + * `server.sh` + * `client0.sh` + * `client1.sh` + +The above files are copied to their respective instances and executed +as soon as the instance is online. That means the `server-tests.sh` script is +executed before the client nodes are even online since the server node is +brought online first. + +### Cleanup +It's important to remember to clean up any EC2, EBS, & EFS resources that may +have been created along the way. To do so simply execute the following command: + +```bash +vagrant destroy -f +``` diff --git a/examples/ec2/vagrant/Vagrantfile b/examples/ec2/vagrant/Vagrantfile new file mode 100644 index 00000000..d3d88d31 --- /dev/null +++ b/examples/ec2/vagrant/Vagrantfile @@ -0,0 +1,590 @@ +# -*- mode: ruby -*- +# vi: set ft=ruby : +require 'fileutils' +require 'shellwords' +require 'fog' + +# general config +$autostart_clients = true # set to false to prevent clients from auto-starting + +# aws config +$aws_akey = ENV['AWS_AKEY'] +$aws_skey = ENV['AWS_SKEY'] +$aws_kpnm = ENV['AWS_KPNM'] +$aws_ami = ENV['AWS_AMI'] ? ENV['AWS_AMI'] : "ami-ef2af28f" +$aws_regn = ENV['AWS_REGN'] ? ENV['AWS_REGN'] : "us-west-2" +$aws_zone = ENV['AWS_ZONE'] ? ENV['AWS_ZONE'] : "a" + +# ssh config +$ssh_user = "centos" +$ssh_pkey = ENV['AWS_SSHK'] # path to SSH private key for AWS + +# node info +$node0_name = "libstorage-server" +$node0_itype = "m4.large" +$node0_texps = "server.sh" + +$node1_name = "libstorage-client0" +$node1_itype = "m4.large" +$node1_texps = "client0.sh" + +$node2_name = "libstorage-client1" +$node2_itype = "t2.micro" +$node2_texps = "client1.sh" + +# Golang information +$goos = "linux" +$goarch = "amd64" +$gover = "1.7.1" +$gotgz = "go#{$gover}.#{$goos}-#{$goarch}.tar.gz" +$gourl = "https://storage.googleapis.com/golang/#{$gotgz}" +$gopath = "/opt/go" + +# project info +$srcs = "#{$gopath}/src/github.com/codedellemc/libstorage" + +# the script to ensure the copied working source is a git repo +$validate_copied_sources = <