Skip to content

Commit

Permalink
csi/api: populate ReadAllocs/WriteAllocs fields (#9377)
Browse files Browse the repository at this point in the history
The API is missing values for `ReadAllocs` and `WriteAllocs` fields, resulting
in allocation claims not being populated in the web UI. These fields mirror
the fields in `nomad/structs.CSIVolume`. Returning a separate list of stubs
for read and write would be ideal, but this can't be done without either
bloating the API response with repeated full `Allocation` data, or causing a
panic in previous versions of the CLI.

The `nomad/structs` fields are persisted with nil values and are populated
during RPC, so we'll do the same in the HTTP API and populate the `ReadAllocs`
and `WriteAllocs` fields with a map of allocation IDs, but with null
values. The web UI will then create its `ReadAllocations` and
`WriteAllocations` fields by mapping from those IDs to the values in
`Allocations`, instead of flattening the map into a list.
  • Loading branch information
tgross authored Nov 25, 2020
1 parent bda5049 commit 8351c3f
Show file tree
Hide file tree
Showing 10 changed files with 80 additions and 62 deletions.
30 changes: 9 additions & 21 deletions api/csi.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,9 +41,6 @@ func (v *CSIVolumes) Info(id string, q *QueryOptions) (*CSIVolume, *QueryMeta, e
return nil, nil, err
}

// Cleanup allocation representation for the ui
resp.allocs()

return &resp, qm, nil
}

Expand Down Expand Up @@ -110,11 +107,17 @@ type CSIVolume struct {
Parameters map[string]string `hcl:"parameters"`
Context map[string]string `hcl:"context"`

// Allocations, tracking claim status
ReadAllocs map[string]*Allocation
// ReadAllocs is a map of allocation IDs for tracking reader claim status.
// The Allocation value will always be nil; clients can populate this data
// by iterating over the Allocations field.
ReadAllocs map[string]*Allocation

// WriteAllocs is a map of allocation IDs for tracking writer claim
// status. The Allocation value will always be nil; clients can populate
// this data by iterating over the Allocations field.
WriteAllocs map[string]*Allocation

// Combine structs.{Read,Write}Allocs
// Allocations is a combined list of readers and writers
Allocations []*AllocationListStub

// Schedulable is true if all the denormalized plugin health fields are true
Expand All @@ -136,21 +139,6 @@ type CSIVolume struct {
ExtraKeysHCL []string `hcl1:",unusedKeys" json:"-"`
}

// allocs is called after we query the volume (creating this CSIVolume struct) to collapse
// allocations for the UI
func (v *CSIVolume) allocs() {
for _, a := range v.WriteAllocs {
if a != nil {
v.Allocations = append(v.Allocations, a.Stub())
}
}
for _, a := range v.ReadAllocs {
if a != nil {
v.Allocations = append(v.Allocations, a.Stub())
}
}
}

type CSIVolumeIndexSort []*CSIVolumeListStub

func (v CSIVolumeIndexSort) Len() int {
Expand Down
25 changes: 21 additions & 4 deletions command/agent/csi_endpoint.go
Original file line number Diff line number Diff line change
Expand Up @@ -310,7 +310,7 @@ func structsCSIVolumeToApi(vol *structs.CSIVolume) *api.CSIVolume {
return nil
}

allocs := len(vol.WriteAllocs) + len(vol.ReadAllocs)
allocCount := len(vol.ReadAllocs) + len(vol.WriteAllocs)

out := &api.CSIVolume{
ID: vol.ID,
Expand All @@ -326,7 +326,11 @@ func structsCSIVolumeToApi(vol *structs.CSIVolume) *api.CSIVolume {
Context: vol.Context,

// Allocations is the collapsed list of both read and write allocs
Allocations: make([]*api.AllocationListStub, 0, allocs),
Allocations: make([]*api.AllocationListStub, 0, allocCount),

// tracking of volume claims
ReadAllocs: map[string]*api.Allocation{},
WriteAllocs: map[string]*api.Allocation{},

Schedulable: vol.Schedulable,
PluginID: vol.PluginID,
Expand All @@ -342,15 +346,28 @@ func structsCSIVolumeToApi(vol *structs.CSIVolume) *api.CSIVolume {
ModifyIndex: vol.ModifyIndex,
}

// WriteAllocs and ReadAllocs will only ever contain the Allocation ID,
// with a null value for the Allocation; these IDs are mapped to
// allocation stubs in the Allocations field. This indirection is so the
// API can support both the UI and CLI consumer in a safely backwards
// compatible way
for _, a := range vol.WriteAllocs {
if a != nil {
out.Allocations = append(out.Allocations, structsAllocListStubToApi(a.Stub(nil)))
alloc := structsAllocListStubToApi(a.Stub(nil))
if alloc != nil {
out.WriteAllocs[alloc.ID] = nil
out.Allocations = append(out.Allocations, alloc)
}
}
}

for _, a := range vol.ReadAllocs {
if a != nil {
out.Allocations = append(out.Allocations, structsAllocListStubToApi(a.Stub(nil)))
alloc := structsAllocListStubToApi(a.Stub(nil))
if alloc != nil {
out.ReadAllocs[alloc.ID] = nil
out.Allocations = append(out.Allocations, alloc)
}
}
}

Expand Down
26 changes: 17 additions & 9 deletions ui/app/serializers/volume.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,17 +23,25 @@ export default class VolumeSerializer extends ApplicationSerializer {
hash.ID = JSON.stringify([`csi/${hash.ID}`, hash.NamespaceID || 'default']);
hash.PluginID = `csi/${hash.PluginID}`;

// Convert hash-based allocation embeds to lists
// Populate read/write allocation lists from aggregate allocation list
const readAllocs = hash.ReadAllocs || {};
const writeAllocs = hash.WriteAllocs || {};
const bindIDToAlloc = hash => id => {
const alloc = hash[id];
alloc.ID = id;
return alloc;
};

hash.ReadAllocations = Object.keys(readAllocs).map(bindIDToAlloc(readAllocs));
hash.WriteAllocations = Object.keys(writeAllocs).map(bindIDToAlloc(writeAllocs));

hash.ReadAllocations = [];
hash.WriteAllocations = [];

if (hash.Allocations) {
hash.Allocations.forEach(function(alloc) {
const id = alloc.ID;
if (id in readAllocs) {
hash.ReadAllocations.push(alloc);
}
if (id in writeAllocs) {
hash.WriteAllocations.push(alloc);
}
});
delete hash.Allocations;
}

const normalizedHash = super.normalize(typeHash, hash);
return this.extractEmbeddedRecords(this, this.store, typeHash, normalizedHash);
Expand Down
1 change: 1 addition & 0 deletions ui/mirage/models/csi-volume.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,4 +4,5 @@ export default Model.extend({
plugin: belongsTo('csi-plugin'),
writeAllocs: hasMany('allocation'),
readAllocs: hasMany('allocation'),
allocations: hasMany('allocation'),
});
2 changes: 1 addition & 1 deletion ui/mirage/serializers/csi-volume.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ const groupBy = (list, attr) => {

export default ApplicationSerializer.extend({
embed: true,
include: ['writeAllocs', 'readAllocs'],
include: ['writeAllocs', 'readAllocs', 'allocations'],

serialize() {
var json = ApplicationSerializer.prototype.serialize.apply(this, arguments);
Expand Down
2 changes: 2 additions & 0 deletions ui/tests/acceptance/volume-detail-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,13 @@ import VolumeDetail from 'nomad-ui/tests/pages/storage/volumes/detail';

const assignWriteAlloc = (volume, alloc) => {
volume.writeAllocs.add(alloc);
volume.allocations.add(alloc);
volume.save();
};

const assignReadAlloc = (volume, alloc) => {
volume.readAllocs.add(alloc);
volume.allocations.add(alloc);
volume.save();
};

Expand Down
2 changes: 2 additions & 0 deletions ui/tests/acceptance/volumes-list-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,13 @@ import Layout from 'nomad-ui/tests/pages/layout';

const assignWriteAlloc = (volume, alloc) => {
volume.writeAllocs.add(alloc);
volume.allocations.add(alloc);
volume.save();
};

const assignReadAlloc = (volume, alloc) => {
volume.readAllocs.add(alloc);
volume.allocations.add(alloc);
volume.save();
};

Expand Down
20 changes: 14 additions & 6 deletions ui/tests/unit/serializers/volume-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -173,30 +173,38 @@ module('Unit | Serializer | Volume', function(hooks) {
NodesExpected: 2,
CreateIndex: 1,
ModifyIndex: 38,
WriteAllocs: {
'alloc-id-1': {
Allocations: [
{
ID: 'alloc-id-1',
TaskGroup: 'foobar',
CreateTime: +REF_DATE * 1000000,
ModifyTime: +REF_DATE * 1000000,
JobID: 'the-job',
Namespace: 'namespace-2',
},
'alloc-id-2': {
{
ID: 'alloc-id-2',
TaskGroup: 'write-here',
CreateTime: +REF_DATE * 1000000,
ModifyTime: +REF_DATE * 1000000,
JobID: 'the-job',
Namespace: 'namespace-2',
},
},
ReadAllocs: {
'alloc-id-3': {
{
ID: 'alloc-id-3',
TaskGroup: 'look-if-you-must',
CreateTime: +REF_DATE * 1000000,
ModifyTime: +REF_DATE * 1000000,
JobID: 'the-job',
Namespace: 'namespace-2',
},
],
WriteAllocs: {
'alloc-id-1': null,
'alloc-id-2': null,
},
ReadAllocs: {
'alloc-id-3': null,
},
},
out: {
Expand Down
30 changes: 9 additions & 21 deletions vendor/github.com/hashicorp/nomad/api/csi.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 4 additions & 0 deletions website/pages/api-docs/volumes.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -230,6 +230,10 @@ $ curl \
"ModifyTime": 1495747371794276400
}
],
"ReadAllocs": {
"a8198d79-cfdb-6593-a999-1e9adabcba2e": null
},
"WriteAllocs": {},
"Schedulable": true,
"PluginID": "plugin-id1",
"Provider": "ebs",
Expand Down

0 comments on commit 8351c3f

Please sign in to comment.