1.10 hyperkube kubelet is losing some parameters from help #62009
Assignees
Comments
k8s-ci-robot
added
kind/bug
|
This is kubelet help info from Kubernetes 1.9. |
|
/assign @mtaufen |
|
All KubeletConfiguration flags are now deprecated, see this |
|
@hzxuzhonghu what do you mean of |
|
Sorry, did not look into flag parsing |
|
I think the behavior of hiding deprecated flags needs to change. That has caused problems with masking important information about deprecated behaviors like |
|
Based on my test, not only deprecated parameters are being hidden, but also non-deprecated parameters, such as |
All values that can be passed via a kubelet config file are marked as deprecated (this included max-pods, etc). See https://github.com/kubernetes/kubernetes/blob/master/cmd/kubelet/app/options/options.go#L430-L442 |
|
@liggitt so where can we get a list of parameters supported by kubelet? |
|
unfortunately, it looks as though looking at the code is the only way to see flags marked as deprecated. @mtaufen, that's a fairly significant issue for these... can you look into restoring visibility to those flags? |
|
I'll see what I can do.
…On Tue, Apr 3, 2018, 6:26 AM Jordan Liggitt ***@***.***> wrote:
unfortunately, it looks as though looking at the code is the only way to
see flags marked as deprecated. @mtaufen <https://github.com/mtaufen>,
that's a fairly significant issue for these... can you look into restoring
visibility to those flags?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#62009 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AA3Jwcs8IDGOmnRkhYT_clRsAFmqKECIks5tk3hmgaJpZM4TDMxC>
.
|
|
If we want to do this, we may have to generate our own usage text instead
of relying on pflag's built-in FlagSet.FlagUsagesWrapped method.
FlagUsagesWrapped hides deprecated flags from the usage text it returns.
An alternative would be to simply change the usage text to note
deprecation, without marking deprecated, but then we would lose warn-on-use.
On Tue, Apr 3, 2018, 7:54 AM Michael Taufen <notifications@github.com>
wrote:
… I'll see what I can do.
On Tue, Apr 3, 2018, 6:26 AM Jordan Liggitt ***@***.***>
wrote:
> unfortunately, it looks as though looking at the code is the only way to
> see flags marked as deprecated. @mtaufen <https://github.com/mtaufen>,
> that's a fairly significant issue for these... can you look into
restoring
> visibility to those flags?
>
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub
> <
#62009 (comment)
>,
> or mute the thread
> <
https://github.com/notifications/unsubscribe-auth/AA3Jwcs8IDGOmnRkhYT_clRsAFmqKECIks5tk3hmgaJpZM4TDMxC
>
> .
>
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<#62009 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AA3JwUL2A5OnsHS6V-isaB-Giby8Qj8eks5tk40YgaJpZM4TDMxC>
.
|
Anyone using deprecated flags should be getting a warning in their log, but it's a crapshoot whether people actually pay attention to the logs, so I agree that this is an issue. |
|
I'd like to just make a custom copy of It looks like we do Would it suffice to put a single file in I'm not a lawyer, I don't know how to handle these things. @thockin who should I talk to? |
|
Can we try to send a patch to pflag? I know a maintainer :) @eparis thoughts? |
|
We don't want to fork. I don't think it's the right answer, but we'd probably set < rant >But any of those options sound like us making the horrible mistake of no working with other communities. NIH syndrome is oddly high recently. Weird that we'd talk about forking pflag before even filing an issue or sending a patch to change it. < /rant > If I'm understanding the issue it seems like we have a weird case. Where for some commands (kubelet) these flags really are deprecated and we really don't want users to find them anymore. We want users to use the kubelet config. But in other commands (hyperkube) that same flagset has flags which aren't really deprecated and shouldn't really be hidden from users. Is that right? Or are they deprecated for both but we really want to put them right in the user's face so they use them anyway? If they really are deprecated I'm not sure I understand @liggitt assertion that this is a 'significant issue'. I'm not sure I understand the use case of continuing to show users flags they shouldn't ever be setting. Let's assume I'm convinced that Jordan's position is correct. Would we rather have a whole new section in the help text just with deprecated flags? Would we rather have them in the Local Flags or Inherited Flags sections, but with some disclaimer? Would we use the It's relatively easy to set our own cobra.Command.UsageTemplate and add our own section which prints the deprecated flags however we want. But if someone can convince me that showing deprecated flags is a win, and describe how they think we should show them for that use case, I might get off my lazy butt and make changes to cobra and pflag. |
|
We're trying to reduce user surprise.
When a bunch of flags suddenly disappear from the help, that's surprising.
It would be less surprising for deprecated flags to appear like
`Deprecated: -f, --flag val {Deprecation text} {Help text}`, or perhaps
have them in a separate section.
I'll add that it's important to provide notice in several places;
deprecation warnings in logs often miss the engineers who need to adapt to
them.
Agree that the best case scenario is if this becomes possible in pflag,
natively :).
…On Wed, Apr 4, 2018 at 7:22 PM Eric Paris ***@***.***> wrote:
We don't want to fork. I don't think it's the right answer, but we'd
probably set FlagSet.Usage or edit cobra.Command.UsageTemplate if we were
going to do it ourselves. Not fork.
But any of those options sound like us making the horrible mistake of no
working with other communities. NIH syndrome is oddly high recently. Weird
that we'd talk about forking pflag before even filing an issue or sending a
patch to change it.
If I'm understanding the issue it seems like we have a weird case. Where
for some commands (kubelet) these flags really are deprecated and we really
don't want users to find them anymore. We want users to use the kubelet
config. But in other commands (hyperkube) that same flagset has flags which
aren't really deprecated and shouldn't really be hidden from users. Is that
right?
Or are they deprecated for both but we really want to put them right in
the user's face so they use them anyway? If they really are deprecated I'm
not sure I understand @liggitt <https://github.com/liggitt> assertion
that this is a 'significant issue'. I'm not sure I understand the use case
of continuing to show users flags they shouldn't ever be setting.
Let's assume I'm convinced that Jordan's position is correct. Would we
rather have a whole new section in the help text just with deprecated
flags? Would we rather have them in the Local Flags or Inherited Flags
sections, but with some disclaimer? Would we use the Deprecated flag text
somehow? How would we designate them deprecated?
It's relatively easy to set our own cobra.Command.UsageTemplate and add
our own section which prints the deprecated flags however we want.
But if someone can convince me that showing deprecated flags is a win, and
describe how they think we should show them for that use case, I might get
off my lazy butt and make changes to cobra and pflag.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#62009 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AA3JwXjvo9f2VzwyV6lbzsjyKwMtNg4dks5tlX_8gaJpZM4TDMxC>
.
--
Michael Taufen
Google SWE
|
|
I'm not convinced this is the right thing to do. The help page should tell the users the right thing to do, not expose the wrong thing to do. But, just in case, people really really think it's important to keep telling users the wrong way to do things, I wrote spf13/pflag#163 |
|
I think your comment on that PR outlines the problem well; some people (like me 😉) want a deprecation policy that keeps deprecated flags visible with a warning until they are actually removed, while other people want the flags to be hidden as soon as they are marked deprecated. |
We should be able to treat help as an opinionated recommendation, or simply as reference documentation that explains available options, depending on our target audience. In this case, we surprised at least a few people by defaulting to the former... |
|
I really do try to listen, even when I don't agree :) If jordan and tim tell me I'm wrong and we should do this, I'll merge to pflag. |
I definitely want the option to show help for a deprecated flag until it is removed. |
|
I 'definitely' think Jordan's wrong, but I merged he pflag change. So we can rebase and unhide as much as we'd like. :) |
|
Thanks! I blocked out some time to make the corresponding k8s changes tomorrow. |
|
@mtaufen awesome, thank you. Let me know if it doesn't do what we need! |
k8s-github-robot
pushed a commit
that referenced
this issue
Apr 14, 2018
Automatic merge from submit-queue. If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>. Show help for deprecated Kubelet flags We recently deprecated a bunch of Kubelet flags, which caused them to disappear from `--help` output. This PR unhides these flags, so that the deprecation notice is clearly visible in `--help`. Fixes: #62009 ```release-note NONE ``` /cc @eparis
|
Does anyone know how I can pass the cadviors metric flags to hyperkube? i.e disable_metrics |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Is this a BUG REPORT or FEATURE REQUEST?:
/kind bug
What happened:
The above help do not include some parameters such as
max-pods,max-open-filesetc./sig cli
The text was updated successfully, but these errors were encountered: