Consider to move from shell to go

[guillaumerose]

snc code is becoming large: multi-arch support, cluster profiles, etc. We have now more than 800 lines of shell, it's maybe time to move to go. It will improve the console output (goodbye set -x), give us better error messages and tooling for templating (see crc_libvirt.template).

[gbraad]

currently the problem with moving to go would be a needed compile process. we'd rather have no need for external toolds...

... however, we are considering how to handle the generation process and tooling; command based., etc... ?

[cfergeau]

A bit torn on this one, code would be nicer to read in go, but would it be easier to add new features? In some cases, yes, in other cases, I'm not fully sure. I don't feel the maintainance of the existing code has been impeded by the fact that it's bash code, and we'd have to spend time writing the go code to get feature parity.

[gbraad]

It might be more suited to refactor some of the shell script or even explore other options?

[anjannath]

Maybe in python? and not Go, one thing i'd like in the scripts is to perform only a set of things ((like run only the cert rotation part after snc has finished(only one example, though)) and not have to run the whole script, right now i manually edit the script to achieve this, (not sure if there are better ways) So having some kind of command/switch for this would be helpful imo.

[guillaumerose]

With Go, I would hope that we can reuse some code in crc (preflights for instance) and possibly vendor some interesting code (installer, some openshift types).

[gbraad]

This nullifies: https://github.com/code-ready/snc/issues/156, but should be considered. Also, how does this work for the stuff that @mtarsel did for PPP64LE? We might need some golang integration with libvirt in that case

[guillaumerose]

The transition will not happen in one day so I guess running shellcheck for time to time is still valuable. It should be straightforward in the CI with the help of a container.

For ppp64le, I would say that we will stick with virsh still a long time. The go library doesn't bring much (no timeout support, still have to parse XML) and everything is already written with virsh commands.

The value I see in go is in the error handling and more clarity in the code: no global variable everywhere, little functions, reuse of k8s structs.

The transition can be really easy and can start small:

• rename snc.sh -> installer.sh
• create a new snc.sh script that compiles a small go program that will run installer.sh (be sure to add a subcommand for it).
• starting by the tail of the shell script, move the code to go.
• starting by the top, inject the correct env variable to the remaining shell script.
• repeat with createdisk.sh

[gbraad]

create a new snc.sh script that compiles a small go program that will run installer.sh (be sure to add a subcommand for it).

which means to use subcommands on the script. This is pretty much what we did for the older OSP4 PoC. So this would look like:

$ snc setup   # make sure requirements exist
$ snc install   # invoke needed commands
#    installer.sh start
#    installer.sh ...
$ snc bundle ./output.crcbundle
$ snc delete

for example?

[guillaumerose]

Yes it seems reasonable

[gbraad]

would be ideal if some of the commands or patterns can be shared between both crc and snc. One of the requests has been to allow the export/bundling of a 'modified' image for reuse by other users, like for instance in a team. This might help with that.

[mtarsel]

There should not be any specific ppc64le or multi-arch issues with the move to go lang. We use go in our CI for openshift on ppc64le. Here is my opinion on this...

From @guillaumerose I somewhat agree with this:

The value I see in go is in the error handling and more clarity in the code: no global variable everywhere, little functions, reuse of k8s structs.

The global variables in bash can get messy and the error handling could be better in this bash script however there could be a refactor of the current scripts to make it more modular (easier to read), and have better error handling.

From the ppc64le perspective, I really like this use case "flow" from @gbraad comment above: https://github.com/code-ready/snc/issues/210#issuecomment-708997371 (setup, install, bundle, delete)

I think implementing this in bash would provide more clarity of the code and possibly reduce number of global variables. Using separate scripts could also help with error handling. A little similar to that OSP4 PoC, there could be snc-setup.sh, snc-install.sh, snc-bundle.sh and then snc-destroy.sh . These scripts could then just be sym-linked to /usr/bin so a user can do just that:

$ snc setup   # make sure requirements exist
$ snc install   # invoke needed commands
$ snc bundle ./output.crcbundle
$ snc delete

So that being said, I like that the current state of snc because it does not have a lot of dependencies on other libraries and there is no need to install special rpms (besides one-time setup). I am not completely against the move to golang. @guillaumerose idea in https://github.com/code-ready/snc/issues/210#issuecomment-708947510 makes sense to me. I am just proposing a bash refactor to make the project more modular which can resolve some of the issues mentioned here and might be less effort than changing to a new language. Maybe after that bash refactor there could be another evaluation of a move to go or not 🤷

[gbraad]

I think a batch refactor can (and should) be the first step.

As proposed, external script invocation would likely be an intermediate necessity anyway.

[cfergeau]

There is a lack of experience with bash in the team, which leads to this suboptimal error handling, maintainance difficulties, ... Dunno how much we'd manage to improve it by ourselves. @mtarsel we are definitely welcoming any contributions towards improving the bash we are writing :) (though better to discuss big refactors before spending a lot of time on it)

[praveenkumar]

We have done quite of refactor and have better error handling so I think we are still good with these bash script.

Will close this after having discussion with team.