This repository has been archived by the owner on Feb 8, 2024. It is now read-only.
CORTX-28885: Remove yq binary from project
#240
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Users are now expected to ensure `yq` is installed. This means CORTX on Kubernetes deployment scripts can be run on more platforms. Signed-off-by: Keith Pine <[email protected]>
keithpine
added
the
breaking-change
osowski
approved these changes
May 12, 2022
There was a problem hiding this comment.
This looks good to me. We'll want to get @shailesh-vaidya a heads up on this change, as well as hit it with some of @walterlopatka regression test scripts, but that should all be good from what I can see.
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
This changes removes the bundled
yqcommand line utility from the Git repository. Users are now expected to install a minimum version of the software themselves. Version 4.25.1.One problem with the bundled yq is that it was specific to the Linux amd64 platform. This means the deployment scripts could not be run on MacOS or Linux arm installations.
Another problem is that Git does not support binaries well. Updating the version of yq increases the project sizes dramatically.
Breaking change
Deploying with the
deploy-cortx-cloud.shscript will now fail if yq is not installed (and is not accessible from$PATH).The minimum version of yq is 4.25.1 (currently the latest). A version lower than that will produce an error causing the deployment to fail. Be sure to upgrade to this version.
Type of change
Applicable issues
With CORTX-30141 we'll be relying more on the native capabilities of yq to parse the solution YAML file. See #239 for example.
How was this tested?
Deployed, un-deployed, started, and stopped a cluster. Ran utility scripts.
Tested error conditions with invalid version and missing yq.
Additional information
I actually reduced dependency on most scripts of yq, except for deploy, for now. I'd expect we'll want to actually take advantage of yq (since it's already required) and just remove the custom yaml parsing.
Completely removed the yaml parsing scripts, as any use of them has been removed, or some were already unused.
I updated the README.md file to mention yq as a requirement. I took the liberty of re-organizing the pre-reqs section.
Fixed some markdown lint warnings along the way.
Checklist
If this change addresses a CORTX Jira issue:
CORTX-XXXXX:)View rendered README.md