Remove long/golang version information making short the default #116720
+12
−33
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
k8s-ci-robot
added
size/M
k8s-ci-robot
added
kind/cleanup
|
/triage accepted |
k8s-ci-robot
added
do-not-merge/hold
k8s-ci-robot
added
area/kubectl
area/test
sig/testing
|
|
+1 to doing this in 1.28, but the test failures look legit |
test/cmd/version.sh
Outdated
| @@ -32,8 +32,8 @@ run_kubectl_version_tests() { | |||
|
|
|||
| # create version files, one for the client, one for the server. | |||
| # these are the files we will use to ensure that the remainder output is correct | |||
| kube::test::version::object_to_file "Client" "" "${TEMP}/client_version_test" | |||
| kube::test::version::object_to_file "Server" "" "${TEMP}/server_version_test" | |||
| kube::test::version::json_client_server_object_to_file "" "clientVersion.gitVersion" "${TEMP}/client_version_test" | |||
There was a problem hiding this comment.
client_version_test and server_version_test now just contain a version number, whereas before they contained all the various fields parsed out of the long version output.
What if you leave these two lines (35 and 36) unchanged and change lines 46-56, to use .gitVersion like this:
kube::log::status "Testing kubectl version: verify json output"
kube::test::version::json_client_server_object_to_file "" "clientVersion.gitVersion" "${TEMP}/client_json_version_test"
kube::test::version::json_client_server_object_to_file "" "serverVersion.gitVersion" "${TEMP}/server_json_version_test"
kube::test::version::diff_assert "${TEMP}/client_version_test" "eq" "${TEMP}/client_json_version_test" "--output json has correct client info"
kube::test::version::diff_assert "${TEMP}/server_version_test" "eq" "${TEMP}/server_json_version_test" "--output json has correct server info"
kube::log::status "Testing kubectl version: verify json output using additional --client flag does not contain serverVersion"
kube::test::version::json_client_server_object_to_file "--client" "clientVersion.gitVersion" "${TEMP}/client_only_json_version_test"
kube::test::version::json_client_server_object_to_file "--client" "serverVersion.gitVersion" "${TEMP}/server_client_only_json_version_test"
kube::test::version::diff_assert "${TEMP}/client_version_test" "eq" "${TEMP}/client_only_json_version_test" "--client --output json has correct client info"
kube::test::version::diff_assert "${TEMP}/server_version_test" "ne" "${TEMP}/server_client_only_json_version_test" "--client --output json has no server info"
Does that fix the integration tests?
There was a problem hiding this comment.
The suggestion is reasonable, but that makes us compare the same thing against each other, which I believe is not the point of this tests. Lemme see what's needs fixing there and I'll try to push an updated version in a few.
You're right, I read your suggestion the other way around 🙃
|
/test integration |
|
@soltysh: The specified target(s) for
The following commands are available to trigger optional jobs:
Use
In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
|
/test pull-kubernetes-integration |
soltysh
force-pushed
the
remove_short_version
branch
from
316407c to
b99f401
Compare
@moficodes yes it's targeted at 1.28 |
|
/hold cancel |
k8s-ci-robot
removed
the
do-not-merge/hold
|
@soltysh hey! Can you please take a look at failing tests to get this moving? FYI, the code freeze is starting 01:00 UTC Wednesday 19th July 2023 / 18:00 PDT Tuesday 18th July 2023 (about less than 2 weeks from now). |
soltysh
force-pushed
the
remove_short_version
branch
from
b99f401 to
3f07fc3
Compare
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: soltysh The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
ardaguclu
reviewed
Jul 10, 2023
| @@ -32,7 +32,7 @@ run_kubectl_version_tests() { | |||
|
|
|||
| # create version files, one for the client, one for the server. | |||
| # these are the files we will use to ensure that the remainder output is correct | |||
| kube::test::version::object_to_file "Client" "" "${TEMP}/client_version_test" | |||
| kube::test::version::object_to_file "Client" "" "${TEMP}/client_version_test" | |||
k8s-ci-robot
added
the
lgtm
|
LGTM label has been added. Git tree hash: 8b291b42c871f499b185503e68134487ab9ec24f
|
10 tasks
5 tasks
5 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
What type of PR is this?
/sig cli
/kind cleanup
/kind deprecation
/milestone v1.28
What this PR does / why we need it:
This is followup to #108987 where we've announced deprecation of the golang-based short version information in favor of making the
--shortthe new default, and structured output being available through--output json|yaml.Special notes for your reviewer:
/assign @KnVerey @sftim
Does this PR introduce a user-facing change?