bug, deep coupling: virtctl.sh expects virtctl binary to be present in a certain folder

[dhiller]

Issue
When working on moving kubevirtci into it's own folder we noticed that kubevirtci expects the virtctl binary from k/kubevirt to be present in the system at a certain hard coded path 1 .

This is a deep coupling from one project to another which forces users of virtctl.sh to "somehow" provide said binary. This is likely a leftover of the move of kubevirtci to it's own repository.

See #94 (review)

Short-term
Short term it should be band-aided firstly by deprecating the virtctl.sh binary by issuing a WARN to stderr. This should give users time to move to other ways of calling the virtctl binary.

Also repos that show such calls 3, namely

• kubevirt/containerized-data-importer
• kubevirt/common-instancetypes
• kubevirt/community
• kubevirt/machine-remediation (likely one of the repos ready to archive - last change was 4 years ago)
• kubevirt/user-guide

should be updated to use the proper script from kubevirt/kubevirt (if necessary).

Search queries:

• virtctl.sh start|... - https://github.com/search?q=org%3Akubevirt+AND+-repo%3Akubevirt%2Fkubevirt+AND+%28%28%22virtctl.sh+console%22%29+OR+%28%22virtctl.sh+vnc%22%29+OR+%28%22virtctl.sh+start%22%29+OR+%28%22virtctl.sh+stop%22%29+OR+%28%22virtctl.sh+migrate%22%29%29&type=code
• kubectl.sh start|... - https://github.com/search?q=org%3Akubevirt+AND+-repo%3Akubevirt%2Fkubevirt+AND+%28%28%22kubectl.sh+console%22%29+OR+%28%22kubectl.sh+vnc%22%29+OR+%28%22kubectl.sh+start%22%29+OR+%28%22kubectl.sh+stop%22%29+OR+%28%22kubectl.sh+migrate%22%29+OR+%28%22kubectl.sh+virt%22%29%29&type=code
• virtctl.sh - https://github.com/search?q=org%3Akubevirt+AND++-repo%3Akubevirt%2Fkubevirt+AND+%22virtctl.sh%22&type=code

Long-term
Long term virtctl.sh should be removed. It will be replaced by a script inside k/kubevirt below the hack folder with this PR: kubevirt/kubevirt#12872

Workaround
An easy workaround would be installing it as a krew plugin and then using kubectl virt 2

/kind bug
/kind tracker

[andreabolognani]

@dhiller I just hit the deprecation message during development:

$ ./cluster-up/kubectl.sh apply -f examples/vmi-nocloud.yaml && ./cluster-up/virtctl.sh console vmi-nocloud
virtualmachineinstance.kubevirt.io/vmi-nocloud created
WARNING: usage of './cluster-up/virtctl.sh' is deprecated!
         see: https://github.com/kubevirt/kubevirtci/issues/1277
Successfully connected to vmi-nocloud console. The escape sequence is ^]
...

Even after reading the issue description, it's unclear to me what the recommended replacement for my usage of virtctl.sh would be. Can you please clarify? Thanks!

[dhiller]

@dhiller I just hit the deprecation message during development:

$ ./cluster-up/kubectl.sh apply -f examples/vmi-nocloud.yaml && ./cluster-up/virtctl.sh console vmi-nocloud
virtualmachineinstance.kubevirt.io/vmi-nocloud created
WARNING: usage of './cluster-up/virtctl.sh' is deprecated!
         see: https://github.com/kubevirt/kubevirtci/issues/1277
Successfully connected to vmi-nocloud console. The escape sequence is ^]
...

Even after reading the issue description, it's unclear to me what the recommended replacement for my usage of virtctl.sh would be. Can you please clarify? Thanks!

Hey @andreabolognani thanks for pointing this out! I figure this has happened while you were working on k/kubevirt, right?

Actually this will be addressed with this PR: kubevirt/kubevirt#12872

There I am replacing the kubevirtci virtctl.sh with a version that is inside k/kubevirt. Will that work for you or do we need to keep the virtctl.sh in a way?

[dhiller]

@andreabolognani I've addressed this in the issue comment - hope that makes it clear now.

[andreabolognani]

I figure this has happened while you were working on k/kubevirt, right?

Correct.

There I am replacing the kubevirtci virtctl.sh with a version that is inside k/kubevirt. Will that work for you or do we need to keep the virtctl.sh in a way?

So IIUC instead of calling ./cluster-up/virtctl.sh I will have to call ./hack/virtctl.sh? I'll have to adapt my muscle memory, but other than that it suits me just fine.

Hopefully kubectl.sh will also live next to virtctl.sh in the same folder? It would be annoying if that were no longer the case.

[dhiller]

I figure this has happened while you were working on k/kubevirt, right?

Correct.

There I am replacing the kubevirtci virtctl.sh with a version that is inside k/kubevirt. Will that work for you or do we need to keep the virtctl.sh in a way?

So IIUC instead of calling ./cluster-up/virtctl.sh I will have to call ./hack/virtctl.sh? I'll have to adapt my muscle memory, but other than that it suits me just fine.

Hopefully kubectl.sh will also live next to virtctl.sh in the same folder? It would be annoying if that were no longer the case.

I understand where you are coming from - we could just add a wrapper hack/kubectl.sh that calls kubevirtci/cluster-up/kubectl.sh. Or you'd define an alias? WDYT?

[andreabolognani]

I think a wrapper would be a good way to go about it.

Bonus points if ./cluster-up/kubeconfig.sh doesn't need to get a wrapper in hack/, which means that I will be able to type just ./h<TAB>/ku<TAB> to get the correct result. For comparison, right now I need to type ./cl<TAB>/kubect<TAB> so the number of keystrokes saved would be quite significant!

By the way, and this is only tangentially related: it used to be the case, a few years back, that you could run kubectl.sh without having kubectl installed on your machine, since the binary would be pulled from the running cluster. At some point that broke, so I resigned myself to installing kubectl on my machine despite not needing it outside of KubeVirt development. If at all feasible, going back to the previous status quo would be very appreciated. Maybe that's already the case and I simply haven't noticed? One can dream :)