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

2845: klog flag deprecation: clarify scope and timeline #3033

Merged
merged 1 commit into from
Jan 14, 2022
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -124,10 +124,11 @@ best practices.

## Proposal

I propose to remove klog specific feature flags in Kubernetes core components
(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".
We propose to remove klog specific feature flags in Kubernetes components
(including, but not limited to, kube-apiserver, kube-scheduler,
Copy link
Contributor

Choose a reason for hiding this comment

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

Just to confirm, this extends the previous list of components to projects owned by K8s Org. In more detail, components that use kubernetes/component-base/logs, which also include kube-proxy, cluster-autoscaler etc.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Everything that uses kubernetes/component-base/logs to add klog flags worked without modifications, so sig-instrumentation doesn't need to do additional work because of this increased scoped.

Moving some options into LoggingConfiguration is more intrusive, but only a few commands use that, so it's still doable.

Copy link
Contributor

Choose a reason for hiding this comment

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

Important thing to note kubernetes/component-base/logs is used by non-Kubernetes Org projects, we didn't give any compatibility promises, but I expect some people will not be happy. Possibly this could be solved with proper documentation that would describe how those project could directly use klog.

Choose a reason for hiding this comment

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

Important thing to note kubernetes/component-base/logs is used by non-Kubernetes Org projects, we didn't give any compatibility promises, but I expect some people will not be happy. Possibly this could be solved with proper documentation that would describe how those project could directly use klog.

Would it make sense to add something about this to alpha or beta graduation criteria? All we have is Documentation on migrating off klog flags is publicly available but that doesnt seem to cover @serathius suggestion above.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Where would such documentation live? A blog post describing the recent changes?

There's the API documentation in gopkg. Do we need more than that?

Copy link
Member

Choose a reason for hiding this comment

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

Yeah this would need to go on the blog at minimum. Ensure we are engaging the comms team.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I can write a blog post and submit a talk for the next KubeCon.

kube-controller-manager, kubelet) and leave them with defaults. From klog flags
we would remove all flags besides "-v" and "-vmodule". The component-base
"-log-flush-frequency" flag is also kept.

### Removed klog flags

Expand Down Expand Up @@ -269,17 +270,20 @@ all existing klog features.

#### Alpha

- Klog can be configured without registering flags
- Kubernetes logging configuration drops global state
- The remaining supported klog options (`-v`, `--vmodule`) and
`--log-flush-frequency` can be configured without registering flags.
- Kubernetes logging configuration is completely stored in one struct
(`LoggingConfiguration`) before being applied to the process.
Comment on lines +275 to +276
Copy link
Member

Choose a reason for hiding this comment

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

We removed the line "drops global state", I assume this is the substitute. Are we still doing that? If so, can we provide a bit more detail about the global state this is replacing?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As long as we have klog, there is going to be global state in klog (the global logger and its configuration). There's no way around that in alpha and later because this KEP isn't about removing klog.

I discussed this with @serathius in https://kubernetes.slack.com/archives/C020CCMUEAX/p1635445845031900 and he explained that the intention was to treat all logging configuration as local state until applied to the process, so the "stored in one struct" line item is the replacement for "drops global state".

- Go-runner is feature complementary to klog flags planned for deprecation
- Projects in Kubernetes Org are migrated to go-runner
- JSON logs format splits stdout and stderr logs
- The klog flags which will be removed are marked as deprecated in command line
help output and the deprecation is announced in the Kubernetes release notes.
Copy link
Member

Choose a reason for hiding this comment

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

👍


#### 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

#### GA

Expand All @@ -290,6 +294,12 @@ all existing klog features.
- 20/06/2021 - Original proposal created in https://github.com/kubernetes/kubernetes/issues/99270
- 30/07/2021 - KEP draft was created
- 26/08/2021 - Merged in provisional state
- 09/09/2021 - Merged as implementable
- Kubernetes 1.23 (tenatative): alpha, [deprecation
period](https://kubernetes.io/docs/reference/using-api/deprecation-policy/#deprecating-a-flag-or-cli)
starts
- Kubernetes 1.24 (tentative): beta
- Kubernetes 1.26 (tentative): GA, deprecated flags get removed

## Drawbacks

Expand Down Expand Up @@ -474,4 +484,4 @@ No

###### What steps should be taken if SLOs are not being met to determine the problem?

N/A
N/A