fix(kumactl) warn version unknown #2438

parkanzky · 2021-07-27T01:34:15Z

Summary

Added warning in case where server version cannot be retrieved.

This required to update every kumactl test, since they all depended upon there being no kuma-cp on localhost and the resulting connection-refused from the server, on every command, being ignored.

So, added a mock client which always returns a given version, or defaults to current version if none is passed in.

Full changelog

Fix silent ignore of missing server version

Issues resolved

#2400

Documentation

Link to the website documentation PR

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.

Signed-off-by: Paul Parkanzky <paul.parkanzky@konghq.com>

jpeach

Does this PR update #2400?

jpeach · 2021-07-27T03:50:43Z

app/kumactl/cmd/root.go

-				kumaBuildVersion, _ = client.GetVersion(context.Background())
+				kumaBuildVersion, err = client.GetVersion(context.Background())
+				if err != nil {
+					kumactlLog.Error(err, "Unable to retrieve server version")


If we log an error here, we immediately print the same error below, can we omit one of these?

I think we should remove this log altogether. It won't be visible at all. We don't use logging in kumactl to not disrupt stdout.

app/kumactl/cmd/root.go

app/kumactl/cmd/apply/apply_test.go

pkg/util/test/mock_api_client.go

Signed-off-by: Bart Smykla <bartek@smykla.com>

also sorted imports in some of the tests Signed-off-by: Bart Smykla <bartek@smykla.com>

Signed-off-by: Bart Smykla <bartek@smykla.com>

codecov-commenter · 2021-07-27T09:22:50Z

Codecov Report

Merging #2438 (7f0cb83) into master (7118e27) will increase coverage by 0.17%.
The diff coverage is 85.71%.

@@            Coverage Diff             @@
##           master    #2438      +/-   ##
==========================================
+ Coverage   52.41%   52.59%   +0.17%     
==========================================
  Files         875      878       +3     
  Lines       47833    47874      +41     
==========================================
+ Hits        25072    25179     +107     
+ Misses      20722    20643      -79     
- Partials     2039     2052      +13

Impacted Files	Coverage Δ
app/kumactl/cmd/root.go	`73.68% <50.00%> (-5.57%)`	⬇️
pkg/util/test/mock_api_client.go	`100.00% <100.00%> (ø)`
app/kumactl/pkg/resources/apiserver_client.go	`35.00% <0.00%> (-45.00%)`	⬇️
app/kumactl/pkg/client/api_server_client.go	`66.66% <0.00%> (-16.67%)`	⬇️
app/kumactl/pkg/resources/request.go	`50.00% <0.00%> (-14.29%)`	⬇️
pkg/util/http/tls.go	`70.00% <0.00%> (-10.00%)`	⬇️
pkg/core/resources/manager/cache.go	`81.81% <0.00%> (-2.60%)`	⬇️
api/internal/util/proto/types.go	`100.00% <0.00%> (ø)`
... and 11 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 7118e27...7f0cb83. Read the comment docs.

parkanzky · 2021-07-27T14:45:56Z

Does this PR update #2400?

Yes it does. Will link.

* fix(kumactl) Warn when server version cannot be retrieved Signed-off-by: Paul Parkanzky <paul.parkanzky@konghq.com> * fix(kumactl) Warn when server version cannot be retrieved Signed-off-by: Paul Parkanzky <paul.parkanzky@konghq.com> * fix(kumactl) Add mock client Signed-off-by: Paul Parkanzky <paul.parkanzky@konghq.com> * fix(kumactl) make check Signed-off-by: Paul Parkanzky <paul.parkanzky@konghq.com> * chore(*) address review comments Signed-off-by: Bart Smykla <bartek@smykla.com> * chore(*) make check results Signed-off-by: Bart Smykla <bartek@smykla.com> * chore(*) update config cp add tests Signed-off-by: Bart Smykla <bartek@smykla.com> * chore(*) rename import kuma_test to default also sorted imports in some of the tests Signed-off-by: Bart Smykla <bartek@smykla.com> * chore(*) fix config cp kumactl remove tests Signed-off-by: Bart Smykla <bartek@smykla.com> Co-authored-by: Bart Smykla <bartek@smykla.com> Signed-off-by: Paul Parkanzky <paul.parkanzky@konghq.com> (cherry picked from commit 755ccf8)

* fix(kumactl) Warn when server version cannot be retrieved Signed-off-by: Paul Parkanzky <paul.parkanzky@konghq.com> * fix(kumactl) Warn when server version cannot be retrieved Signed-off-by: Paul Parkanzky <paul.parkanzky@konghq.com> * fix(kumactl) Add mock client Signed-off-by: Paul Parkanzky <paul.parkanzky@konghq.com> * fix(kumactl) make check Signed-off-by: Paul Parkanzky <paul.parkanzky@konghq.com> * chore(*) address review comments Signed-off-by: Bart Smykla <bartek@smykla.com> * chore(*) make check results Signed-off-by: Bart Smykla <bartek@smykla.com> * chore(*) update config cp add tests Signed-off-by: Bart Smykla <bartek@smykla.com> * chore(*) rename import kuma_test to default also sorted imports in some of the tests Signed-off-by: Bart Smykla <bartek@smykla.com> * chore(*) fix config cp kumactl remove tests Signed-off-by: Bart Smykla <bartek@smykla.com> Co-authored-by: Bart Smykla <bartek@smykla.com> Signed-off-by: Paul Parkanzky <paul.parkanzky@konghq.com> (cherry picked from commit 755ccf8) Signed-off-by: Paul Parkanzky <paul.parkanzky@konghq.com>

* fix(kumactl) Warn when server version cannot be retrieved Signed-off-by: Paul Parkanzky <paul.parkanzky@konghq.com> * fix(kumactl) Warn when server version cannot be retrieved Signed-off-by: Paul Parkanzky <paul.parkanzky@konghq.com> * fix(kumactl) Add mock client Signed-off-by: Paul Parkanzky <paul.parkanzky@konghq.com> * fix(kumactl) make check Signed-off-by: Paul Parkanzky <paul.parkanzky@konghq.com> * chore(*) address review comments Signed-off-by: Bart Smykla <bartek@smykla.com> * chore(*) make check results Signed-off-by: Bart Smykla <bartek@smykla.com> * chore(*) update config cp add tests Signed-off-by: Bart Smykla <bartek@smykla.com> * chore(*) rename import kuma_test to default also sorted imports in some of the tests Signed-off-by: Bart Smykla <bartek@smykla.com> * chore(*) fix config cp kumactl remove tests Signed-off-by: Bart Smykla <bartek@smykla.com> Co-authored-by: Bart Smykla <bartek@smykla.com> Signed-off-by: Paul Parkanzky <paul.parkanzky@konghq.com> (cherry picked from commit 755ccf8) Signed-off-by: Paul Parkanzky <paul.parkanzky@konghq.com> Co-authored-by: parkanzky <42279121+parkanzky@users.noreply.github.com> Co-authored-by: Bart Smykla <bartek@smykla.com> Signed-off-by: Paul Parkanzky <paul.parkanzky@konghq.com>

This test failure is a result of the combination of #2419 and #2438. The fix is to test for error output using a substring match, which was already done in most places in #2438, but this instance was overlooked. Signed-off-by: James Peach <james.peach@konghq.com>

This test failure is a result of the combination of #2419 and #2438. The fix is to test for error output using a substring match, which was already done in most places in #2438, but this instance was overlooked. Signed-off-by: James Peach <james.peach@konghq.com> (cherry picked from commit 10d4926)

This test failure is a result of the combination of #2419 and #2438. The fix is to test for error output using a substring match, which was already done in most places in #2438, but this instance was overlooked. Signed-off-by: James Peach <james.peach@konghq.com> (cherry picked from commit 10d4926) Co-authored-by: James Peach <james.peach@konghq.com>

Paul Parkanzky added 2 commits July 26, 2021 13:53

fix(kumactl) Warn when server version cannot be retrieved

3b616ea

Signed-off-by: Paul Parkanzky <paul.parkanzky@konghq.com>

fix(kumactl) Warn when server version cannot be retrieved

543dfad

Signed-off-by: Paul Parkanzky <paul.parkanzky@konghq.com>

parkanzky added the backport-to-stable label Jul 27, 2021

parkanzky requested a review from a team as a code owner July 27, 2021 01:34

Paul Parkanzky added 2 commits July 26, 2021 21:35

fix(kumactl) Add mock client

5418bfe

Signed-off-by: Paul Parkanzky <paul.parkanzky@konghq.com>

fix(kumactl) make check

bda6c0a

Signed-off-by: Paul Parkanzky <paul.parkanzky@konghq.com>

jpeach reviewed Jul 27, 2021

View reviewed changes

chore(*) address review comments

6fa7a88

Signed-off-by: Bart Smykla <bartek@smykla.com>

bartsmykla changed the title ~~Fix/kumactl warn version unknown~~ fix(kumactl) warn version unknown Jul 27, 2021

bartsmykla added 4 commits July 27, 2021 09:16

chore(*) make check results

3cef572

Signed-off-by: Bart Smykla <bartek@smykla.com>

chore(*) update config cp add tests

72cf25e

Signed-off-by: Bart Smykla <bartek@smykla.com>

chore(*) rename import kuma_test to default

5afead1

also sorted imports in some of the tests Signed-off-by: Bart Smykla <bartek@smykla.com>

chore(*) fix config cp kumactl remove tests

7f0cb83

Signed-off-by: Bart Smykla <bartek@smykla.com>

jakubdyszkiewicz approved these changes Jul 27, 2021

View reviewed changes

parkanzky merged commit 755ccf8 into master Jul 27, 2021

parkanzky deleted the fix/kumactl-warn-version-unknown branch July 27, 2021 17:27

mergify bot mentioned this pull request Jul 27, 2021

fix(kumactl) warn version unknown (backport #2438) #2442

Merged

jpeach mentioned this pull request Jul 29, 2021

fix(ci) kumactl test output matching #2446

Merged

5 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(kumactl) warn version unknown #2438

fix(kumactl) warn version unknown #2438

parkanzky commented Jul 27, 2021 •

edited by subnetmarco

Loading

jpeach left a comment

jpeach Jul 27, 2021

jakubdyszkiewicz Jul 27, 2021

codecov-commenter commented Jul 27, 2021

parkanzky commented Jul 27, 2021

fix(kumactl) warn version unknown #2438

fix(kumactl) warn version unknown #2438

Conversation

parkanzky commented Jul 27, 2021 • edited by subnetmarco Loading

Summary

Full changelog

Issues resolved

Documentation

Testing

Backwards compatibility

jpeach left a comment

Choose a reason for hiding this comment

jpeach Jul 27, 2021

Choose a reason for hiding this comment

jakubdyszkiewicz Jul 27, 2021

Choose a reason for hiding this comment

codecov-commenter commented Jul 27, 2021

Codecov Report

parkanzky commented Jul 27, 2021

parkanzky commented Jul 27, 2021 •

edited by subnetmarco

Loading