kube-proxy component config graduation to beta

[rosti]

This change is intended to propose a process and desired goals by which kube-proxy's component configuration is to be graduated to beta.

Note: This is set to implementable as work can start on it as soon as it merges.
ToDo: Tracking issue.

/assign @thockin
/cc @luxas @mtaufen @sttts @vllry

[sttts]

/assign @squeed

[vllry]

Thanks for posting. I can see the rationale behind all the struct split choices (mode, platform, instance/shared), but put together it seems complex for users to configure. Do you have adoption feedback from a comparable config breakdown?

[rosti]

Thanks for posting. I can see the rationale behind all the struct split choices (mode, platform, instance/shared), but put together it seems complex for users to configure. Do you have adoption feedback from a comparable config breakdown?

I have some feedback from kubeadm, which underwent a similar split on its path to beta. We mostly saw some confusion among existing users, who were accustomed to a large flat config (much like what is kube-proxy's today). However, these were rare cases and usually were easily fixable.
Also, in kubeadm, we provided a config migration tool and a good documentation.

[neolit123]

/assign

[thockin]

I can't follow the abstract names here - can we get specific?

Can you please make sure all comments that are going to be addressed are addressed and that all conversations are marked as "resolved"? It's not clear if there is pending work to be done.

Lastly: We've talked about breaking kube-proxy up into either platform-specific binaries or else per-mode binaries. If the config echos that (i.e. don't try to de-dup "common" fields), that might feel "redundant" but actually make for easier migration...

[thockin]

Again, this is pretty abstract - can we talk in concrete terms? Which SPECIFIC fields do we see as instance?

@mtaufen at some point we argued about leaving instance-specific configs as flags, but I forget where we eventually settled?

[rosti]

I have it updated with concrete fields (HostnameOverride and the *BindAddress fields). I also removed the mode specific stuff from there, as this is not applicable to the kube-proxy case, but was done more from an academic accuracy standpoint.
As far as the instance specific split is concerned, I am not fully sure about it. Certainly, we can enable instance specific (and probably shared too) fields to be overwritten from the command line. It's also certain, that some people may prefer the config file option for instance specifics. In that case a split is nice to have, as the shared config won't have to be patched (and therefore we can eliminate a possible source of errors).
However, this poses its own set of headaches - such as "What to do if we get a couple of InstanceConfigurations from different files?".

[mtaufen]

@thockin as I remember it we were both ok with it as a stopgap but it's not really a long term solution if we want the interface to the component to be uniform (in terms of versioning and schema definition).

[mtaufen]

"What to do if we get a couple of InstanceConfigurations from different files?"

Is that likely to happen? Seems like something we should just reject.

[thockin]

I agree - I don't mind some duplication if it means the blocks are more self-contained.

…

On Fri, Jul 12, 2019 at 6:46 AM Rostislav M. Georgiev ***@***.***> wrote: @rosti commented on this pull request. ________________________________ In keps/sig-network/20190613-kube-proxy-component-config.md: > +### Group platform specific fields together + +Platform specific fields need to be grouped together by platform name. This can be done by introducing structs to house them - one per platform (one for Linux, one for Windows, etc.). Then these structs will have an instance in the KubeProxyConfiguration. +That can allow for users to specify settings for more than one platform per a YAML document, thus allowing for deduplication of platform independent shared fields. This can reduce the possibility of an error for users. + +#### Example + +```yaml +commonSetting1: ... +commonSetting2: ... +... +modeA: ... # modeA is platform independent +platformX: + modeB: ... +platformY: + modeC: ... We don't have a good example of "someGeneralWindowsField" or "someGeneralLinuxField". In fact, I am a bit skeptical if we'll ever have such fields. I'd rather keep things as: kind: KubeProxyConfiguration clusterCIDR: ... windows: kernel: networkName: ... sourceVIP: ... enableDSR: ... and update the KEP accordingly. — You are receiving this because you were assigned. Reply to this email directly, view it on GitHub, or mute the thread.

[rosti]

@thockin @mtaufen @vllry @neolit123 I would appreciate another round of review, thank you!

[neolit123]

just a Q, have we established naming conventions for API types (possibly should be done in WG component-base).
e.g. kubeadm has InitConfiguration (no component name prefix), yet kubelet has KubeletConfiguration (component name prefixes the config). kubeproxy v1alpha1 has KubeProxyConfiguration.

[rosti]

I think, that as kinds should not have a component prefix. It's already given in the API group.
The only exception to this should be the cases, where a component config API group has a single kind in it. In that case it may be more practical to name the kind KubeProxyConfiguration as opposed to just Configuration.
However, this is just a personal preference of mine and I am almost certain, that it isn't codified anywhere.

[mtaufen]

The prefix is there for historical reasons - all componentconfig used to be in the same API group.

I did use {Component}Configuration as the convention in the Versioned Component Configuration Files doc: https://docs.google.com/document/d/1FdaEJUEh091qf5B98HM6_8MS764iXrxxigNIdwHYW9c/edit# because it's easy to read and was the convention at that point.

But it doesn't really matter. The package and/or API group information is typically close at hand.

Config and InstanceConfig could work fine as the convention IMO.

Fingers crossed I'm not contradicting something I said a long time ago 😉

[mtaufen]

@rosti have you had a chance to update the KEP per the resolved conversation threads?

[rosti]

@mtaufen I did it and I think, that I've covered all (although I may have missed something).

[vllry]

I'm not a fan of splitting up the config this much.

• Mode-specific config is a great idea
• Instance vs shared config is an okay idea
• Platform-specific config doesn't seem valuable

I'll rattle my sabre more if this resonates with other reviewers, otherwise I'll LGTM and we'll see what user feedback from the beta is.

[justinsb]

FYI I drafted a KEP to make more use of optional fields in componentconfig: #1173 I am not sure how that impacts beta plans here.

[mtaufen]

@vllry

Platform-specific config doesn't seem valuable

I can agree with this as long as each mode applies to at most one platform (or, I suppose, is so generic it will always work everywhere).

Also agree with your other two points.

[rosti]

One possibility, that I can think of is to remove the platform and instance/shared splitting parts for now. Thus, leave the mode consolidation bit for now.
Instead, we can have a reassessment after an alpha version is released with the mode consolidation changes as part of it.
@mtaufen @vllry @thockin WDYT?

[mtaufen]

Since we have the most consensus on the modes refactor, let's prototype that in a new alpha version. Maybe update the KEP to lay out the steps:

1. Modes refactor.
2. Instance/shared refactor.
3. Think about platform config if it turns out to be necessary after 1 and 2.

Then let's get this merged.

[rosti]

Updated by narrowing down the proposal to just mode consolidation for now. Hence, the platform split and the instance local/shared split is removed from the KEP for now.
Those may be reintroduced (along with some new proposals) after more thorough discussion later on in some of the next release cycles.
@mtaufen @vllry @thockin PTAL

[mtaufen]

This lgtm as a good next step for kube-proxy.
/lgtm

@rosti would you be interested in writing a more general componentconfig KEP for instance config?

[mtaufen]

Need someone from sig-network to approve. @thockin?

[thockin]

I'll approve the KEP for progress sake, but I have not had time to think deeply about a more ideal schema.

/lgtm
/approve

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: mtaufen, rosti, thockin

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:

• keps/sig-network/OWNERS [thockin]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[rosti]

@mtaufen

@rosti would you be interested in writing a more general componentconfig KEP for instance config?

I am, but I am not sure if it would be in this cycle (as I've got quite a few things to do ATM).