Skip to content

Fix for bundle version in the installer CLI#449

Open
georgievaVMW wants to merge 1 commit intovmware-tanzu:mainfrom
georgievaVMW:hai-cli-fix
Open

Fix for bundle version in the installer CLI#449
georgievaVMW wants to merge 1 commit intovmware-tanzu:mainfrom
georgievaVMW:hai-cli-fix

Conversation

@georgievaVMW
Copy link
Copy Markdown
Contributor

Made small changes so the CLI shows correct bundle version.
Made a map from k8s filter to k8s bundle (this is in order to easily output correct bundle for example 1.23.* -> 1.22)
Made changes to installer.go and the registry tests to accommodate the added second argument to the function.

Made a map from k8s filter to k8s bundle (this is in order to easily output correct bundle for example 1.23.* -> 1.22)
Made changes to installer.go and the registry tests to accomodate the added second argument to the function.
@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Mar 22, 2022

Codecov Report

❌ Patch coverage is 80.00000% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 68.39%. Comparing base (cc605cf) to head (9db95f3).
⚠️ Report is 149 commits behind head on main.

Files with missing lines Patch % Lines
agent/installer/registry.go 71.42% 2 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #449      +/-   ##
==========================================
- Coverage   68.40%   68.39%   -0.01%     
==========================================
  Files          23       23              
  Lines        1728     1734       +6     
==========================================
+ Hits         1182     1186       +4     
- Misses        474      476       +2     
  Partials       72       72              
Files with missing lines Coverage Δ
agent/installer/installer.go 78.10% <100.00%> (ø)
agent/installer/registry.go 96.66% <71.42%> (-3.34%) ⬇️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@jeuxdemains
Copy link
Copy Markdown
Contributor

A note on why we switched from filter array to filter map.
We had a discussion with Alex about this update and the path he can take to implement it.

Initially the installer was meant to use a simple container (array) to hold the filters and that was the right choice at the time.
However, the CLI require certain level of predictability in order to perform as expected (to print the supported versions) which is not possible without predefining to which K8s versions the filters match.

We discussed different solutions like stripping the filter masks (the dot asterisk part) from the string of the help of the CLI, as well as extracting the map to separate module which could be shared between modules (CLI and Installer).
Ultimately we concluded integrating it to the Installer as a map would cost less time and be better prepared for the future, requiring less portions of the code to be maintained.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants