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
Remove kubectl log #78098
Remove kubectl log #78098
Conversation
/sig cli |
@@ -1233,7 +1233,7 @@ metadata: | |||
Testname: Kubectl, logs | |||
Description: When a Pod is running then it MUST generate logs. | |||
Starting a Pod should have a log line indicating the server is running and ready to accept connections. Also log command options MUST work as expected and described below. | |||
‘kubectl log -tail=1’ should generate a output of one line, the last line in the log. | |||
‘kubectl logs -tail=1’ should generate a output of one line, the last line in the log. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is still running framework.RunKubectlOrDie("log"...
below.
@kubernetes/cncf-conformance-wg what's the protocol for removing a deprecated thing used by a conformance test? change the test in 1.15 and remove the thing in 1.16?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, we can just change the test now and remove the thing. Conformance tests evaluate whether the subject cluster conforms. If it can't handle the new command, it's not really conforming. If it can't handle the old one, and it is within the deprecation period (the CLI deprecation policy is the longer of 12 months or 2 releases (https://kubernetes.io/docs/reference/using-api/deprecation-policy/#deprecating-a-flag-or-cli)), then it wouldn't be considered conforming either. But 1) it's been more than 3 years so we are safely beyond that 12 months; and 2) this test isn't about backwards compat.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree the command is within rights to be removed.
I just wasn't sure if 1.14 conformance tests needed to be able to run against 1.15 clusters.
@soltysh sounds like we can drop this, just need to fix the invocations from the e2e tests |
/retest |
/lgtm |
ping...this is approved but now needs rebase |
/hold cancel |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: liggitt, 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 |
/lgtm |
/lgtm |
@sallyom: changing LGTM is restricted to collaborators 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. |
This PR may require API review. If so, when the changes are ready, complete the pre-review checklist and request an API review. Status of requested reviews is tracked in the API Review project. |
Remove kubectl log Kubernetes-commit: 3cf2d32
What type of PR is this?
/kind api-change
/kind cleanup
What this PR does / why we need it:
kubectl log
was deprecated back in cb872e9 which is even before first release. I think we are now ready to finally remove that command.Special notes for your reviewer:
/assign @liggitt
Does this PR introduce a user-facing change?: