Lift embedded structure out of eviction-related KubeletConfiguration fields

- Changes the following KubeletConfiguration fields from `string` to
`map[string]string`:
  - `EvictionHard`
  - `EvictionSoft`
  - `EvictionSoftGracePeriod`
  - `EvictionMinimumReclaim`
- Adds flag parsing shims to maintain Kubelet's public flags API, while
enabling structured input in the file API.
- Also removes `kubeletconfig.ConfigurationMap`, which was an ad-hoc flag
parsing shim living in the kubeletconfig API group, and replaces it
with the `MapStringString` shim introduced in this PR. Flag parsing
shims belong in a common place, not in the kubeletconfig API.
I manually audited these to ensure that this wouldn't cause errors
parsing the command line for syntax that would have previously been
error free (`kubeletconfig.ConfigurationMap` was unique in that it
allowed keys to be provided on the CLI without values. I believe this was
done in `flags.ConfigurationMap` to facilitate the `--node-labels` flag,
which rightfully accepts value-free keys, and that this shim was then
just copied to `kubeletconfig`). Fortunately, the affected fields
(`ExperimentalQOSReserved`, `SystemReserved`, and `KubeReserved`) expect
non-empty strings in the values of the map, and as a result passing the
empty string is already an error. Thus requiring keys shouldn't break
anyone's scripts.
- Updates code and tests accordingly.

Regarding eviction operators, directionality is already implicit in the
signal type (for a given signal, the decision to evict will be made when
crossing the threshold from either above or below, never both). There is
no need to expose an operator, such as `<`, in the API. By changing
`EvictionHard` and `EvictionSoft` to `map[string]string`, this PR
simplifies the experience of working with these fields via the
`KubeletConfiguration` type. Again, flags stay the same.

Other things:
- There is another flag parsing shim, `flags.ConfigurationMap`, from the
shared flag utility. The `NodeLabels` field still uses
`flags.ConfigurationMap`. This PR moves the allocation of the
`map[string]string` for the `NodeLabels` field from
`AddKubeletConfigFlags` to the defaulter for the external
`KubeletConfiguration` type. Flags are layered on top of an internal
object that has undergone conversion from a defaulted external object,
which means that previously the mere registration of flags would have
overwritten any previously-defined defaults for `NodeLabels` (fortunately
there were none).
This commit is contained in:
Michael Taufen
2017-10-19 15:42:07 -07:00
parent a82460d772
commit 1085b6f730
28 changed files with 871 additions and 356 deletions

View File

@@ -23,52 +23,49 @@ import (
"testing"
"k8s.io/api/core/v1"
"k8s.io/kubernetes/pkg/kubelet/apis/kubeletconfig"
)
func Test(t *testing.T) {
tests := []struct {
input string
input map[string]string
expected *map[v1.ResourceName]int64
}{
{
input: "memory",
input: map[string]string{"memory": ""},
expected: nil,
},
{
input: "memory=a",
input: map[string]string{"memory": "a"},
expected: nil,
},
{
input: "memory=a%",
input: map[string]string{"memory": "a%"},
expected: nil,
},
{
input: "memory=200%",
input: map[string]string{"memory": "200%"},
expected: nil,
},
{
input: "memory=0%",
input: map[string]string{"memory": "0%"},
expected: &map[v1.ResourceName]int64{
v1.ResourceMemory: 0,
},
},
{
input: "memory=100%",
input: map[string]string{"memory": "100%"},
expected: &map[v1.ResourceName]int64{
v1.ResourceMemory: 100,
},
},
{
// need to change this when CPU is added as a supported resource
input: "memory=100%,cpu=50%",
input: map[string]string{"memory": "100%", "cpu": "50%"},
expected: nil,
},
}
for _, test := range tests {
m := kubeletconfig.ConfigurationMap{}
m.Set(test.input)
actual, err := ParseQOSReserved(m)
actual, err := ParseQOSReserved(test.input)
if actual != nil && test.expected == nil {
t.Errorf("Unexpected success, input: %v, expected: %v, actual: %v, err: %v", test.input, test.expected, actual, err)
}