Opinion: "Default" vs non-specified

[Tim Hockin]

in a few places we have policy-style fields (e.g. DNSPolicy). I'm
looking at a PR that add a new one
(https://github.com/kubernetes/kubernetes/pull/79386/commits/6a42005f6984d5879e84ec3bda50b3454119074c)
and I want opinions.

Should we enumerate "Default" as a value and use defaulting logic to
set it, or should we document "not specified" (nil) as doing the
default behavior? In this case I kind of lean to the latter, since it
is an optional policy. omitempty would be nicer than seeing
"Default".

Maybe we should document a convention (I volunteer) if we can agree on
what it should be?

Tim

[Daniel Smith]

In general, my default position is that it's good to give names to opinionated values ("Default" is definitely one), and reserve unset / nil / go default for clients that don't understand the field or don't wish to have an opinion.

Although there's other moving pieces to get right* before it helps significantly, this is part of helping old clients deal with new fields. Assigning a meaning to the unset/nil/default value makes it impossible to tell if a client's omission of a field was intentional or not.

[*] e.g., on PUT, a general chunk of code that does s/unset/prior value if there is one, default value otherwise/.

--
You received this message because you are subscribed to the Google Groups "kubernetes-api-reviewers" group.
To unsubscribe from this group and stop receiving emails from it, send an email to kubernetes-api-rev...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/kubernetes-api-reviewers/CAO_RewZMo%2BUPCkoauanqEs9ikAoTs5d9rhKsJ4abhmqHEfkwgg%40mail.gmail.com.

[Jordan Liggitt]

I agree that it is better to make the previously implicit behavior explicit (ideally with a more descriptive enum value than "Default").

There are two patterns we see frequently:

• Original version v1 has implicit default behavior Foo. New version v2 has improved default behavior Bar.
• Original version v1 has implicit default behavior Foo. New version v2 requires API consumers to specify what behavior they want (v2 has no default value and the field is required)

Both of those require v1 to assign an explicit default value.

To view this discussion on the web visit https://groups.google.com/d/msgid/kubernetes-api-reviewers/CAB_J3bZESyze56Fz66ZMjr6Z-BxHJ%3DS8Cye_4r0MjRtafxaShg%40mail.gmail.com.

[Daniel Smith]

On Mon, Aug 12, 2019 at 12:37 PM Jordan Liggitt <lig...@google.com> wrote:

I agree that it is better to make the previously implicit behavior explicit (ideally with a more descriptive enum value than "Default").

Yeah, agreed-- if you can give a name to the old semantic, it's best to do so; "Default" doesn't help people understand the behavior they're about to get.

[Tim Hockin]

On Mon, Aug 12, 2019 at 12:41 PM Daniel Smith <dbs...@google.com> wrote:
>
>
>
> On Mon, Aug 12, 2019 at 12:37 PM Jordan Liggitt <lig...@google.com> wrote:
>>
>> I agree that it is better to make the previously implicit behavior explicit (ideally with a more descriptive enum value than "Default").
>
>
> Yeah, agreed-- if you can give a name to the old semantic, it's best to do so; "Default" doesn't help people understand the behavior they're about to get.
>
>>
>>
>> There are two patterns we see frequently:
>>
>> Original version v1 has implicit default behavior Foo. New version v2 has improved default behavior Bar.
>> Original version v1 has implicit default behavior Foo. New version v2 requires API consumers to specify what behavior they want (v2 has no default value and the field is required)
>>
>> Both of those require v1 to assign an explicit default value.

In this case, v1 has implicit default behavior which depends on some
config that was determined by cluster admin, and we're now offering
users control over that. They can choose A or B or "don't care" (aka
"Default").

The argument of making intention clear when not specified holds water
for me. In this particular case, the field is immutable after
creation, but as a pattern I can buy it.

[Daniel Smith]

On Mon, Aug 12, 2019 at 12:49 PM Tim Hockin <tho...@google.com> wrote:

On Mon, Aug 12, 2019 at 12:41 PM Daniel Smith <dbs...@google.com> wrote:
>
>
>
> On Mon, Aug 12, 2019 at 12:37 PM Jordan Liggitt <lig...@google.com> wrote:
>>
>> I agree that it is better to make the previously implicit behavior explicit (ideally with a more descriptive enum value than "Default").
>
>
> Yeah, agreed-- if you can give a name to the old semantic, it's best to do so; "Default" doesn't help people understand the behavior they're about to get.
>
>>
>>
>> There are two patterns we see frequently:
>>
>> Original version v1 has implicit default behavior Foo. New version v2 has improved default behavior Bar.
>> Original version v1 has implicit default behavior Foo. New version v2 requires API consumers to specify what behavior they want (v2 has no default value and the field is required)
>>
>> Both of those require v1 to assign an explicit default value.

In this case, v1 has implicit default behavior which depends on some
config that was determined by cluster admin, and we're now offering
users control over that.  They can choose A or B or "don't care" (aka
"Default").

What happens in practice if they choose "don't care"? Name this behavior. If e.g. the system admin chose and they had two choices, name each choice and present that in this field. 

[Tim Hockin]

On Mon, Aug 12, 2019 at 12:54 PM Daniel Smith <dbs...@google.com> wrote:
>
>
>
> On Mon, Aug 12, 2019 at 12:49 PM Tim Hockin <tho...@google.com> wrote:
>>
>> On Mon, Aug 12, 2019 at 12:41 PM Daniel Smith <dbs...@google.com> wrote:
>> >
>> >
>> >
>> > On Mon, Aug 12, 2019 at 12:37 PM Jordan Liggitt <lig...@google.com> wrote:
>> >>
>> >> I agree that it is better to make the previously implicit behavior explicit (ideally with a more descriptive enum value than "Default").
>> >
>> >
>> > Yeah, agreed-- if you can give a name to the old semantic, it's best to do so; "Default" doesn't help people understand the behavior they're about to get.
>> >
>> >>
>> >>
>> >> There are two patterns we see frequently:
>> >>
>> >> Original version v1 has implicit default behavior Foo. New version v2 has improved default behavior Bar.
>> >> Original version v1 has implicit default behavior Foo. New version v2 requires API consumers to specify what behavior they want (v2 has no default value and the field is required)
>> >>
>> >> Both of those require v1 to assign an explicit default value.
>>
>> In this case, v1 has implicit default behavior which depends on some
>> config that was determined by cluster admin, and we're now offering
>> users control over that. They can choose A or B or "don't care" (aka
>> "Default").
>
>
> What happens in practice if they choose "don't care"? Name this behavior. If e.g. the system admin chose and they had two choices, name each choice and present that in this field.

That may be possible - I'll have to look.

[Tim Hockin]

Hit send too fast

On Mon, Aug 12, 2019 at 1:45 PM Tim Hockin <tho...@google.com> wrote:
>
> On Mon, Aug 12, 2019 at 12:54 PM Daniel Smith <dbs...@google.com> wrote:
> >
> >
> >
> > On Mon, Aug 12, 2019 at 12:49 PM Tim Hockin <tho...@google.com> wrote:
> >>
> >> On Mon, Aug 12, 2019 at 12:41 PM Daniel Smith <dbs...@google.com> wrote:
> >> >
> >> >
> >> >
> >> > On Mon, Aug 12, 2019 at 12:37 PM Jordan Liggitt <lig...@google.com> wrote:
> >> >>
> >> >> I agree that it is better to make the previously implicit behavior explicit (ideally with a more descriptive enum value than "Default").
> >> >
> >> >
> >> > Yeah, agreed-- if you can give a name to the old semantic, it's best to do so; "Default" doesn't help people understand the behavior they're about to get.
> >> >
> >> >>
> >> >>
> >> >> There are two patterns we see frequently:
> >> >>
> >> >> Original version v1 has implicit default behavior Foo. New version v2 has improved default behavior Bar.
> >> >> Original version v1 has implicit default behavior Foo. New version v2 requires API consumers to specify what behavior they want (v2 has no default value and the field is required)
> >> >>
> >> >> Both of those require v1 to assign an explicit default value.
> >>
> >> In this case, v1 has implicit default behavior which depends on some
> >> config that was determined by cluster admin, and we're now offering
> >> users control over that. They can choose A or B or "don't care" (aka
> >> "Default").
> >
> >
> > What happens in practice if they choose "don't care"? Name this behavior. If e.g. the system admin chose and they had two choices, name each choice and present that in this field.
>
> That may be possible - I'll have to look.

The problem is that the config for that is not present in defaulting
logic - do we want defaulting logic in apiserver examining things like
flags? Is there a sane way to do that?

[Daniel Smith]

On Mon, Aug 12, 2019 at 1:47 PM Tim Hockin <tho...@google.com> wrote:

Hit send too fast

On Mon, Aug 12, 2019 at 1:45 PM Tim Hockin <tho...@google.com> wrote:
>
> On Mon, Aug 12, 2019 at 12:54 PM Daniel Smith <dbs...@google.com> wrote:
> >
> >
> >
> > On Mon, Aug 12, 2019 at 12:49 PM Tim Hockin <tho...@google.com> wrote:
> >>
> >> On Mon, Aug 12, 2019 at 12:41 PM Daniel Smith <dbs...@google.com> wrote:
> >> >
> >> >
> >> >
> >> > On Mon, Aug 12, 2019 at 12:37 PM Jordan Liggitt <lig...@google.com> wrote:
> >> >>
> >> >> I agree that it is better to make the previously implicit behavior explicit (ideally with a more descriptive enum value than "Default").
> >> >
> >> >
> >> > Yeah, agreed-- if you can give a name to the old semantic, it's best to do so; "Default" doesn't help people understand the behavior they're about to get.
> >> >
> >> >>
> >> >>
> >> >> There are two patterns we see frequently:
> >> >>
> >> >> Original version v1 has implicit default behavior Foo. New version v2 has improved default behavior Bar.
> >> >> Original version v1 has implicit default behavior Foo. New version v2 requires API consumers to specify what behavior they want (v2 has no default value and the field is required)
> >> >>
> >> >> Both of those require v1 to assign an explicit default value.
> >>
> >> In this case, v1 has implicit default behavior which depends on some
> >> config that was determined by cluster admin, and we're now offering
> >> users control over that.  They can choose A or B or "don't care" (aka
> >> "Default").
> >
> >
> > What happens in practice if they choose "don't care"? Name this behavior. If e.g. the system admin chose and they had two choices, name each choice and present that in this field.
>
> That may be possible - I'll have to look.

The problem is that the config for that is not present in defaulting
logic - do we want defaulting logic in apiserver examining things like
flags?  Is there a sane way to do that?

I'm not sure if we have done that in other contexts... But maybe first answer this question: if the config changes, do you want all objects to instantly get the configured behavior? or do you want existing objects to continue with the behavior they're getting?

I think the answer to that determines how it should be represented in the API.

If you want everything to change instantly, then maybe something like "AsConfigured" is better than "Default"

[Tim Hockin]

On Mon, Aug 12, 2019 at 1:53 PM Daniel Smith <dbs...@google.com> wrote:
>
>
>
> On Mon, Aug 12, 2019 at 1:47 PM Tim Hockin <tho...@google.com> wrote:
>>
>> Hit send too fast
>>
>> On Mon, Aug 12, 2019 at 1:45 PM Tim Hockin <tho...@google.com> wrote:
>> >
>> > On Mon, Aug 12, 2019 at 12:54 PM Daniel Smith <dbs...@google.com> wrote:
>> > >
>> > >
>> > >
>> > > On Mon, Aug 12, 2019 at 12:49 PM Tim Hockin <tho...@google.com> wrote:
>> > >>
>> > >> On Mon, Aug 12, 2019 at 12:41 PM Daniel Smith <dbs...@google.com> wrote:
>> > >> >
>> > >> >
>> > >> >
>> > >> > On Mon, Aug 12, 2019 at 12:37 PM Jordan Liggitt <lig...@google.com> wrote:
>> > >> >>
>> > >> >> I agree that it is better to make the previously implicit behavior explicit (ideally with a more descriptive enum value than "Default").
>> > >> >
>> > >> >
>> > >> > Yeah, agreed-- if you can give a name to the old semantic, it's best to do so; "Default" doesn't help people understand the behavior they're about to get.
>> > >> >
>> > >> >>
>> > >> >>
>> > >> >> There are two patterns we see frequently:
>> > >> >>
>> > >> >> Original version v1 has implicit default behavior Foo. New version v2 has improved default behavior Bar.
>> > >> >> Original version v1 has implicit default behavior Foo. New version v2 requires API consumers to specify what behavior they want (v2 has no default value and the field is required)
>> > >> >>
>> > >> >> Both of those require v1 to assign an explicit default value.
>> > >>
>> > >> In this case, v1 has implicit default behavior which depends on some
>> > >> config that was determined by cluster admin, and we're now offering
>> > >> users control over that. They can choose A or B or "don't care" (aka
>> > >> "Default").
>> > >
>> > >
>> > > What happens in practice if they choose "don't care"? Name this behavior. If e.g. the system admin chose and they had two choices, name each choice and present that in this field.
>> >
>> > That may be possible - I'll have to look.
>>
>> The problem is that the config for that is not present in defaulting
>> logic - do we want defaulting logic in apiserver examining things like
>> flags? Is there a sane way to do that?
>
>
> I'm not sure if we have done that in other contexts... But maybe first answer this question: if the config changes, do you want all objects to instantly get the configured behavior? or do you want existing objects to continue with the behavior they're getting?

The value they get is reified into other fields as things like
ClusterIP and LoadBalancer IP get allocated. So it will not adapt if
the global default changes, thought it MIGHT be possible (i'd have to
see) to find a path through defaulting and updates to trick that.
Part of why Service needs a reboot.

I just tested it, and there *is* a way to get your ClusterIP
re-allocated, despite us documenting it as immutable. I don't know
what we want here. This is a corner-case. In general we will NOT
update this on live objects, so crystalizing it could be OK if the
plumbng works.

[Clayton Coleman]

I’ll note when we added the Proc mount behavior (so people could do
rootless as non-root), we defined the current default (masking) with
something suitably specific and had it be nil with a defaulted enum.

> To view this discussion on the web visit https://groups.google.com/d/msgid/kubernetes-api-reviewers/CAO_RewYd-UiWmB8JJNoWvuWa2iu8xV8hTYw6wsi3ZZuGVcYfgg%40mail.gmail.com.