Add support for kubernetes --feature-gates in template

[stuartleeks]

Is this a request for help?: No

---

Is this an ISSUE or FEATURE REQUEST? (choose one): FEATURE REQUEST

---

What version of acs-engine?: latest master (8fd4ac4)

---

Orchestrator and version (e.g. Kubernetes, DC/OS, Swarm): Kubernetes - all versions

What you expected to happen: Various behaviours (and preview features) in Kubernetes are controlled by --feature-gates. As per the docs, specifying --feature-gates is currently not permitted.

Proposal:

1. Allow --feature-gates to be specified in the kubeletConfig.
   Looking at the code, the agents currently include Accelerators=true in --feature-gates for kubernetes version 1.6.0 and above, so this behaviour should be preserved.
2. Add an apiserverConfig section to kubernetesConfig as per the kubeletConfig to allow apiserver specific configuration (including --feature-gates). This was discussed in a comment here

Sample config:

"kubernetesConfig": {
    "kubeletConfig": {
        "--cluster-dns": "172.17.1.11",
        "--max-pods": "30",
        "--feature-gates": {
            "MountPropagation": "true",
            "TaintNodesByCondition": "true"
        },
        <etc...>
    },
    "apiserverConfig": {
        "--storage-backend": "etcd3",
        "--feature-gates": {
            "MountPropagation": "true"
        },
        <etc...>
    }
}

Update:
As per the comments from @paulbouwer we should ensure that this covers settings --feature-gates for:

• kubelet
• api-server
• kube-controller-manager
• kube-proxy
• kube-scheduler

[stuartleeks]

@jackfrancis, @CecileRobertMichon, @JackQuincy - would love your thoughts on the approach :-)

[stuartleeks]

@croomes - does this look like it would fit your requirements?

[stuartleeks]

Looking at the implementation, kubeletConfig is a map[string][string], so would you prefer a config like the one below to reduce the impact of the change?

"kubernetesConfig": {
    "kubeletConfig": {
        "--cluster-dns": "172.17.1.11",
        "--max-pods": "30",
        "--feature-gates": "MountPropagation=true,TaintNodesByCondition=true",
        <etc...>
    },
    "apiserverConfig": {
        "--storage-backend": "etcd3",
        "--feature-gates": "MountPropagation=true",
        <etc...>
    }
}

The implementation would need to split the feature gates that are specified (in order to apply the Accelerators=true on the agents for >= v1.6.0)

[CecileRobertMichon]

@jackfrancis implemented the apiServerConfig interface in #2012 which was merged yesterday

[jackfrancis]

I think we can re-use the existing (since yesterday!) apiServerConfig implementation like this:

"apiserverConfig": {
    "--storage-backend": "etcd3",
    "--feature-gates": "['key1'='val','key2'='val']",
    <etc...>
}

In other words, --feature-gates would be a special property that we'd deserialize and expand as k8s runtime config separately.

My representation of JSON inside of a JSON string is crude, someone has probably solved that problem before. We should do something like that, in my view.

[stuartleeks]

Thanks @CecileRobertMichon and @jackfrancis! I have started implementing this with the comma-separated format for now, and spotted the apiServerConfig as I was going through making the changes :-)

Once I have a version working I'll submit a PR for review and we can refine the config format along with any other changes from the review - does that sound ok?

[stuartleeks]

I was just thinking about the handling for --feature-gates in the kubeletConfig with master and agent profiles and want to get your opinions.

As I understand the behaviour there, any value that is specified in the masterProfile (as an example) will be used, but if a value is absent from masterProfile and present in the kubernetesConfig/kubeletConfig then the kubernetesConfig/kubeletConfig value is used for the master. I.e. values are inherited unless overridden.

So in the case of the following (pseudo config)

"kubernetesConfig": {
    "kubeletConfig": {
        "--dummysetting1": "shared1",
        "--dummysetting2": "shared2",
        <etc...>
    }
}
"masterProfile" : {
    "kubernetesConfig" : {
        "kubeletConfig" : {
            "--dummysetting1": "master1",
        }
}

the masterProfile that would be applied is

"masterProfile" : {
    "kubernetesConfig" : {
        "kubeletConfig" : {
            "--dummysetting1": "master1",
            "--dummysetting2": "shared2",
        }
}

So... for feature gates, what level should we merge at?

I.e. if our input is as below, what should the output be?

"kubernetesConfig": {
    "kubeletConfig": {
        "--feature-gates": "gate1=true"
        <etc...>
    }
}
"masterProfile" : {
    "kubernetesConfig" : {
        "kubeletConfig" : {
            "--feature-gates": "gate2=true"
        }
}

One option is to simply override to give this effective master config:

"masterProfile" : {
    "kubernetesConfig" : {
        "kubeletConfig" : {
            "--feature-gates": "gate2=true"
        }
}

Another option is to treat the feature gates as another level that should be merged, like the kubeletConfig giving this effective master config:

"masterProfile" : {
    "kubernetesConfig" : {
        "kubeletConfig" : {
            "--feature-gates": "gate1=true,gate2=true"
        }
}

I already have the code for merging (to support adding in Accelerators=true for the agents), so either can easily be achieved - the question is which do you feel is right :-)

[jackfrancis]

• your description of how agent and master override config delivery is implemented is correct
• in terms of where to implement feature-gates, I think from a high level the answer is that we want the following
  • cluster-wide, agent-specific, and master-override use the same implementation (look for --feature-gates property inside kubeletConfig and do special deserialization)
  • cluster-wide, agent-specific, and master-override are able to have distinct --feature-gates config values which should be properly reconciled in the existing kubelet config foo
  • we don't try to tackle merging/interleaving between the three
    • i.e., if cluster-wide --feature-gates is "A,B,C,D" and master-override is "A,B,C,D,E" then those distinct, respective values are declared comprehensively in their appropriate config interfaces
    • i.e., we don't get to say "just add 'E' to the preëxisting 'A,B,C,D' vals present in the cluster-wide config

I hope that makes sense and answers your questions!

[stuartleeks]

@jackfrancis - as the PR stands:

• it uses the same format for kubelet, agent/master-override and apiserver. Currently I've opted for the key1=value1,key2=value2 format that works with the --feature-gates switch.
• it preserves the behaviour of specifying the Accelerators=true feature gate on the agent nodes
• it allows feature gates to be overridden by master or agent profile, without performing any merging

[stuartleeks]

@jackfrancis can you re-open to track changes to kubernetes components (I forgot to update the PR comment to prevent the issue being auto-closed)!

[jackfrancis]

Done.

[campbelldgunn]

@jackfrancis Please when considering --feature-gates to include CoreDNS=true as I am fighting with addon manager cannot get rid of Kube-DNS.

[jackfrancis]

@campbelldgunn the --feature-gates option is wide open (with a few exceptions): you should be able to specify that key/val pair in your api model when building a cluster.

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contribution. Note that acs-engine is deprecated--see https://github.com/Azure/aks-engine instead.