Feature #11644

[keepstore] mount-oriented APIs

Added by Tom Clegg 19 days ago. Updated 10 days ago.

Status:ResolvedStart date:05/09/2017
Priority:NormalDue date:
Assignee:Tom Clegg% Done:

100%

Category:Keep
Target version:2017-05-24 sprint
Story points3.0Remaining (hours)0.00 hour
Velocity based estimate-

Description

Keepstore's role in the management part of the Keep storage tiers story.
  • A "mounts" request (GET /mounts) should return information about all currently mounted volumes (see below; "DeviceID" is a string that can enable keep-balance to detect when multiple Keep mounts, possibly on multiple keepstore nodes, are using the same underlying storage device).
  • A block-index request for a specific mount (GET /mounts/zzzzz-aaaaa-aaaabbbbccccddd/blocks) should return a list of blocks stored on that mount. A prefix can be provided as a query parameter (.../blocks?prefix=aaa).
  • An entry in a pull request may include a "MountUUID" field indicating which mount the new copy should be written to.
  • An entry in a trash request may include a "MountUUID" field indicating which mount the block should be deleted from.

The new "/mounts" API is unrestricted. The other three APIs here, including the list of blocks for a specified mount, are only available to system processes (i.e., require "data manager" token).

Example /mounts response:

[
 {
  "UUID":"zzzzz-aaaaa-aaaabbbbccccddd",
  "Tier":1,
  "ReadOnly":false,
  "Replication":1,
  "DeviceID":"9febe660-c4e4-4db4-9f59-fbc9d559547c/keep" 
 }
]

Subtasks

Task #11685: Report current mounts using UUIDs assigned at runtimeResolvedTom Clegg

Task #11686: Get filesystem UUIDs from linux APIsResolvedTom Clegg

Task #11687: Obey MountUUID restrictions in trash listResolvedTom Clegg

Task #11661: Review 11644-mounts-apiResolvedTom Clegg

Task #11688: Obey MountUUID restrictions in pull listResolvedTom Clegg

Task #11689: Accept index request for a specific mountResolvedTom Clegg


Related issues

Duplicated by Arvados - Story #7928: [Keep] keepstore identifies underlying volumes to clients Duplicate 12/02/2015

Associated revisions

Revision d121e087
Added by Tom Clegg 10 days ago

Merge branch '11644-mounts-api'

closes #11644

History

#1 Updated by Tom Clegg 19 days ago

  • Description updated (diff)

#2 Updated by Tom Morris 19 days ago

  • Target version set to 2017-05-24 sprint
  • Story points set to 3.0

#3 Updated by Tom Clegg 18 days ago

  • Description updated (diff)

#4 Updated by Tom Clegg 18 days ago

  • Assignee set to Tom Clegg

#5 Updated by Tom Clegg 12 days ago

  • Status changed from New to In Progress

#6 Updated by Lucas Di Pentima 10 days ago

Some questions:

  • File services/keepstore/volume.go
    • Lines 329-333: Can you please explain to me what’s happening there? (just curious, I suppose it's doing some kind of introspection, right? If so, what happens with mnt.DeviceID if DeviceID() isn't there?)
    • Line 322: Why declare a method with an “anonymous receiver”, isn’t the same a declaring it as a normal function?
  • File services/keepstore/volume_unix.go
    • The utility command /sbin/blkid could be used to get the mounted device UUID (it works running it as a normal user, as tested on Debian Jessie)
  • File services/keepstore/handlers.go
    • Lines 65, 66: Those routes could be grouped with the one at Line 55? Should we maintain a consistent URL naming scheme and use /mounts/{uuid}/index/{prefix:…} instead?
    • Couldn’t IndexHandler and MountBlocks be unified? If not, shouldn’t be better to name MountBlocks as MountBlocksHandler to keep naming consistency? (the same with rtr.Mount)

#7 Updated by Tom Clegg 10 days ago

  • Description updated (diff)

Lucas Di Pentima wrote:

  • File services/keepstore/volume.go
    • Lines 329-333: Can you please explain to me what’s happening there? (just curious, I suppose it's doing some kind of introspection, right? If so, what happens with mnt.DeviceID if DeviceID() isn't there?)

It's a type check on an unnamed interface, equivalent to


type DeviceIDer interface {
        DeviceID() string
}

if v, ok := v.(DeviceIDer); ok {
        mnt.DeviceID = v.DeviceID
}

If the volume doesn't support that interface (i.e., doesn't have a DeviceID() func) then mnt.DeviceID will still be "", which (presumably) keep-balance will have to interpret as "no device id available".

...but this actually only made sense while some volume types didn't support DeviceID(). It would be simpler to just make DeviceID() part of the Volume interface. So I've done that.

  • Line 322: Why declare a method with an “anonymous receiver”, isn’t the same a declaring it as a normal function?

It's just a way of keeping it in the VolumeMount namespace. The calling syntax is a bit weird but it's more or less equivalent to a class or static method.

  • File services/keepstore/volume_unix.go
    • The utility command /sbin/blkid could be used to get the mounted device UUID (it works running it as a normal user, as tested on Debian Jessie)

On my debian 8 box, that only works after filling the cache by running blkid as root. Before that, it prints empty results.

The blkid man page says to use lsblk instead, which always prints empty UUIDs unless running as root.

(Aside: I was tempted to parse /proc/self/mountinfo instead of invoking findmnt -- that way we wouldn't have to fork any processes at all -- but that seems like premature optimization at this point.)

  • File services/keepstore/handlers.go
    • Lines 65, 66: Those routes could be grouped with the one at Line 55? Should we maintain a consistent URL naming scheme and use /mounts/{uuid}/index/{prefix:…} instead?

I went with "blocks" because "blocks/" is a web convention for getting an index of blocks, while "index" doesn't say what it's getting an index of.

But on the subject of web conventions... /mounts/{uuid}/blocks/?prefix={prefix} would probably be even better. That way /mounts/{uuid}/blocks/{locator} can be reserved for a more obvious purpose, i.e., fetch {locator}'s data from the specified mount.

  • Couldn’t IndexHandler and MountBlocks be unified? If not, shouldn’t be better to name MountBlocks as MountBlocksHandler to keep naming consistency? (the same with rtr.Mount)

Yes to both. Done.

I was thinking that it would be better to move in the opposite direction and call the handlers Index and Mounts and Get, but now that you mention it, it does seem better to stay consistent until we change them all at once.

11644-mounts-api @ 370036888e75b509ebf84ba8337273f7b9146f37

#8 Updated by Lucas Di Pentima 10 days ago

Thanks for the explanations!
Ran services/keepstore tests locally, all good.
LGTM.

#9 Updated by Tom Clegg 10 days ago

  • Status changed from In Progress to Resolved
  • % Done changed from 83 to 100

Applied in changeset arvados|commit:d121e087ad1b4e91f869dbd57534c6d6ce51d19d.

Also available in: Atom PDF