Skip to content

Commit

Permalink
dynamic host volumes: Enterprise stubs and refactor API
Browse files Browse the repository at this point in the history
Most Nomad upsert RPCs accept a single object with the notable exception of
CSI. But in CSI we don't actually expose this to users except through the Go
API. It deeply complicates how we present errors to users, especially once
Sentinel policy enforcement enters the mix.

Refactor the `HostVolume.Create` and `HostVolume.Register` RPCs to take a single
volume instead of a slice of volumes.

Add a stub function for Enterprise policy enforcement. This requires splitting
out placement from the `createVolume` function so that we can ensure we've
completed placement before trying to enforce policy.

Ref: #24479
  • Loading branch information
tgross committed Nov 22, 2024
1 parent 1b40c07 commit 1f7b72d
Show file tree
Hide file tree
Showing 12 changed files with 364 additions and 315 deletions.
22 changes: 11 additions & 11 deletions api/host_volumes.go
Original file line number Diff line number Diff line change
Expand Up @@ -147,11 +147,11 @@ func (c *Client) HostVolumes() *HostVolumes {
}

type HostVolumeCreateRequest struct {
Volumes []*HostVolume
Volume *HostVolume
}

type HostVolumeRegisterRequest struct {
Volumes []*HostVolume
Volume *HostVolume
}

type HostVolumeListRequest struct {
Expand All @@ -163,30 +163,30 @@ type HostVolumeDeleteRequest struct {
VolumeIDs []string
}

// Create forwards to client agents so host volumes can be created on those
// hosts, and registers the volumes with Nomad servers.
func (hv *HostVolumes) Create(req *HostVolumeCreateRequest, opts *WriteOptions) ([]*HostVolume, *WriteMeta, error) {
// Create forwards to client agents so a host volume can be created on those
// hosts, and registers the volume with Nomad servers.
func (hv *HostVolumes) Create(req *HostVolumeCreateRequest, opts *WriteOptions) (*HostVolume, *WriteMeta, error) {
var out struct {
Volumes []*HostVolume
Volume *HostVolume
}
wm, err := hv.client.put("/v1/volume/host/create", req, &out, opts)
if err != nil {
return nil, wm, err
}
return out.Volumes, wm, nil
return out.Volume, wm, nil
}

// Register registers host volumes that were created out-of-band with the Nomad
// Register registers a host volume that was created out-of-band with the Nomad
// servers.
func (hv *HostVolumes) Register(req *HostVolumeRegisterRequest, opts *WriteOptions) ([]*HostVolume, *WriteMeta, error) {
func (hv *HostVolumes) Register(req *HostVolumeRegisterRequest, opts *WriteOptions) (*HostVolume, *WriteMeta, error) {
var out struct {
Volumes []*HostVolume
Volume *HostVolume
}
wm, err := hv.client.put("/v1/volume/host/register", req, &out, opts)
if err != nil {
return nil, wm, err
}
return out.Volumes, wm, nil
return out.Volume, wm, nil
}

// Get queries for a single host volume, by ID
Expand Down
20 changes: 10 additions & 10 deletions command/agent/host_volume_endpoint_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,8 @@ func TestHostVolumeEndpoint_CRUD(t *testing.T) {
vol.NodePool = ""
vol.Constraints = nil
reqBody := struct {
Volumes []*structs.HostVolume
}{Volumes: []*structs.HostVolume{vol}}
Volume *structs.HostVolume
}{Volume: vol}
buf := encodeReq(reqBody)
req, err := http.NewRequest(http.MethodPut, "/v1/volume/host/create", buf)
must.NoError(t, err)
Expand All @@ -37,12 +37,12 @@ func TestHostVolumeEndpoint_CRUD(t *testing.T) {
must.NoError(t, err)
must.NotNil(t, obj)
resp := obj.(*structs.HostVolumeCreateResponse)
must.Len(t, 1, resp.Volumes)
must.Eq(t, vol.Name, resp.Volumes[0].Name)
must.Eq(t, s.client.NodeID(), resp.Volumes[0].NodeID)
must.NotNil(t, resp.Volume)
must.Eq(t, vol.Name, resp.Volume.Name)
must.Eq(t, s.client.NodeID(), resp.Volume.NodeID)
must.NotEq(t, "", respW.Result().Header.Get("X-Nomad-Index"))

volID := resp.Volumes[0].ID
volID := resp.Volume.ID

// Verify volume was created

Expand All @@ -61,17 +61,17 @@ func TestHostVolumeEndpoint_CRUD(t *testing.T) {
vol = respVol.Copy()
vol.Parameters = map[string]string{"bar": "foo"} // swaps key and value
reqBody = struct {
Volumes []*structs.HostVolume
}{Volumes: []*structs.HostVolume{vol}}
Volume *structs.HostVolume
}{Volume: vol}
buf = encodeReq(reqBody)
req, err = http.NewRequest(http.MethodPut, "/v1/volume/host/register", buf)
must.NoError(t, err)
obj, err = s.Server.HostVolumeSpecificRequest(respW, req)
must.NoError(t, err)
must.NotNil(t, obj)
regResp := obj.(*structs.HostVolumeRegisterResponse)
must.Len(t, 1, regResp.Volumes)
must.Eq(t, map[string]string{"bar": "foo"}, regResp.Volumes[0].Parameters)
must.NotNil(t, regResp.Volume)
must.Eq(t, map[string]string{"bar": "foo"}, regResp.Volume.Parameters)

// Verify volume was updated

Expand Down
26 changes: 11 additions & 15 deletions command/volume_create_host.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,9 @@ func (c *VolumeCreateCommand) hostVolumeCreate(
}

req := &api.HostVolumeCreateRequest{
Volumes: []*api.HostVolume{vol},
Volume: vol,
}
vols, _, err := client.HostVolumes().Create(req, nil)
vol, _, err = client.HostVolumes().Create(req, nil)
if err != nil {
c.Ui.Error(fmt.Sprintf("Error creating volume: %s", err))
return 1
Expand All @@ -39,19 +39,15 @@ func (c *VolumeCreateCommand) hostVolumeCreate(
var volID string
var lastIndex uint64

// note: the command only ever returns 1 volume from the API
for _, vol := range vols {
if detach || vol.State == api.HostVolumeStateReady {
c.Ui.Output(fmt.Sprintf(
"Created host volume %s with ID %s", vol.Name, vol.ID))
return 0
} else {
c.Ui.Output(fmt.Sprintf(
"==> Created host volume %s with ID %s", vol.Name, vol.ID))
volID = vol.ID
lastIndex = vol.ModifyIndex
break
}
if detach || vol.State == api.HostVolumeStateReady {
c.Ui.Output(fmt.Sprintf(
"Created host volume %s with ID %s", vol.Name, vol.ID))
return 0
} else {
c.Ui.Output(fmt.Sprintf(
"==> Created host volume %s with ID %s", vol.Name, vol.ID))
volID = vol.ID
lastIndex = vol.ModifyIndex
}

err = c.monitorHostVolume(client, volID, lastIndex, verbose)
Expand Down
11 changes: 4 additions & 7 deletions command/volume_register_host.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,18 +18,15 @@ func (c *VolumeRegisterCommand) hostVolumeRegister(client *api.Client, ast *ast.
}

req := &api.HostVolumeRegisterRequest{
Volumes: []*api.HostVolume{vol},
Volume: vol,
}
vols, _, err := client.HostVolumes().Register(req, nil)
vol, _, err = client.HostVolumes().Register(req, nil)
if err != nil {
c.Ui.Error(fmt.Sprintf("Error registering volume: %s", err))
return 1
}
for _, vol := range vols {
// note: the command only ever returns 1 volume from the API
c.Ui.Output(fmt.Sprintf(
"Registered host volume %s with ID %s", vol.Name, vol.ID))
}
c.Ui.Output(fmt.Sprintf(
"Registered host volume %s with ID %s", vol.Name, vol.ID))

return 0
}
2 changes: 1 addition & 1 deletion nomad/fsm.go
Original file line number Diff line number Diff line change
Expand Up @@ -2428,7 +2428,7 @@ func (n *nomadFSM) applyHostVolumeRegister(msgType structs.MessageType, buf []by
panic(fmt.Errorf("failed to decode request: %v", err))
}

if err := n.state.UpsertHostVolumes(index, req.Volumes); err != nil {
if err := n.state.UpsertHostVolume(index, req.Volume); err != nil {
n.logger.Error("UpsertHostVolumes failed", "error", err)
return err
}
Expand Down
Loading

0 comments on commit 1f7b72d

Please sign in to comment.