Skip to content

Commit

Permalink
lxc: Accept volume full name on detach (#14593)
Browse files Browse the repository at this point in the history 
The completions suggest the volume name but the client doesn't parse it
correctly and this has been annoying me, so I took some of my time off
to fix it and also improved the completions for detach/attach commands.
Along with other small improvements.
  • Loading branch information
@tomponline
tomponline authored Dec 11, 2024
2 parents d56d152 + 2ef5355 commit edcc404
Show file tree
Hide file tree
Showing 3 changed files with 49 additions and 13 deletions.
35 changes: 29 additions & 6 deletions lxc/completion.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1368,10 +1368,8 @@ func (g *cmdGlobal) cmpStoragePoolWithVolume(toComplete string) ([]string, cobra

var results []string
for _, volume := range volumes {
volName, volType := parseVolume("custom", volume)
if volType == "custom" {
results = append(results, pool+"/"+volName)
}
volName, _ := parseVolume("custom", volume)
results = append(results, pool+"/"+volName)
}

return results, cobra.ShellCompDirectiveNoFileComp
Expand Down Expand Up @@ -1549,7 +1547,8 @@ func (g *cmdGlobal) cmpStoragePoolVolumeSnapshots(poolName string, volumeName st

// cmpStoragePoolVolumes provides shell completion for storage pool volumes.
// It takes a storage pool name and returns a list of storage pool volumes along with a shell completion directive.
func (g *cmdGlobal) cmpStoragePoolVolumes(poolName string) ([]string, cobra.ShellCompDirective) {
// Parameter volumeTypes determines which types of volumes are valid as completion options, none being provided means all types are valid.
func (g *cmdGlobal) cmpStoragePoolVolumes(poolName string, volumeTypes ...string) ([]string, cobra.ShellCompDirective) {
// Parse remote
resources, err := g.ParseServers(poolName)
if err != nil || len(resources) == 0 {
Expand All @@ -1569,5 +1568,29 @@ func (g *cmdGlobal) cmpStoragePoolVolumes(poolName string) ([]string, cobra.Shel
return nil, cobra.ShellCompDirectiveError
}

return volumes, cobra.ShellCompDirectiveNoFileComp
// If no volume type is provided, don't filter on type.
if len(volumeTypes) == 0 {
return volumes, cobra.ShellCompDirectiveNoFileComp
}

// Pre-allocate slice capacity.
customKeyCount := 0
for _, volume := range volumes {
_, volType := parseVolume("custom", volume)
if shared.ValueInSlice(volType, volumeTypes) {
customKeyCount++
}
}

customVolumeNames := make([]string, 0, customKeyCount)

// Only include custom volumes
for _, volume := range volumes {
_, volType := parseVolume("custom", volume)
if shared.ValueInSlice(volType, volumeTypes) {
customVolumeNames = append(customVolumeNames, volume)
}
}

return customVolumeNames, cobra.ShellCompDirectiveNoFileComp
}
23 changes: 17 additions & 6 deletions lxc/storage_volume.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,7 @@ func (c *cmdStorageVolumeAttach) command() *cobra.Command {
}

if len(args) == 1 {
return c.global.cmpStoragePoolVolumes(args[0])
return c.global.cmpStoragePoolVolumes(args[0], "custom")
}

if len(args) == 2 {
Expand Down Expand Up @@ -236,6 +236,7 @@ func (c *cmdStorageVolumeAttach) run(cmd *cobra.Command, args []string) error {
case "block", "iso":
devName = args[3]
case "filesystem":
// If using a filesystem volume, the path must also be provided as the fourth argument.
if !strings.HasPrefix(args[3], "/") {
devPath = path.Join("/", args[3])
} else {
Expand Down Expand Up @@ -291,7 +292,7 @@ func (c *cmdStorageVolumeAttachProfile) command() *cobra.Command {
}

if len(args) == 1 {
return c.global.cmpStoragePoolVolumes(args[0])
return c.global.cmpStoragePoolVolumes(args[0], "custom")
}

if len(args) == 2 {
Expand Down Expand Up @@ -815,7 +816,7 @@ func (c *cmdStorageVolumeDetach) command() *cobra.Command {
}

if len(args) == 1 {
return c.global.cmpStoragePoolVolumes(args[0])
return c.global.cmpStoragePoolVolumes(args[0], "custom")
}

if len(args) == 2 {
Expand Down Expand Up @@ -859,10 +860,15 @@ func (c *cmdStorageVolumeDetach) run(cmd *cobra.Command, args []string) error {
return err
}

volName, volType := parseVolume("custom", args[1])
if volType != "custom" {
return errors.New(i18n.G(`Only "custom" volumes can be attached to instances`))
}

// Find the device
if devName == "" {
for n, d := range inst.Devices {
if d["type"] == "disk" && d["pool"] == resource.name && d["source"] == args[1] {
if d["type"] == "disk" && d["pool"] == resource.name && d["source"] == volName {
if devName != "" {
return errors.New(i18n.G("More than one device matches, specify the device name"))
}
Expand Down Expand Up @@ -913,7 +919,7 @@ func (c *cmdStorageVolumeDetachProfile) command() *cobra.Command {
}

if len(args) == 1 {
return c.global.cmpStoragePoolVolumes(args[0])
return c.global.cmpStoragePoolVolumes(args[0], "custom")
}

if len(args) == 2 {
Expand Down Expand Up @@ -956,10 +962,15 @@ func (c *cmdStorageVolumeDetachProfile) run(cmd *cobra.Command, args []string) e
return err
}

volName, volType := parseVolume("custom", args[1])
if volType != "custom" {
return errors.New(i18n.G(`Only "custom" volumes can be attached to instances`))
}

// Find the device
if devName == "" {
for n, d := range profile.Devices {
if d["type"] == "disk" && d["pool"] == resource.name && d["source"] == args[1] {
if d["type"] == "disk" && d["pool"] == resource.name && d["source"] == volName {
if devName != "" {
return errors.New(i18n.G("More than one device matches, specify the device name"))
}
Expand Down
4 changes: 3 additions & 1 deletion test/suites/storage.sh
View file
Original file line number Diff line number Diff line change
Expand Up @@ -337,10 +337,12 @@ EOF
lxc storage volume set "lxdtest-$(basename "${LXD_DIR}")-pool1" c1pool1 zfs.use_refquota true
lxc storage volume attach "lxdtest-$(basename "${LXD_DIR}")-pool1" c1pool1 c1pool1 testDevice /opt
! lxc storage volume attach "lxdtest-$(basename "${LXD_DIR}")-pool1" c1pool1 c1pool1 testDevice2 /opt || false
lxc config show c1pool1 | grep -Pz " testDevice:\n path: /opt\n pool: lxdtest-$(basename "${LXD_DIR}")-pool1\n source: c1pool1\n type: disk\n"
lxc storage volume detach "lxdtest-$(basename "${LXD_DIR}")-pool1" c1pool1 c1pool1
! lxc config show c1pool1 | grep "testDevice" || false
lxc storage volume attach "lxdtest-$(basename "${LXD_DIR}")-pool1" custom/c1pool1 c1pool1 testDevice /opt
! lxc storage volume attach "lxdtest-$(basename "${LXD_DIR}")-pool1" custom/c1pool1 c1pool1 testDevice2 /opt || false
lxc storage volume detach "lxdtest-$(basename "${LXD_DIR}")-pool1" c1pool1 c1pool1
lxc storage volume detach "lxdtest-$(basename "${LXD_DIR}")-pool1" custom/c1pool1 c1pool1

lxc storage volume create "lxdtest-$(basename "${LXD_DIR}")-pool1" c2pool2
lxc storage volume attach "lxdtest-$(basename "${LXD_DIR}")-pool1" c2pool2 c2pool2 testDevice /opt
Expand Down

0 comments on commit edcc404

Please sign in to comment.