Support shell completion in vic-machine

[zjs]

Implement a cli.BashCompleteFunc factory and use it to supply an appropriate instance with each cli.Command in vic-machine.

---

To try this functionality on Linux, you could run the following command after checking out and building this topic branch. (Replace linux with darwin or windows and/or bash with zsh as necessary.)

PROG=vic-machine-linux source infra/scripts/bash_autocomplete

A customer would just run a command like the following from the root of the extracted vic_*.tar.gz.

source autocomplete/bash/vic-machine-linux

Some example output showing completion suggestions:

~/go/src/github.com/vmware/vic (topic/bash-completion)$ ./bin/vic-machine-linux 
configure  create     debug      delete     h          help       inspect    ls         update     upgrade    version    
~/go/src/github.com/vmware/vic (topic/bash-completion)$ ./bin/vic-machine-linux create -
--affinity-vm-group           --container-name-convention   --ksz                         --public-network-ip
--ai                          --container-network           --kv                          -r
--appliance-iso               --container-network-dns       --management-network          --rc
--ar                          --container-network-firewall  --management-network-gateway  --registry-ca
--asymmetric-routes           --container-network-gateway   --management-network-ip       --syslog-address
-b                            --container-network-ip-range  --mem                         -t
--base-image-size             --cpu                         --memory                      --target
--bi                          --cpur                        --memory-reservation          --thumbprint
--bnr                         --cpu-reservation             --memory-shares               --timeout
--bootstrap-iso               --cpus                        --memr                        --tls-ca
--bridge-network              --cpu-shares                  --mems                        --tls-cert-path
--bridge-network-range        --debug                       --mn                          --tls-cname
--ca                          --dir                         -n                            --tls-server-cert
--certificate-key-size        --dns-server                  --name                        --tls-server-key
--client-network              --endpoint-cpu                --no-tls                      -u
--client-network-gateway      --endpoint-memory             --no-tlsverify                --user
--client-network-ip           --extended-help               --ops-grant-perms             -v
--cln                         -f                            --ops-password                --volume-store
--cn                          --force                       --ops-user                    --vs
--cnc                         --http-proxy                  --organization                --whitelist-registry
--cnd                         --https-proxy                 -p                            --wr
--cnf                         -i                            --password                    -x
--cng                         --image-store                 --pn                          
~/go/src/github.com/vmware/vic (topic/bash-completion)$ ./bin/vic-machine-linux update 
firewall  help      
shephezj@shepherdz-devl:~/go/src/github.com/vmware/vic (topic/bash-completion)$ ./bin/vic-machine-linux update firewall --t
--target      --thumbprint  --timeout     

[codecov-io]

Codecov Report

Merging #7785 into master will increase coverage by 0.83%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master    #7785      +/-   ##
==========================================
+ Coverage   25.69%   26.52%   +0.83%     
==========================================
  Files          35       36       +1     
  Lines        5125     5183      +58     
==========================================
+ Hits         1317     1375      +58     
  Misses       3701     3701              
  Partials      107      107

Impacted Files	Coverage Δ
cmd/vic-machine/common/completion.go	100% <100%> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0f3106b...f30e171. Read the comment docs.

[zjs]

Testing of changes to .drone.yml: https://ci-vic.vmware.com/vmware/vic/18576/9

[vburenin]

Commenting on this doesn't make much sense, but I will still comment it from the future standpoint.
Since you know the lengths of trimmed names array you could preallocate it all at once, thus resulting in a less memory allocations.

trimmedNames := make([]string, len(names))
for i, name := range names {
   trimmedNames[i] = strings.Trim(name, " ")
}

[zjs]

Looking at this blog post, it seems like I might be able to specify the desired capacity without setting a length:

	trimmedNames := make([]string, 0, len(names))
	for _, name := range names {
		trimmedNames = append(trimmedNames, strings.Trim(name, " "))
	}

I find that more intuitive than setting a length up-front, since this way the length is always accurate (after the second iteration through the loop the length would be 2, and so on).

Would there be disadvantages to doing it this way, versus what you suggest?

[vburenin]

O(m*n) :(
sure... it works fine.

[zjs]

A couple of factors led me here:

1. To do better, I need a sorted list. With the sorted list, I could do searches in O(log(n*m)). However initializing that sorted list actually takes O(n*m*log(n*m)). If I needed to find a lot of things (more than log(n*m)), that might be worth it, but in this case I'm only ever searching once so the initialization cost would dominate.
2. In our code, getNames always returns 1-2 elements (a long flag and maybe a short flag), so that's essential a constant factor.
3. The current pattern allows us to bail out as soon as we've found what we're looking for.

[vburenin]

Yah, that's why I said it works fine. Even if there were hundreds of elements in each list, it would not be really noticeable.

[vburenin]

didn't you get lastArg already? it would simplify this code

[mdharamadas1]

Don't we need integration tests for this? At least, just to make sure autocomplete can be enabled?

[zjs]

Don't we need integration tests for this? At least, just to make sure autocomplete can be enabled?

I'm not sure how to write an integration test for this. We'd need a way to open a shell, source a script, and enter characters as a user would (including pressing "tab"). That doesn't seem possible from Robot.

Edit: For what it's worth, the autocomplete scripts will not be changing frequently (even across the history of urfave/cli, each only has a handful of revisions). It seems like the easiest thing might be to manually test those any time we do have to make changes to them; the go code is entirely covered by unit tests.

[mdharamadas1]

@zjs I agree its complicated to write such tests but I think its possible. We already execute lot of shell commands in existing robot tests but tab press would complicate it a little. I would suggest at least creating an issue to add tests later. Otherwise, LGTM.

[mdharamadas1]

lgtm

[zjs]

Issue to track integration testing: #7850

[stuclem]

Documented in https://vmware.github.io/vic-product/assets/files/html/1.4/vic_vsphere_admin/running_vicmachine_cmds.html