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.

Sign up for GitHub

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

Move KEP-2845 to implementable #2912

Merged
merged 7 commits into from
Sep 9, 2021
Merged

Move KEP-2845 to implementable #2912

Changes from 1 commit
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
f347407
Move KEP-2845 to implementable
serathius Aug 30, 2021
3a83f0a
Add PRR for #2845
serathius Sep 2, 2021
921f7e6
Remove mention of --logtostdout flag in #2845
serathius Sep 7, 2021
d6dbc22
KEP #2485: Require splitting stdout/stderr to Json format
serathius Sep 8, 2021
fba63de
Add pohly as reviewer for KEP 2845
serathius Aug 26, 2021
ed0bd31
#2845 Update PRR
serathius Sep 9, 2021
af7fdba
#2845 Add dims as SIG arch approver
serathius Sep 9, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
82 changes: 9 additions & 73 deletions ...-instrumentation/2845-deprecate-klog-specific-flags-in-k8s-components/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,16 +121,14 @@ best practices.
### Non-Goals

* Change klog output format
* Remove flags from klog

## Proposal

I propose to remove klog specific feature flags in Kubernetes core components
(kube-apiserver, kube-scheduler, kube-controller-manager, kubelet) and set them
to agreed good defaults. From klog flags we would remove all flags besides "-v"
and "-vmodule". With removal of flags to route logs based on type we want to
change the default routing that will work as better default. Changing the
defaults will be done in via multi release process, that will introduce some
temporary flags that will be removed at the same time as other klog flags.
(kube-apiserver, kube-scheduler, kube-controller-manager, kubelet) and leave
them with defaults. From klog flags we would remove all flags besides "-v"
and "-vmodule".
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we add:

Support for "-vmodule" will depend on the selected log format and be supported at least for the traditional "text" format.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added in section below


### Removed klog flags

Expand Down Expand Up @@ -172,41 +170,12 @@ directly impacting log volume and component performance.

With removal of configuration alternatives we need to make sure that defaults
make sense. List of logging features implemented by klog and proposed actions:
* Routing logs based on type/verbosity - Should be reconsidered.
* Routing logs based on type/verbosity - Feature removed.
Copy link
Contributor

Choose a reason for hiding this comment

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

We could promise to implement that for JSON output. But I prefer to keep that out of the KEP for now if that helps to get it merged.

If we want to include it, then here we could replace "Feature removed" with "Move into JSON backend" and below under "beta graduation" add "configuration of JSON backend implemented, providing at least message routing based on type and verbosity".

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added to Alpha stage as I want to implement it in next release to unblock JSON graduation to Beta.

* Writing logs to file - Feature removed.
* Log file rotation based on file size - Feature removed.
* Configuration of log headers - Use the current defaults.
* Adding stacktrace - Feature removed.

For log routing I propose to adopt UNIX convention of writing info logs to
stdout and errors to stderr. For log headers I propose to use the current
default.

#### Split stdout and stderr

As logs should be treated as event streams I would propose that we separate two
main streams "info" and "error" based on log method called. As error logs should
usually be treated with higher priority, having two streams prevents single
pipeline from being clogged down (for example
[kubernetes/klog#209](https://github.com/kubernetes/klog/issues/209)).
For logging formats writing to standard streams, we should follow UNIX standard
of mapping "info" logs to stdout and "error" logs to stderr.

Splitting stdout from stderr would be a breaking change in both klog and
kubernetes components. However, we expect only minimal impact on users, as
redirecting both streams is a common practice. In rare cases that will be
impacted, adapting to this change should be a 1 line change. Still we will want
to give users a proper heads up before making this change, so we will hide the
change behind a new logging flag `--logtostdout`. This flag will be used avoid
introducing breaking change in klog.

With this flag we can follow multi release plan to minimize user impact (each
point should be done in a separate Kubernetes release):
1. Introduce the flag in disabled state and start using it in tests.
1. Announce flag availability and encourage users to adopt it.
1. Enable the flag by default and deprecate it (allows users to flip back to previous behavior)
1. Remove the flag following the deprecation policy.

#### Logging headers

Default logging headers configuration results in klog writing information about
Expand Down Expand Up @@ -295,19 +264,16 @@ all existing klog features.
- Kubernetes logging configuration drops global state
- Go-runner is feature complementary to klog flags planned for deprecation
- Projects in Kubernetes Org are migrated to go-runner
- Add --logtostdout flag to klog disabled by default
- Use --logtostdout in kubernetes tests

#### Beta

- Go-runner project is well maintained and documented
- Documentation on migrating off klog flags is publicly available
- Kubernetes klog flags are marked as deprecated
- Enable --logtostdout in Kubernetes components by default

#### GA

- Kubernetes klog specific flags are removed (including --logtostdout)
- Kubernetes klog specific flags are removed

## Implementation History

Expand Down Expand Up @@ -336,21 +302,13 @@ just k8s core components

### Upgrade / Downgrade Strategy

Proposed user impacting changes are as follows:
* Deprecate and remove subset of klog flags
* Split log output (previously only stderr) into stdout/stderr based on type (info/error)

In case of the first change, we will be following K8s deprecation policy. There
will be 3 releases between informing users about deprecation and full removal.
For removal of klog specific flags we will be following K8s deprecation policy.
There will be 3 releases between informing users about deprecation and full removal.
During deprecation period there will not be any changes in behavior for clusters
using deprecated features, however after removal there will not be a way to
restore previous behavior. 3 releases should be enough heads up for users to
make necessary changes to avoid breakage.

In case of changing log output, we will introduce a temporary flag
`--logtostdout` that will allow users to flip back to previous behavior.
This flag too will be marked as deprecated and removed with same period.

### Version Skew Strategy

Proposed changes have no impact on cluster that would require coordination.
Expand All @@ -362,17 +320,6 @@ k8s binaries, but this can be done one by one independently of other components.

### Feature Enablement and Rollback

For stdout/stderr log split feature we are introducing dedicated flag
`--logtostdout`, which can be used to enable and rollback the feature.

As this feature will be releases in stages, let's discuss them separately:
* Alpha - feature is available, users can enable the feature by passing
`--logtostdout` flag to binaries.
* Beta - feature is enabled by default. Users can rollback by passing
`--logtostdout=false` flag to binaries. Old configuration is deprecated.
* GA - Old configuration is no longer supported. This is ok wait 3 releases to
uphold Kubernetes deprecation policy.

###### How can this feature be enabled / disabled in a live cluster?

- [ ] Feature gate (also fill in values in `kep.yaml`)
Expand Down Expand Up @@ -416,20 +363,10 @@ This section must be completed when targeting beta to a release.

###### How can a rollout or rollback fail? Can it impact already running workloads?

There are two cases of failed rollouts I would consider:
* users logging setup doesn't handle stdout
* users didn't adjust flags and left deprecated flags in their config

When writing to stdout becomes the default, users will need to ensure that their
logging setup handles it. If they fail to do so, we are still leaving an escape
hatch for them via a `--logtostdout=false` flag.

For removing klog flags, we don't have any escape hatch. Such breaking changes
will be properly announced, but users will need to make adjustments before
deprecation period finishes.

I don't expect rollbacks to fail,

###### What specific metrics should inform a rollback?

Users could observe number of logs from K8s components that they ingest. If
Expand All @@ -452,7 +389,6 @@ This section must be completed when targeting beta to a release.

###### How can an operator determine if the feature is in use by workloads?

N/A

###### How can someone using this feature know that it is working for their instance?

Expand Down Expand Up @@ -523,7 +459,7 @@ details). For now, we leave it here.

###### How does this feature react if the API server and/or etcd is unavailable?

N/A
Logs don't have a remote dependency on the API server or etcd.

###### What are other known failure modes?

Expand Down