guild-operators
guild-operators copied to clipboard
cardano-community
[Mithril] - Add version check if update_check is set to 'Y'
Describe the bug
Mithril versions need to be updated - especially post a release of node release, and can easily be missed given that not everyone would use guild-deploy scripts to perform updates. Hence, the scripts themselves should be able to check the latest version of mithril from github (if UPDATE_CHECK is set to 'Y') and prompt for update - similar to cncli.sh
What is the current logic to avoid introducing breaking changes, to set UPDATE_CHECK default to N, or do the existing update checks have a separate method to avoid updating the release binaries to an incompatible version with the current node, introducing breaking changes?
I did a cursory review after work today of env, cntools.sh and cntools.library it appears UPDATE_CHECK and checkUpdate are specific to the scripts and not any third party release binaries. I thought there might be similar updates for other binaries in the previous question.
At the moment nothing else comes to mind to prevent breaking changes from Mithril release binaries intended only for the latest node version. I'll put something together to update binaries from the scripts without guild-deoloy.sh and consider if other options exist besides setting UPDATE_CHECK to default to N.
@rdlrt
I communicated with the Mithril developers on the topic of avoiding breaking changes in input-output-hk/mithril discussion #1606 . A method should be implemented in input-output-hk/mithril issue #1615 to provide a simple way to avoid updates with breaking changes, or allow the SPO to update anyway once they understand it will lead to breaking changes between the updated Mithril binaries and the installed Cardano binaries.
After that I'll implement the logic requested in this issue.
Describe the bug
Mithril versions need to be updated - especially post a release of node release, and can easily be missed given that not everyone would use guild-deploy scripts to perform updates. Hence, the scripts themselves should be able to check the latest version of mithril from github (if UPDATE_CHECK is set to 'Y') and prompt for update - similar to cncli.sh
@rdlrt
After further review I'm a bit confused by this issues request. I checked the referenced cncli.sh script, it's cncliInit and logic for when UPDATE_CHECK is set to 'Y'. I've also reviewed the checkUpdate function inherited by cncli.sh from env. I don't see any logic where cncli.sh updates its own binary. I only see it updating the script itself, similar to other scripts, but no method for cncli.sh to actualy update the binary cncli it wraps.
From what I can tell if cardano node updates and creates a breaking change between the node version and cncli binary version the SPO would still be required to use guild-deploy.sh to update the CNCLI binary regardless if they update the cncli.sh script with its built in checks.
I'm not against building something new for Mithril to handle this outside of guild-deploy.sh, but after review I do not see a method an SPO would use to update cncli, cardano-node, or any other binary without relying on guild-deploy.sh, please advise.
After further review I'm a bit confused by this issues request. I checked the referenced cncli.sh script, it's cncliInit and logic for when UPDATE_CHECK is set to 'Y'. I've also reviewed the checkUpdate function inherited by cncli.sh from env. I don't see any logic where cncli.sh updates its own binary. I only see it updating the script itself, similar to other scripts, but no method for cncli.sh to actualy update the binary cncli it wraps.
I was referring to cncli version check (indeed it is in gLiveView.sh not cncli.sh itself) here.
@rdlrt I think the majority of this request was handled in #1761. The part which may need additional testing is the ensuring the minimum node version required for mithril against the currently installed node version, to avoid version incompatibility.
I'm tempted to close this issue as resolved, as the original issue description I believe was achieved already. If this is closed and I find that minimum version compatibility is not working as intended I will open a new issue for tracking and a PR to resolve.