Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(kumactl) Only warn about version compatibility where it makes sense #2828

Merged
merged 2 commits into from
Sep 24, 2021

Conversation

michaelbeaumont
Copy link
Contributor

@michaelbeaumont michaelbeaumont commented Sep 23, 2021

Summary

fix(kumactl) Only warn about version compatibility where it makes sense

Warn for: apply, delete, generate, get, inspect
Don't warn for: completion, config, install, uninstall, version

Full changelog

  • Only warn about version compatibility where it makes sense

Testing

  • Unit tests
  • E2E tests
  • Manual testing on Universal
  • Manual testing on Kubernetes

Backwards compatibility

  • Add backport-to-stable label if the code is backwards compatible. Otherwise, list breaking changes.

@michaelbeaumont michaelbeaumont force-pushed the fix/version_warning branch 4 times, most recently from 081d4bb to 90f74a9 Compare September 23, 2021 19:02
@codecov-commenter
Copy link

codecov-commenter commented Sep 23, 2021

Codecov Report

Merging #2828 (667c3d2) into master (1333e7c) will increase coverage by 0.03%.
The diff coverage is 65.71%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2828      +/-   ##
==========================================
+ Coverage   52.01%   52.04%   +0.03%     
==========================================
  Files         882      883       +1     
  Lines       51321    51358      +37     
==========================================
+ Hits        26694    26731      +37     
+ Misses      22514    22495      -19     
- Partials     2113     2132      +19     
Impacted Files Coverage Δ
app/kumactl/cmd/apply/apply.go 74.73% <0.00%> (-1.61%) ⬇️
app/kumactl/cmd/delete/delete.go 82.92% <0.00%> (-4.26%) ⬇️
app/kumactl/cmd/root.go 76.59% <ø> (+2.45%) ⬆️
app/kumactl/pkg/cmd/util.go 60.00% <60.00%> (ø)
app/kumactl/cmd/generate/generate.go 73.33% <63.63%> (-26.67%) ⬇️
app/kumactl/cmd/get/get.go 80.00% <63.63%> (-20.00%) ⬇️
app/kumactl/cmd/inspect/inspect.go 77.77% <71.42%> (-22.23%) ⬇️
app/kumactl/pkg/cmd/root_context.go 71.13% <73.33%> (+0.40%) ⬆️
app/kumactl/cmd/config/config.go 85.71% <80.00%> (-14.29%) ⬇️
pkg/plugins/leader/postgres/leader_elector.go 87.23% <0.00%> (-10.64%) ⬇️
... and 7 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1333e7c...667c3d2. Read the comment docs.

Also remove logging with the k8s logger

Signed-off-by: Michael Beaumont <mjboamail@gmail.com>
Warn for: apply, delete, generate, get, inspect
Don't warn for: completion, config, install, uninstall, version

Signed-off-by: Michael Beaumont <mjboamail@gmail.com>
@michaelbeaumont michaelbeaumont marked this pull request as ready for review September 24, 2021 11:32
@michaelbeaumont michaelbeaumont requested a review from a team as a code owner September 24, 2021 11:32
@jakubdyszkiewicz
Copy link
Contributor

Nice, that's much better. I'm a bit hesitant whether to include checks in version command or not. I think it makes sense to improve kumactl version to provide something like

kumactl version: XYZ
kuma-cp version: ZYX

like it was suggested on the planning. That's a separate PR, but since we don't have a warning on kumactl version, it would be great to have server version explicit in kumactl

@jakubdyszkiewicz
Copy link
Contributor

(please make sure to backport it)

@michaelbeaumont
Copy link
Contributor Author

Nice, that's much better. I'm a bit hesitant whether to include checks in version command or not. I think it makes sense to improve kumactl version to provide something like

kumactl version: XYZ
kuma-cp version: ZYX

like it was suggested on the planning. That's a separate PR, but since we don't have a warning on kumactl version, it would be great to have server version explicit in kumactl

Completely agree, I'm going to add this.

@michaelbeaumont michaelbeaumont merged commit 4bf896a into kumahq:master Sep 24, 2021
@michaelbeaumont michaelbeaumont deleted the fix/version_warning branch September 24, 2021 14:07
mergify bot pushed a commit that referenced this pull request Sep 24, 2021
…se (#2828)

* fix(kumactl) Add method to check server version compatibility

Also remove logging with the k8s logger

Signed-off-by: Michael Beaumont <mjboamail@gmail.com>

* fix(kumactl) Only warn about version compatibility where it makes sense

Warn for: apply, delete, generate, get, inspect
Don't warn for: completion, config, install, uninstall, version

Signed-off-by: Michael Beaumont <mjboamail@gmail.com>
(cherry picked from commit 4bf896a)
michaelbeaumont added a commit that referenced this pull request Sep 24, 2021
…se (#2828)

* fix(kumactl) Add method to check server version compatibility

Also remove logging with the k8s logger

Signed-off-by: Michael Beaumont <mjboamail@gmail.com>

* fix(kumactl) Only warn about version compatibility where it makes sense

Warn for: apply, delete, generate, get, inspect
Don't warn for: completion, config, install, uninstall, version

Signed-off-by: Michael Beaumont <mjboamail@gmail.com>
(cherry picked from commit 4bf896a)

Signed-off-by: Mike <2266568+michaelbeaumont@users.noreply.github.com>
michaelbeaumont added a commit that referenced this pull request Sep 26, 2021
…se (#2828) (#2833)

* fix(kumactl) Add method to check server version compatibility

Also remove logging with the k8s logger

Signed-off-by: Michael Beaumont <mjboamail@gmail.com>

* fix(kumactl) Only warn about version compatibility where it makes sense

Warn for: apply, delete, generate, get, inspect
Don't warn for: completion, config, install, uninstall, version

Signed-off-by: Michael Beaumont <mjboamail@gmail.com>
(cherry picked from commit 4bf896a)

Signed-off-by: Mike <2266568+michaelbeaumont@users.noreply.github.com>

Co-authored-by: Mike <2266568+michaelbeaumont@users.noreply.github.com>
nikita15p pushed a commit to nikita15p/kuma that referenced this pull request Sep 28, 2021
…se (kumahq#2828)

* fix(kumactl) Add method to check server version compatibility

Also remove logging with the k8s logger

Signed-off-by: Michael Beaumont <mjboamail@gmail.com>

* fix(kumactl) Only warn about version compatibility where it makes sense

Warn for: apply, delete, generate, get, inspect
Don't warn for: completion, config, install, uninstall, version

Signed-off-by: Michael Beaumont <mjboamail@gmail.com>
nikita15p pushed a commit to nikita15p/kuma that referenced this pull request Sep 28, 2021
…se (kumahq#2828)

* fix(kumactl) Add method to check server version compatibility

Also remove logging with the k8s logger

Signed-off-by: Michael Beaumont <mjboamail@gmail.com>

* fix(kumactl) Only warn about version compatibility where it makes sense

Warn for: apply, delete, generate, get, inspect
Don't warn for: completion, config, install, uninstall, version

Signed-off-by: Michael Beaumont <mjboamail@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants