Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Volume Attachment States #319

Merged
merged 1 commit into from
Nov 4, 2016
Merged

Volume Attachment States #319

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 22 additions & 0 deletions api/server/router/volume/volume_routes.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -395,6 +395,9 @@ func (r *router) volumeCreate(
}
}

if v.AttachmentState == 0 {
v.AttachmentState = types.VolumeAvailable
}
return v, nil
}

Expand Down Expand Up @@ -439,6 +442,9 @@ func (r *router) volumeCopy(
}
}

if v.AttachmentState == 0 {
v.AttachmentState = types.VolumeAvailable
}
return v, nil
}

Expand Down Expand Up @@ -517,6 +523,10 @@ func (r *router) volumeAttach(
}
}

if v.AttachmentState == 0 {
v.AttachmentState = types.VolumeAttached
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why is this one 0 -> attached and all the others 0 -> available? My guess is that this is basically setting the default value, and if a call to Attach() is returning without an error, it can be assumed that the volume is attached. makes sense, but want to double check.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You inferred the reasoning correctly.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Although now that you point this out, I need to update the above to the following logic:

if v.AttachmentState == 0 && len(v.Attachments) > 0 {
    if "attached to the same instance as the instance ID in the context" {
        v.AttachmentState = types.VolumeAttached
    } else {
        v.AttachmentState = types.VolumeUnavailable
    }
}

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wait, no I won't. The API call asserts the instance ID must be included to do the attach op, which means when the volume returns the volume WILL be attached (if successful) to whichever instance was specified in the call.

return &types.VolumeAttachResponse{
Volume: v,
AttachToken: attTokn,
Expand Down Expand Up @@ -569,6 +579,10 @@ func (r *router) volumeDetach(
}
}

if v.AttachmentState == 0 {
v.AttachmentState = types.VolumeAvailable
}

return v, nil
}

Expand Down Expand Up @@ -655,6 +669,10 @@ func (r *router) volumeDetachAll(
}
}

if v.AttachmentState == 0 {
v.AttachmentState = types.VolumeAvailable
}

volumeMap[v.ID] = v
}

Expand Down Expand Up @@ -735,6 +753,10 @@ func (r *router) volumeDetachAllForService(
}
}

if v.AttachmentState == 0 {
v.AttachmentState = types.VolumeAvailable
}

reply[v.ID] = v
}

Expand Down
44 changes: 37 additions & 7 deletions api/types/types_model.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"`
Expand All @@ -139,28 +169,28 @@ 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"`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is YAML omitempty, but not JSON? Seems like the only outlier?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, because I got Copy/Paste happy with the YAML update. Doesn't matter ultimately because the YAML tags were originally there to influence the CLI output which is no longer based on YAML.


// 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
// guaranteed to be unique across multiple, configured services.
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.
Expand Down
6 changes: 6 additions & 0 deletions api/utils/schema/schema_generated.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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."
Expand Down
2 changes: 2 additions & 0 deletions drivers/storage/vfs/tests/vfs_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1006,6 +1006,7 @@ const volJSON = `{
},
"status": "attached"
}],
"attachmentState": 2,
"fields": {
"owner": "[email protected]",
"priority": "2"
Expand All @@ -1019,6 +1020,7 @@ const volNoAttachJSON = `{
"size": 10240,
"id": "vfs-%03[1]d",
"type": "myType",
"attachmentState": 3,
"fields": {
"owner": "[email protected]",
"priority": "2"
Expand Down
6 changes: 6 additions & 0 deletions libstorage.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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."
Expand Down