2845: klog flag deprecation: clarify scope and timeline #3033
Conversation
pohly
commented
Nov 2, 2021
•
pohly
commented
- One-line PR description: clarify scope and timeline: removal in 1.26
- Issue link: component-base: move v/vmodule/log-flush-frequency into LoggingConfiguration kubernetes#106090
- Other comments: Deprecating the flags only in some binaries is not going far enough because the full feature set would still have to be supported for the remaining ones.
k8s-ci-robot
added
cncf-cla: yes
k8s-ci-robot
added
kind/kep
|
/cc @kubernetes/wg-structured-logging-members |
k8s-ci-robot
added
the
lgtm
| period](https://kubernetes.io/docs/reference/using-api/deprecation-policy/#deprecating-a-flag-or-cli) | ||
| starts | ||
| - Kubernetes 1.24 (tentative): beta | ||
| - Kubernetes 1.25 (tentative): GA, deprecated flags get removed |
There was a problem hiding this comment.
I hope I understood the "available for two releases" rule correctly (https://kubernetes.io/docs/reference/using-api/deprecation-policy/#deprecating-a-flag-or-cli): 1.23 and 1.24 still have them, 1.25 won't.
There was a problem hiding this comment.
We'll also have to check the "for one year" rule. But we can discuss that when we get closer to the actual removal.
There was a problem hiding this comment.
I think this is an okay tentative timeline, but given how long these flags have been present, we may want to wait until 1.26 for removal.
There was a problem hiding this comment.
I'd prefer to remove them as soon as possible because the code for supporting them during the transition is really ugly. Can we keep 1.25 as tentative and check again when we are closer to that to determine whether we can go ahead?
There was a problem hiding this comment.
Per the linked policy + discussion with @logicalhan on today's triage, https://kubernetes.io/docs/reference/using-api/deprecation-policy/#deprecating-a-flag-or-cli states 12 months or 2 releases, whichever is longer - I think it has to be 1.26.
There was a problem hiding this comment.
I find that rule ambiguous because it's unclear which releases are counted for that "2 releases" check.
But let's be conservative and aim for 1.26. Commit and PR description was updated.
| 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, |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Important thing to note
kubernetes/component-base/logsis 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.
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
Yeah this would need to go on the blog at minimum. Ensure we are engaging the comms team.
There was a problem hiding this comment.
I can write a blog post and submit a talk for the next KubeCon.
| 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, |
There was a problem hiding this comment.
Yeah this would need to go on the blog at minimum. Ensure we are engaging the comms team.
| - Kubernetes logging configuration is completely stored in one struct | ||
| (`LoggingConfiguration`) before being applied to the process. |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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. |
| period](https://kubernetes.io/docs/reference/using-api/deprecation-policy/#deprecating-a-flag-or-cli) | ||
| starts | ||
| - Kubernetes 1.24 (tentative): beta | ||
| - Kubernetes 1.25 (tentative): GA, deprecated flags get removed |
There was a problem hiding this comment.
I think this is an okay tentative timeline, but given how long these flags have been present, we may want to wait until 1.26 for removal.
k8s-ci-robot
removed
the
lgtm
Deprecating the flags only in some binaries is not going far enough because the full feature set would still have to be supported for the remaining ones. The goal was to remove them everywhere and the KEP gets updated to reflect that. Because of the deprecation period for command line flags we want to announce the intent to deprecate as soon as possible. The proposal is to do it already at the alpha stage in Kubernetes 1.23. Most of the work is done or can be completed. Depending on the interpretation of "must function for 2 releases after their announced deprecation" (does that include the release for which the announcement was made?) we might be able to remove in 1.25. But to be safe and also to ensure the "for 12 months" rule, we aim for removal in 1.26.
|
@ehashman: did I address your concerns? The flags are now listed as "to be removed in 1.26". |
k8s-ci-robot
added
the
lgtm
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: ehashman, pohly 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 |
k8s-ci-robot
added
the
approved
