dynamic host volumes: add -id arg for updates of existing volumes (#24996)

If you create a volume via `volume create/register` and want to update it later,
you need to change the volume spec to add the ID that was returned. This isn't a
very nice UX, so let's add an `-id` argument that allows you to update existing
volumes that have that ID.

Ref: https://hashicorp.atlassian.net/browse/NET-12083
This commit is contained in:
Tim Gross
2025-02-03 10:26:30 -05:00
committed by GitHub
parent e4659970b1
commit cc99e8f0a2
7 changed files with 77 additions and 8 deletions

View File

@@ -42,6 +42,10 @@ Create Options:
command. If -detach is omitted or false, the command will monitor the state
of the volume until it is ready to be scheduled.
-id
Update a volume previously created with this ID prefix. Used for dynamic
host volumes only.
-verbose
Display full information when monitoring volume state. Used for dynamic host
volumes only.
@@ -60,6 +64,7 @@ func (c *VolumeCreateCommand) AutocompleteFlags() complete.Flags {
"-detach": complete.PredictNothing,
"-verbose": complete.PredictNothing,
"-policy-override": complete.PredictNothing,
"-id": complete.PredictNothing,
})
}
@@ -75,10 +80,12 @@ func (c *VolumeCreateCommand) Name() string { return "volume create" }
func (c *VolumeCreateCommand) Run(args []string) int {
var detach, verbose, override bool
var volID string
flags := c.Meta.FlagSet(c.Name(), FlagSetClient)
flags.BoolVar(&detach, "detach", false, "detach from monitor")
flags.BoolVar(&verbose, "verbose", false, "display full volume IDs")
flags.BoolVar(&override, "policy-override", false, "override soft mandatory Sentinel policies")
flags.StringVar(&volID, "id", "", "update an existing dynamic host volume")
flags.Usage = func() { c.Ui.Output(c.Help()) }
if err := flags.Parse(args); err != nil {
@@ -129,7 +136,7 @@ func (c *VolumeCreateCommand) Run(args []string) int {
case "csi":
return c.csiCreate(client, ast)
case "host":
return c.hostVolumeCreate(client, ast, detach, verbose, override)
return c.hostVolumeCreate(client, ast, detach, verbose, override, volID)
default:
c.Ui.Error(fmt.Sprintf("Error unknown volume type: %s", volType))
return 1

View File

@@ -19,13 +19,34 @@ import (
)
func (c *VolumeCreateCommand) hostVolumeCreate(
client *api.Client, ast *ast.File, detach, verbose, override bool) int {
client *api.Client, ast *ast.File, detach, verbose, override bool, volID string) int {
vol, err := decodeHostVolume(ast)
if err != nil {
c.Ui.Error(fmt.Sprintf("Error decoding the volume definition: %s", err))
return 1
}
if volID != "" {
ns := c.namespace
if vol.Namespace != "" {
ns = vol.Namespace
}
stub, possible, err := getHostVolumeByPrefix(client, volID, ns)
if err != nil {
c.Ui.Error(fmt.Sprintf("Could not update existing volume: %s", err))
return 1
}
if len(possible) > 0 {
out, err := formatHostVolumes(possible, formatOpts{short: true})
if err != nil {
c.Ui.Error(fmt.Sprintf("Error formatting: %s", err))
return 1
}
c.Ui.Error(fmt.Sprintf("Prefix matched multiple volumes\n\n%s", out))
return 1
}
vol.ID = stub.ID
}
req := &api.HostVolumeCreateRequest{
Volume: vol,
@@ -44,7 +65,6 @@ func (c *VolumeCreateCommand) hostVolumeCreate(
fmt.Sprintf("[bold][yellow]Volume Warnings:\n%s[reset]\n", resp.Warnings)))
}
var volID string
var lastIndex uint64
if detach || vol.State == api.HostVolumeStateReady {

View File

@@ -80,6 +80,13 @@ parameters {
got, _, err := client.HostVolumes().Get(id, &api.QueryOptions{Namespace: "prod"})
must.NoError(t, err)
must.NotNil(t, got)
// Verify we can update the volume without changes
args = []string{"-address", url, "-detach", "-id", got.ID, file.Name()}
code = cmd.Run(args)
must.Eq(t, 0, code, must.Sprintf("got error: %s", ui.ErrorWriter.String()))
list, _, err := client.HostVolumes().List(nil, &api.QueryOptions{Namespace: "prod"})
must.Len(t, 1, list, must.Sprintf("new volume should not be created on update"))
}
func TestHostVolume_HCLDecode(t *testing.T) {

View File

@@ -38,6 +38,10 @@ General Options:
Register Options:
-id
Update a volume previously created with this ID prefix. Used for dynamic
host volumes only.
-policy-override
Sets the flag to force override any soft mandatory Sentinel policies. Used
for dynamic host volumes only.
@@ -50,6 +54,7 @@ func (c *VolumeRegisterCommand) AutocompleteFlags() complete.Flags {
return mergeAutocompleteFlags(c.Meta.AutocompleteFlags(FlagSetClient),
complete.Flags{
"-policy-override": complete.PredictNothing,
"-id": complete.PredictNothing,
})
}
@@ -65,8 +70,10 @@ func (c *VolumeRegisterCommand) Name() string { return "volume register" }
func (c *VolumeRegisterCommand) Run(args []string) int {
var override bool
var volID string
flags := c.Meta.FlagSet(c.Name(), FlagSetClient)
flags.BoolVar(&override, "policy-override", false, "override soft mandatory Sentinel policies")
flags.StringVar(&volID, "id", "", "update an existing dynamic host volume")
flags.Usage = func() { c.Ui.Output(c.Help()) }
if err := flags.Parse(args); err != nil {
@@ -118,7 +125,7 @@ func (c *VolumeRegisterCommand) Run(args []string) int {
case "csi":
return c.csiRegister(client, ast)
case "host":
return c.hostVolumeRegister(client, ast, override)
return c.hostVolumeRegister(client, ast, override, volID)
default:
c.Ui.Error(fmt.Sprintf("Error unknown volume type: %s", volType))
return 1

View File

@@ -10,7 +10,7 @@ import (
"github.com/hashicorp/nomad/api"
)
func (c *VolumeRegisterCommand) hostVolumeRegister(client *api.Client, ast *ast.File, override bool) int {
func (c *VolumeRegisterCommand) hostVolumeRegister(client *api.Client, ast *ast.File, override bool, volID string) int {
vol, err := decodeHostVolume(ast)
if err != nil {
c.Ui.Error(fmt.Sprintf("Error decoding the volume definition: %s", err))
@@ -20,6 +20,27 @@ func (c *VolumeRegisterCommand) hostVolumeRegister(client *api.Client, ast *ast.
c.Ui.Error("Node ID is required for registering")
return 1
}
if volID != "" {
ns := c.namespace
if vol.Namespace != "" {
ns = vol.Namespace
}
stub, possible, err := getHostVolumeByPrefix(client, volID, ns)
if err != nil {
c.Ui.Error(fmt.Sprintf("Could not update existing volume: %s", err))
return 1
}
if len(possible) > 0 {
out, err := formatHostVolumes(possible, formatOpts{short: true})
if err != nil {
c.Ui.Error(fmt.Sprintf("Error formatting: %s", err))
return 1
}
c.Ui.Error(fmt.Sprintf("Prefix matched multiple volumes\n\n%s", out))
return 1
}
vol.ID = stub.ID
}
req := &api.HostVolumeRegisterRequest{
Volume: vol,

View File

@@ -92,4 +92,11 @@ parameters {
got, _, err := client.HostVolumes().Get(id, &api.QueryOptions{Namespace: "prod"})
must.NoError(t, err)
must.NotNil(t, got)
// Verify we can update the volume without changes
args = []string{"-address", url, "-id", got.ID, file.Name()}
code = cmd.Run(args)
must.Eq(t, 0, code, must.Sprintf("got error: %s", ui.ErrorWriter.String()))
list, _, err := client.HostVolumes().List(nil, &api.QueryOptions{Namespace: "prod"})
must.Len(t, 1, list, must.Sprintf("new volume should not be registered on update"))
}

View File

@@ -34,7 +34,7 @@ func (c *VolumeStatusCommand) hostVolumeStatus(client *api.Client, id, nodeID, n
// if an exact match is not found. note we can't use the shared getByPrefix
// helper here because the List API doesn't match the required signature
volStub, possible, err := c.getByPrefix(client, id)
volStub, possible, err := getHostVolumeByPrefix(client, id, c.namespace)
if err != nil {
c.Ui.Error(fmt.Sprintf("Error listing volumes: %s", err))
return 1
@@ -91,10 +91,10 @@ func (c *VolumeStatusCommand) listHostVolumes(client *api.Client, nodeID, nodePo
return 0
}
func (c *VolumeStatusCommand) getByPrefix(client *api.Client, prefix string) (*api.HostVolumeStub, []*api.HostVolumeStub, error) {
func getHostVolumeByPrefix(client *api.Client, prefix, ns string) (*api.HostVolumeStub, []*api.HostVolumeStub, error) {
vols, _, err := client.HostVolumes().List(nil, &api.QueryOptions{
Prefix: prefix,
Namespace: c.namespace,
Namespace: ns,
})
if err != nil {