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

logs: remove deprecated klog flags #112120

Merged
merged 2 commits into from Sep 21, 2022

Conversation

pohly
Copy link
Contributor

@pohly pohly commented Aug 30, 2022

What type of PR is this?

/kind cleanup
/kind api-change

What this PR does / why we need it:

This completes the deprecation of klog flags which are no longer supported.
klog itself continues to support them, but Kubernetes components don't. This
makes the command line interfaces simpler and reduces the attack surface
because less functionality is exposed.

Which issue(s) this PR fixes:

Fixes #kubernetes/enhancements#2845

Special notes for your reviewer:

For example, kube-controller-manager now has:

Logs flags:

  --log-flush-frequency duration
            Maximum number of seconds between log flushes (default 5s)
  --log-json-info-buffer-size quantity
            [Alpha] In JSON format with split output streams, the info messages can be buffered for a while to increase performance. The default value of zero
            bytes disables buffering. The size can be specified as number of bytes (512), multiples of 1000 (1K), multiples of 1024 (2Ki), or powers of those (3M,
            4G, 5Mi, 6Gi). Enable the LoggingAlphaOptions feature gate to use this.
  --log-json-split-stream
            [Alpha] In JSON format, write error messages to stderr and info messages to stdout. The default is to write a single stream to stdout. Enable the
            LoggingAlphaOptions feature gate to use this.
  --logging-format string
            Sets the log format. Permitted formats: "json" (gated by LoggingBetaOptions), "text".
   -v, --v Level
            number for the log level verbosity
  --vmodule pattern=N,...
            comma-separated list of pattern=N settings for file-filtered logging (only works for text log format)

Misc flags:

  --kubeconfig string
            Path to kubeconfig file with authorization and master location information.
  --master string
            The address of the Kubernetes API server (overrides any value in kubeconfig).

Global flags:

 -h, --help
            help for kube-controller-manager
  --version version[=true]
            Print version information and quit

For details see
https://github.com/kubernetes/enhancements/tree/master/keps/sig-instrumentation/2845-deprecate-klog-specific-flags-in-k8s-components

Does this PR introduce a user-facing change?

Legacy klog flags are no longer available. Only `-v` and `-vmodule` are still supported.

Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:

- [KEP]: https://github.com/kubernetes/enhancements/issues/2845
- [Usage]: https://github.com/kubernetes/enhancements/blob/master/keps/sig-instrumentation/2845-deprecate-klog-specific-flags-in-k8s-components/README.md#user-stories

/assign @serathius

@k8s-ci-robot k8s-ci-robot added release-note Denotes a PR that will be considered when it comes time to generate release notes. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. kind/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. do-not-merge/needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. needs-priority Indicates a PR lacks a `priority/foo` label and requires one. labels Aug 30, 2022
@k8s-ci-robot k8s-ci-robot added sig/architecture Categorizes an issue or PR as relevant to SIG Architecture. sig/instrumentation Categorizes an issue or PR as relevant to SIG Instrumentation. wg/structured-logging Categorizes an issue or PR as relevant to WG Structured Logging. and removed do-not-merge/needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. labels Aug 30, 2022
@pohly
Copy link
Contributor Author

pohly commented Aug 30, 2022

@serathius: do we still want to have this line here?

Non-default formats don't honor these flags: --vmodule.

It only lists one flag, and that flag itself now says "only works for text log format".

@serathius
Copy link
Contributor

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Aug 30, 2022
@pohly
Copy link
Contributor Author

pohly commented Aug 30, 2022

/hold

I forgot to mention that kubernetes/test-infra#27305 needs to be merged first.

@k8s-ci-robot k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Aug 30, 2022
@pohly
Copy link
Contributor Author

pohly commented Aug 30, 2022

@serathius so do you want me to remove "Non-default formats don't honor these flags: --vmodule." or keep it?

@serathius
Copy link
Contributor

I don't think "Non-default formats don't honor these flags: --vmodule." is needed, we can remove it.

@@ -0,0 +1,42 @@
/*
Copyright 202 The Kubernetes Authors.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Copyright 202 The Kubernetes Authors.
Copyright 2022 The Kubernetes Authors.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, the good old times of 202! You are right of course, good catch.

@k8s-ci-robot k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed lgtm "Looks good to me", indicates that a PR is ready to be merged. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Aug 30, 2022
@pohly
Copy link
Contributor Author

pohly commented Aug 30, 2022

I don't think "Non-default formats don't honor these flags: --vmodule." is needed, we can remove it.

That's also my preference. While at it, I also removed "Non-default choices are currently alpha and subject to change without warning." because that has been wrong for a while now.

We don't need that sentence because non-GA formats list the corresponding feature gate:

          --logging-format string
                    Sets the log format. Permitted formats: "json" (gated by LoggingBetaOptions), "text". (default "text")

@k8s-ci-robot k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Sep 21, 2022
@k8s-triage-robot
Copy link

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.

@k8s-ci-robot k8s-ci-robot merged commit 6dbec8e into kubernetes:master Sep 21, 2022
SIG Node CI/Test Board automation moved this from PRs - Needs Reviewer to Done Sep 21, 2022
SIG Node PR Triage automation moved this from Needs Approver to Done Sep 21, 2022
@k8s-ci-robot k8s-ci-robot added this to the v1.26 milestone Sep 21, 2022
sanchezl added a commit to sanchezl/kubernetes that referenced this pull request Dec 20, 2022
…pohly/klog-flag-removal"

This reverts commit 6dbec8e, reversing
changes made to 17c5066.
openstack-mirroring pushed a commit to openstack/openstack that referenced this pull request May 4, 2023
* Update magnum from branch 'master'
  to f28b25734d3c1b11701076d47b6a37406b7317f0
  - Merge "Support k8s 1.26: remove logtostderr"
  - Support k8s 1.26: remove logtostderr
    
    klog args have been removed from kubernetes in 1.26, and
    deprecated since 1.23. kubernetes/kubernetes#112120
    
    The argument --logtostderr has defaulted to true for a long time, so
    this removal on older versions should have no impact.
    
    Change-Id: I64f934a9bbc39c5e054d8a83b3f6edee061469e6
openstack-mirroring pushed a commit to openstack/magnum that referenced this pull request May 4, 2023
klog args have been removed from kubernetes in 1.26, and
deprecated since 1.23. kubernetes/kubernetes#112120

The argument --logtostderr has defaulted to true for a long time, so
this removal on older versions should have no impact.

Change-Id: I64f934a9bbc39c5e054d8a83b3f6edee061469e6
nectar-gerrit pushed a commit to NeCTAR-RC/magnum that referenced this pull request May 31, 2023
klog args have been removed from kubernetes in 1.26, and
deprecated since 1.23. kubernetes/kubernetes#112120

The argument --logtostderr has defaulted to true for a long time, so
this removal on older versions should have no impact.

Change-Id: I64f934a9bbc39c5e054d8a83b3f6edee061469e6
(cherry picked from commit 16baf85)
enxebre added a commit to enxebre/hypershift that referenced this pull request Jun 5, 2023
enxebre added a commit to enxebre/hypershift that referenced this pull request Jun 6, 2023
mheler pushed a commit to mheler/magnum that referenced this pull request Aug 28, 2023
klog args have been removed from kubernetes in 1.26, and
deprecated since 1.23. kubernetes/kubernetes#112120

The argument --logtostderr has defaulted to true for a long time, so
this removal on older versions should have no impact.

Change-Id: I64f934a9bbc39c5e054d8a83b3f6edee061469e6
mnasiadka pushed a commit to stackhpc/magnum that referenced this pull request Nov 7, 2023
klog args have been removed from kubernetes in 1.26, and
deprecated since 1.23. kubernetes/kubernetes#112120

The argument --logtostderr has defaulted to true for a long time, so
this removal on older versions should have no impact.

Change-Id: I64f934a9bbc39c5e054d8a83b3f6edee061469e6
mheler pushed a commit to mheler/magnum that referenced this pull request Feb 17, 2024
klog args have been removed from kubernetes in 1.26, and
deprecated since 1.23. kubernetes/kubernetes#112120

The argument --logtostderr has defaulted to true for a long time, so
this removal on older versions should have no impact.

Change-Id: I64f934a9bbc39c5e054d8a83b3f6edee061469e6
mnasiadka pushed a commit to stackhpc/magnum that referenced this pull request Mar 28, 2024
klog args have been removed from kubernetes in 1.26, and
deprecated since 1.23. kubernetes/kubernetes#112120

The argument --logtostderr has defaulted to true for a long time, so
this removal on older versions should have no impact.

Change-Id: I64f934a9bbc39c5e054d8a83b3f6edee061469e6
(cherry picked from commit 16baf85)
mnasiadka pushed a commit to stackhpc/magnum that referenced this pull request Mar 28, 2024
klog args have been removed from kubernetes in 1.26, and
deprecated since 1.23. kubernetes/kubernetes#112120

The argument --logtostderr has defaulted to true for a long time, so
this removal on older versions should have no impact.

Change-Id: I64f934a9bbc39c5e054d8a83b3f6edee061469e6
(cherry picked from commit 16baf85)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. area/test cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. lgtm "Looks good to me", indicates that a PR is ready to be merged. priority/important-longterm Important over the long term, but may not be staffed and/or may need multiple releases to complete. release-note Denotes a PR that will be considered when it comes time to generate release notes. sig/architecture Categorizes an issue or PR as relevant to SIG Architecture. sig/cli Categorizes an issue or PR as relevant to SIG CLI. sig/instrumentation Categorizes an issue or PR as relevant to SIG Instrumentation. sig/node Categorizes an issue or PR as relevant to SIG Node. sig/testing Categorizes an issue or PR as relevant to SIG Testing. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. triage/accepted Indicates an issue or PR is ready to be actively worked on. wg/structured-logging Categorizes an issue or PR as relevant to WG Structured Logging.
Development

Successfully merging this pull request may close these issues.

None yet

10 participants