`lxc storage volume copy` completion issue

[simondeziel]

Here's the "completion flow":

# good
$ lxc storage volume copy 
default/     juju-btrfs/  juju-zfs/    

# extraneous ` ` (space)
$ lxc storage volume copy default/ 
default     juju-btrfs  juju-zfs    

# good
$ lxc storage volume copy default/
default/container/ansible    default/custom/ci-sdb    default/container/ansible01

# extraneous `custom/`
$ lxc storage volume copy default/custom/ci-sdb 
default     juju-btrfs  juju-zfs    

# the 2nd storage pool should be tab complete-able but is not
$ lxc storage volume copy default/custom/ci-sdb juju-zfs 
Error: Storage pool volume not found

# removing the bogus `custom/` bit works
# but the 2nd storage pool should have `/` at the end because a volume name needs to be provided
$ lxc storage volume copy default/ci-sdb juju-zfs

$ lxc storage volume copy default/ci-sdb juju-zfs/ci-sdb
Storage volume copied successfully!

@kadinsayani, there is a lot to unpack in the above so please let me if anything is unclear!

[simondeziel]

@MusicDin I hope that covers the bug you found about the bogus custom/ bit showing up when tab completing volumes.

[MusicDin]

Yes, thanks

[simondeziel]

I believe code introduced some different oddities:

$ lxc storage volume copy default/<tab>
default/24.04-desktop                                                     default/b0ded57c28fa07670f2318f5e63ca333413a022065f06829c277742cd6216371  default/noble-builder
default/5a63bc87974e61c631567c0d171fefffb33ebb7525b8295672ef0c5bf2cbd898

In the above, it's listing image volumes (fingerprint) along with instances and image aliases.

$ lxc storage volume list default
+-----------------+------------------------------------------------------------------+-------------+--------------+---------+
|      TYPE       |                               NAME                               | DESCRIPTION | CONTENT-TYPE | USED BY |
+-----------------+------------------------------------------------------------------+-------------+--------------+---------+------+
| container       | noble-builder                                                    |             | filesystem   | 1       |
+-----------------+------------------------------------------------------------------+-------------+--------------+---------+
| custom          | 24.04-desktop                                                    |             | iso          | 0       |
+-----------------+------------------------------------------------------------------+-------------+--------------+---------+
| custom          | backups                                                          |             | filesystem   | 0       |
+-----------------+------------------------------------------------------------------+-------------+--------------+---------+
| custom          | images                                                           |             | filesystem   | 0       |
+-----------------+------------------------------------------------------------------+-------------+--------------+---------+
| image           | 5a63bc87974e61c631567c0d171fefffb33ebb7525b8295672ef0c5bf2cbd898 |             | block        | 1       |
+-----------------+------------------------------------------------------------------+-------------+--------------+---------+
| image           | 6d465d8544583b826bad13fd1c92973b38ff454d3c0d3554a81c4d9dd258974b |             | filesystem   | 1       |
+-----------------+------------------------------------------------------------------+-------------+--------------+---------+
| image           | 7aed2da77e654ceffff832feeae8e07f6665cbbe3315076996ce7617397b5b6c |             | block        | 1       |
+-----------------+------------------------------------------------------------------+-------------+--------------+---------+
| image           | b0ded57c28fa07670f2318f5e63ca333413a022065f06829c277742cd6216371 |             | block        | 1       |
+-----------------+------------------------------------------------------------------+-------------+--------------+---------+
| image           | cd7f830eff3c538b7ec5777d1296a271cc3618e05a1c14c240cbdff43afa18b7 |             | block        | 1       |
+-----------------+------------------------------------------------------------------+-------------+--------------+---------+

I think the custom/ and image/ part should not be removed.

Lastly, the bogus/extraneous space after completing the pool name still affects my local machine.

[kadinsayani]

In the above, it's listing image volumes (fingerprint) along with instances and image aliases.

Is the output of lxc storage volume list default also erroneous? It makes sense for the completions to include all storage volumes for a pool.

Lastly, the bogus/extraneous space after completing the pool name still affects my local machine.

I'm not observing the extraneous space locally. It's possible that you may have old shell completion artifacts on your machine. Can you please try lxc completion bash > <output file> and . <output file>?

[simondeziel]

In the above, it's listing image volumes (fingerprint) along with instances and image aliases.

Is the output of lxc storage volume list default also erroneous? It makes sense for the completions to include all storage volumes for a pool.

lxc storage volume list default shows the expected output. I trimmed its list as well as the lxc storage volume copy default/<tab> output so maybe that was confusing.

Lastly, the bogus/extraneous space after completing the pool name still affects my local machine.

I'm not observing the extraneous space locally. It's possible that you may have old shell completion artifacts on your machine. Can you please try lxc completion bash > <output file> and . <output file>?

That seems to be it as sourcing a freshly generated completion file works. However I couldn't trace where it'd be coming from:

$ ll /etc/bash_completion.d/
total 4
drwxr-xr-x 1 root root   20 May 28 13:06 ./
drwxr-xr-x 1 root root 3906 Oct 16 09:30 ../
-rw-r--r-- 1 root root  439 Jul  5  2022 git-prompt

$ ls -1 /usr/share/bash-completion/completions/ | grep lx
# nothing

[kadinsayani]

I don't see any logic in my change that would cause a regression for the lxc storage volume copy tab completion. If the completion is showing results containing storage volumes across the pool, I think we're good? Or would you expect to only see custom and container volumes for this completion?

I'm glad to hear regenerating the completion script worked! Older versions of lxd might have retained completion specifications even though the script in /etc/bash_completion.d/ was removed. A simple way to verify completion specifications for a command is to execute complete -p lxc. It's also worth noting that there were significant changes introduced with bash completions v3, resulting in differing behaviour across shell environments. Here's an example of an issue raised on the Cobra repo highlighting different behaviour for different shells: https://github.com/spf13/cobra/issues/1740.

[simondeziel]

I don't see any logic in my change that would cause a regression for the lxc storage volume copy tab completion. If the completion is showing results containing storage volumes across the pool, I think we're good? Or would you only expect to see custom and container volumes for this completion?

It seems only custom volumes are copy'able by lxc storage volume copy so the tab completion should only offer those.

ATM it offers much more:

$ lxc storage volume copy default/<tab>
default/24.04-desktop                                                     default/autopkg                                                           default/noble-builder
default/29476d92f722a8c9fac18738ea64a412f76c657a78f3273e40a2c15fdc2d588e  default/b0ded57c28fa07670f2318f5e63ca333413a022065f06829c277742cd6216371  default/oracular-builder
default/5a63bc87974e61c631567c0d171fefffb33ebb7525b8295672ef0c5bf2cbd898  default/backups                                                           default/polar-signal
default/6d465d8544583b826bad13fd1c92973b38ff454d3c0d3554a81c4d9dd258974b  default/cs50-python                                                       default/pylxd
default/7aed2da77e654ceffff832feeae8e07f6665cbbe3315076996ce7617397b5b6c  default/images                                                            default/wine-games
default/ansible                                                           default/jammy-builder                                                     
default/ansible01                                                         default/lxd-core18

The storage volume list is

$ lxc storage volume list default 
+-----------------+------------------------------------------------------------------+-------------+--------------+---------+
|      TYPE       |                               NAME                               | DESCRIPTION | CONTENT-TYPE | USED BY |
+-----------------+------------------------------------------------------------------+-------------+--------------+---------+
| container       | ansible                                                          |             | filesystem   | 1       |
+-----------------+------------------------------------------------------------------+-------------+--------------+---------+
| container       | ansible01                                                        |             | filesystem   | 1       |
+-----------------+------------------------------------------------------------------+-------------+--------------+---------+
| container       | cs50-python                                                      |             | filesystem   | 1       |
+-----------------+------------------------------------------------------------------+-------------+--------------+---------+
| container       | jammy-builder                                                    |             | filesystem   | 1       |
+-----------------+------------------------------------------------------------------+-------------+--------------+---------+
| container       | noble-builder                                                    |             | filesystem   | 1       |
+-----------------+------------------------------------------------------------------+-------------+--------------+---------+
| container       | oracular-builder                                                 |             | filesystem   | 1       |
+-----------------+------------------------------------------------------------------+-------------+--------------+---------+
| container       | wine-games                                                       |             | filesystem   | 1       |
+-----------------+------------------------------------------------------------------+-------------+--------------+---------+
| custom          | 24.04-desktop                                                    |             | iso          | 0       |
+-----------------+------------------------------------------------------------------+-------------+--------------+---------+
| custom          | backups                                                          |             | filesystem   | 0       |
+-----------------+------------------------------------------------------------------+-------------+--------------+---------+
| custom          | images                                                           |             | filesystem   | 0       |
+-----------------+------------------------------------------------------------------+-------------+--------------+---------+
| image           | 5a63bc87974e61c631567c0d171fefffb33ebb7525b8295672ef0c5bf2cbd898 |             | block        | 1       |
+-----------------+------------------------------------------------------------------+-------------+--------------+---------+
| image           | 6d465d8544583b826bad13fd1c92973b38ff454d3c0d3554a81c4d9dd258974b |             | filesystem   | 1       |
+-----------------+------------------------------------------------------------------+-------------+--------------+---------+
| image           | 7aed2da77e654ceffff832feeae8e07f6665cbbe3315076996ce7617397b5b6c |             | block        | 1       |
+-----------------+------------------------------------------------------------------+-------------+--------------+---------+
| image           | 29476d92f722a8c9fac18738ea64a412f76c657a78f3273e40a2c15fdc2d588e |             | block        | 1       |
+-----------------+------------------------------------------------------------------+-------------+--------------+---------+
| image           | b0ded57c28fa07670f2318f5e63ca333413a022065f06829c277742cd6216371 |             | block        | 1       |
+-----------------+------------------------------------------------------------------+-------------+--------------+---------+
| virtual-machine | autopkg                                                          |             | block        | 1       |
+-----------------+------------------------------------------------------------------+-------------+--------------+---------+
| virtual-machine | lxd-core18                                                       |             | block        | 1       |
+-----------------+------------------------------------------------------------------+-------------+--------------+---------+
| virtual-machine | polar-signal                                                     |             | block        | 1       |
+-----------------+------------------------------------------------------------------+-------------+--------------+---------+
| virtual-machine | pylxd                                                            |             | block        | 1       |
+-----------------+------------------------------------------------------------------+-------------+--------------+---------+

So I tried copying one of every type and only custom work so only those should be considered when tab-completing.

I'm glad to hear regenerating the completion script worked! Older versions of lxd might have retained completion specifications even though the script in /etc/bash_completion.d/ was removed. A simple way to verify completion specifications for a command is to execute complete -p lxc. It's also worth noting that there were significant changes introduced with bash completions v3, resulting in differing behaviour across shell environments. Here's an example of an issue raised on the Cobra repo: spf13/cobra#1740 highlighting different behaviour for different shells.

Odd as I don't see any in a fresh shell:

# completion is misbehaving by adding bogus trailing spaces after storage pool names
$ complete -p lxc
bash: complete: lxc: no completion specification

# loading fresh completion rules
$ lxc completion bash > /tmp/comp
$ . /tmp/comp
$ complete -p lxc
complete -o default -F __start_lxc lxc

[kadinsayani]

It seems only custom volumes are copy'able by lxc storage volume copy so the tab completion should only offer those.

Gotcha, thanks for clarifying!

Odd as I don't see any in a fresh shell

You'll have to run lxc [TAB] [TAB] before the completion spec is loaded. And in some cases, you'll have to run the lxd snap prior to loading the completion spec.

[simondeziel]

In a fresh terminal with the bogus completions coming from I don't know where:

$ lxc ls >/dev/null
$ lxc <tab><tab>
# shows loads of options coming from modern completion (your work)

$ complete -p lxc
complete -F _complete_from_snap lxc

Could it be that those bogus completions come from an older snap revision?

[kadinsayani]

I just tested a fresh install of the LXD snap on a fresh VM and I'm observing the extraneous space after tab completing lxc storage volume copy. After regenerating the completion script, I'm not observing the error. I'll look into this further, but I suspect it has something to do with the generation of the completion script during the snap build:

https://github.com/canonical/lxd-pkg-snap/blob/latest-edge/snapcraft.yaml#L1424

[kadinsayani]

Added similar completion logic to lxc storage volume move as well.

[tomponline]

It needs to work with the bash version in core22 (stable and candidate) and core24 (edge).

[kadinsayani]

Reopening to track the extraneous space issue.

[kadinsayani]

I believe I've found the problem causing the extraneous space to be added after running lxc storage volume copy [TAB].

snapd intercepts all shell completion requests. Given that the default behaviour of bash completions is to add a space, even though we've added a ShellCompDirectiveNoSpace directive to the lxc storage volume completer, snap is calling the completion function with:

https://github.com/canonical/snapd/blob/29a3a644254cc48c07eadd376c25073aa32c89f4/data/completion/bash/complete.sh#L120

I'm trying to see if I can get a workaround working - more on this soon :)

[kadinsayani]

There isn't anything wrong with the completion script we're generating in the snap:

root@v1:~# cp /snap/lxd/current/etc/bash_completion.d/snap.lxd.lxc /tmp/comp
root@v1:~# . /tmp/comp
root@v1:~# lxc storage volume copy default/vol1 # no extraneous space

[kadinsayani]

The logic in our completions script is quite verbose and should handle the no space directive. For versions of bash that don't support compopt, we manually add the nospace option to the completion function. The extraneous space issue is likely caused by snapd intercepting the request and running its completion function with default bash completion behaviour (which includes a space after every tab).

I've opened a PR to unhide the lxc completion command. Overall, this method for setting up completions is superior due to snap's approach of intercepting completion requests.

[simondeziel]

Here is additional information that I think is relevant to the issue:

# tab'ing after `default`:
$ lxc storage volume copy default<tab>

# adds `/ ` so the trailing space
# it seems to indicate the completion script thinks the option is complete and hence adds a space
# to be ready to taking the next arg
$ lxc storage volume copy default/ 

# If I remove the extraneous space with backspace to just have `default/`, it starts behaving fine:
$ lxc storage volume copy default/ <backspace>
$ lxc storage volume copy default/
$ lxc storage volume copy default/<tab>
default/24.04-desktop  default/backups        default/images         

So could it be that the complete just thinks it's done with the completion and adds a space in preparation of receiving the next args?

[kadinsayani]

Here is additional information that I think is relevant to the issue:

# tab'ing after `default`:
$ lxc storage volume copy default<tab>

# adds `/ ` so the trailing space
# it seems to indicate the completion script thinks the option is complete and hence adds a space
# to be ready to taking the next arg
$ lxc storage volume copy default/ 

# If I remove the extraneous space with backspace to just have `default/`, it starts behaving fine:
$ lxc storage volume copy default/ <backspace>
$ lxc storage volume copy default/
$ lxc storage volume copy default/<tab>
default/24.04-desktop  default/backups        default/images         

So could it be that the complete just thinks it's done with the completion and adds a space in preparation of receiving the next args?

It looks like the completion receives the right directive. See below from within a lxd snap shell:

bash-5.2# /snap/lxd/current/commands/lxc __complete storage volume copy defa
default/
:2
Completion ended with directive: ShellCompDirectiveNoSpace

I've tried editing the completion script to always send a nospace completion, and that worked.

[kadinsayani]

Upon further examination of snapd's completion marshaller, it appears that handling of the sep variable in the complete.sh script ensures that the separator is blank (see below). Since our completion is separated by a /, if sep is not empty, the script returns 2 and terminates further processing. I believe this is why backspacing and requesting the completion again works. Although, I have no way to test this. bash-completion does provide debugging tools, but it's not possible to get them working in the snap confinement space with snapd's completion marshaller.

https://github.com/canonical/snapd/blob/11c4e842c67206338e11e941edc024b15e121698/data/completion/bash/complete.sh#L71-L75

@simondeziel @MusicDin what are your thoughts on changing the CLI for cases like this from

lxc storage volume copy [<remote>:]<pool>/<volume>[/<snapshot>] [<remote>:]<pool>/<volume> [flags]

to

lxc storage volume copy [<remote>] <pool> <volume> [<snapshot>] [<remote>] <pool> <volume> [flags] (notice the spaces).

Although we would probably need to make similar changes to lxc launch and lxc init.

[tomponline]

Upon further examination of snapd's completion marshaller, it appears that handling of the sep variable in the complete.sh script ensures that the separator is blank (see below). Since our completion is separated by a /, if sep is not empty, the script returns 2 and terminates further processing. I believe this is why backspacing and requesting the completion again works. Although, I have no way to test this. bash-completion does provide debugging tools, but it's not possible to get them working in the snap confinement space with snapd's completion marshaller.

https://github.com/canonical/snapd/blob/11c4e842c67206338e11e941edc024b15e121698/data/completion/bash/complete.sh#L71-L75

@simondeziel @MusicDin what are your thoughts on changing the CLI for cases like this from

lxc storage volume copy [<remote>:]<pool>/<volume>[/<snapshot>] [<remote>:]<pool>/<volume> [flags]

to

lxc storage volume copy [<remote>] <pool> <volume> [<snapshot>] [<remote>] <pool> <volume> [flags] (notice the spaces).

Although we would probably need to make similar changes to lxc launch and lxc init.

That would be an undesirable CLI break for our users because it would break integrations.

[kadinsayani]

That would be an undesirable CLI break for our users because it would break integrations.

Gotcha - I've also reached out to the Snap team for guidance on this issue. I'll keep you all posted.

[simondeziel]

Upon further examination of snapd's completion marshaller, it appears that handling of the sep variable in the complete.sh script ensures that the separator is blank (see below). Since our completion is separated by a /, if sep is not empty, the script returns 2 and terminates further processing. I believe this is why backspacing and requesting the completion again works. Although, I have no way to test this. bash-completion does provide debugging tools, but it's not possible to get them working in the snap confinement space with snapd's completion marshaller.

The snap(d) folks are usually quite approachable if you want some guidance/help (see here :)

Update: sorry, I had not read your other comment, glad that you asked them already!

https://github.com/canonical/snapd/blob/11c4e842c67206338e11e941edc024b15e121698/data/completion/bash/complete.sh#L71-L75

Have you tried bind mounting a complete.sh file patched to your liking that supports our "mad" separator?

[kadinsayani]

Have you tried bind mounting a complete.sh file patched to your liking that supports out "mad" separator?

I just gave it a try and was unsuccessful. snapd also has another script which de-serializes requests and serializes completion results, so there may be a bug in there as well. Since sourcing our completion script directly in the user shell works as expected and the correct completion result is returned, I'm fairly confident that the issue is related to snapd's completion marshalling, specifically the handling of completion options.

[kadinsayani]

After further debugging, I tried modifying the below check in our completion script to always be true:

if (((directive & shellCompDirectiveNoSpace) != 0)); then
            if [[ $(type -t compopt) == builtin ]]; then
                __lxc_debug "Activating no space"
                compopt -o nospace
            else
                __lxc_debug "No space directive not supported in this version of bash"
            fi
fi

After the change, the completions from snapd do not include the extraneous space. I didn't bother attempting to modify this before because I assumed executing type -t compopt would work in the snap environment since it works in a lxd.lxc snap shell.

[kadinsayani]

Resolved with https://github.com/canonical/lxd-pkg-snap/pull/630.