From 7561a0f96e4b2f34685e2cfe6540014d51808a44 Mon Sep 17 00:00:00 2001 From: Artyom Lukianov Date: Wed, 18 Nov 2020 00:19:36 +0200 Subject: [PATCH] memory manager: provide new flag var to parse reserved-memory parameter The new flag will parse the `--reserved-memory` flag straight forward to the []kubeletconfig.MemoryReservation variable instead of parsing it to the middle map representation. It gives us possibility to get rid of a lot of unneeded code and use the single presentation for the reserved-memory. Signed-off-by: Artyom Lukianov --- cmd/kubelet/app/BUILD | 1 - cmd/kubelet/app/options/options.go | 2 +- cmd/kubelet/app/server.go | 61 +----- cmd/kubelet/app/server_test.go | 57 ------ pkg/kubelet/cm/BUILD | 4 +- pkg/kubelet/cm/container_manager.go | 11 +- pkg/util/flag/BUILD | 12 +- pkg/util/flag/flags.go | 104 ++++++++++ pkg/util/flag/flags_test.go | 124 ++++++++++++ .../src/k8s.io/component-base/cli/flag/BUILD | 2 - ...acket_separated_slice_map_string_string.go | 111 ----------- ..._separated_slice_map_string_string_test.go | 178 ------------------ 12 files changed, 249 insertions(+), 418 deletions(-) delete mode 100644 staging/src/k8s.io/component-base/cli/flag/bracket_separated_slice_map_string_string.go delete mode 100644 staging/src/k8s.io/component-base/cli/flag/bracket_separated_slice_map_string_string_test.go diff --git a/cmd/kubelet/app/BUILD b/cmd/kubelet/app/BUILD index 89826ffa84c..bfd511422e2 100644 --- a/cmd/kubelet/app/BUILD +++ b/cmd/kubelet/app/BUILD @@ -18,7 +18,6 @@ go_library( "//cmd/kubelet/app/options:go_default_library", "//pkg/api/legacyscheme:go_default_library", "//pkg/apis/core:go_default_library", - "//pkg/apis/core/v1/helper:go_default_library", "//pkg/capabilities:go_default_library", "//pkg/cloudprovider/providers:go_default_library", "//pkg/credentialprovider:go_default_library", diff --git a/cmd/kubelet/app/options/options.go b/cmd/kubelet/app/options/options.go index 471d083f5a6..0caa42c9830 100644 --- a/cmd/kubelet/app/options/options.go +++ b/cmd/kubelet/app/options/options.go @@ -554,5 +554,5 @@ Runtime log sanitization may introduce significant computation overhead and ther // Memory Manager Flags fs.StringVar(&c.MemoryManagerPolicy, "memory-manager-policy", c.MemoryManagerPolicy, "Memory Manager policy to use. Possible values: 'none', 'static'. Default: 'none'") // TODO: once documentation link is available, replace KEP link with the documentation one. - fs.Var(cliflag.NewBracketSeparatedSliceMapStringString(&c.ReservedMemory), "reserved-memory", "A comma separated list of bracket-enclosed configuration for memory manager (e.g. {numa-node=0, type=memory, limit=1Gi}, {numa-node=1, type=memory, limit=1Gi}). The total sum for each memory type should be equal to the sum of kube-reserved, system-reserved and eviction-threshold. See more details under https://github.com/kubernetes/enhancements/tree/master/keps/sig-node/1769-memory-manager#reserved-memory-flag") + fs.Var(&utilflag.ReservedMemoryVar{Value: &c.ReservedMemory}, "reserved-memory", "A comma separated list of memory reservations for NUMA nodes. (e.g. --reserved-memory 0:memory=1Gi,hugepages-1M=2Gi --reserved-memory 1:memory=2Gi). The total sum for each memory type should be equal to the sum of kube-reserved, system-reserved and eviction-threshold. See more details under https://github.com/kubernetes/enhancements/tree/master/keps/sig-node/1769-memory-manager#reserved-memory-flag") } diff --git a/cmd/kubelet/app/server.go b/cmd/kubelet/app/server.go index 42f0167ea1d..79908ff608a 100644 --- a/cmd/kubelet/app/server.go +++ b/cmd/kubelet/app/server.go @@ -71,7 +71,6 @@ import ( "k8s.io/kubernetes/cmd/kubelet/app/options" "k8s.io/kubernetes/pkg/api/legacyscheme" api "k8s.io/kubernetes/pkg/apis/core" - corev1helper "k8s.io/kubernetes/pkg/apis/core/v1/helper" "k8s.io/kubernetes/pkg/capabilities" "k8s.io/kubernetes/pkg/credentialprovider" "k8s.io/kubernetes/pkg/features" @@ -689,11 +688,6 @@ func run(ctx context.Context, s *options.KubeletServer, kubeDeps *kubelet.Depend klog.Infof("After cpu setting is overwritten, KubeReserved=\"%v\", SystemReserved=\"%v\"", s.KubeReserved, s.SystemReserved) } - reservedMemory, err := parseReservedMemoryConfig(s.ReservedMemory) - if err != nil { - return err - } - kubeReserved, err := parseResourceList(s.KubeReserved) if err != nil { return err @@ -743,7 +737,7 @@ func run(ctx context.Context, s *options.KubeletServer, kubeDeps *kubelet.Depend ExperimentalCPUManagerPolicy: s.CPUManagerPolicy, ExperimentalCPUManagerReconcilePeriod: s.CPUManagerReconcilePeriod.Duration, ExperimentalMemoryManagerPolicy: s.MemoryManagerPolicy, - ExperimentalMemoryManagerReservedMemory: reservedMemory, + ExperimentalMemoryManagerReservedMemory: s.ReservedMemory, ExperimentalPodPidsLimit: s.PodPidsLimit, EnforceCPULimits: s.CPUCFSQuota, CPUCFSQuotaPeriod: s.CPUCFSQuotaPeriod.Duration, @@ -1305,59 +1299,6 @@ func parseResourceList(m map[string]string) (v1.ResourceList, error) { return rl, nil } -func parseReservedMemoryConfig(config []map[string]string) (kubetypes.NUMANodeResources, error) { - if len(config) == 0 { - return nil, nil - } - - const ( - indexKey = "numa-node" - typeKey = "type" - limitKey = "limit" - ) - - keys := []string{indexKey, typeKey, limitKey} - - // check whether all keys are present - for _, m := range config { - for _, key := range keys { - if _, exist := m[key]; !exist { - return nil, fmt.Errorf("key: %s is missing in given ReservedMemory flag: %v", key, config) - } - } - } - - parsed := make(kubetypes.NUMANodeResources, len(config)) - for _, m := range config { - idxInString, _ := m[indexKey] - idx, err := strconv.Atoi(idxInString) - if err != nil || idx < 0 { - return nil, fmt.Errorf("NUMA index conversion error for value: \"%s\"", idxInString) - } - - typeInString, _ := m[typeKey] - v1Type := v1.ResourceName(typeInString) - if v1Type != v1.ResourceMemory && !corev1helper.IsHugePageResourceName(v1Type) { - return nil, fmt.Errorf("memory type conversion error, unknown type: \"%s\"", typeInString) - } - if corev1helper.IsHugePageResourceName(v1Type) { - if _, err := corev1helper.HugePageSizeFromResourceName(v1Type); err != nil { - return nil, fmt.Errorf("memory type conversion error, unknown type: \"%s\"", typeInString) - } - } - - limitInString, _ := m[limitKey] - limit, err := resource.ParseQuantity(limitInString) - if err != nil || limit.Sign() != 1 { - return nil, fmt.Errorf("memory limit conversion error for value \"%s\"", limitInString) - } - parsed[idx] = make(map[v1.ResourceName]resource.Quantity) - parsed[idx][v1Type] = limit - } - - return parsed, nil -} - // BootstrapKubeletConfigController constructs and bootstrap a configuration controller func BootstrapKubeletConfigController(dynamicConfigDir string, transform dynamickubeletconfig.TransformFunc) (*kubeletconfiginternal.KubeletConfiguration, *dynamickubeletconfig.Controller, error) { if !utilfeature.DefaultFeatureGate.Enabled(features.DynamicKubeletConfig) { diff --git a/cmd/kubelet/app/server_test.go b/cmd/kubelet/app/server_test.go index c80d1fd4d27..1db214ab954 100644 --- a/cmd/kubelet/app/server_test.go +++ b/cmd/kubelet/app/server_test.go @@ -61,60 +61,3 @@ func TestValueOfAllocatableResources(t *testing.T) { } } } - -func TestValueOfReservedMemoryConfig(t *testing.T) { - testCases := []struct { - config []map[string]string - errorExpected bool - name string - }{ - { - config: []map[string]string{{"numa-node": "0", "type": "memory", "limit": "2Gi"}}, - errorExpected: false, - name: "Valid resource quantity", - }, - { - config: []map[string]string{{"numa-node": "0", "type": "memory", "limit": "2000m"}, {"numa-node": "1", "type": "memory", "limit": "1Gi"}}, - errorExpected: false, - name: "Valid resource quantity", - }, - { - config: []map[string]string{{"type": "memory", "limit": "2Gi"}}, - errorExpected: true, - name: "Missing key", - }, - { - config: []map[string]string{{"numa-node": "one", "type": "memory", "limit": "2Gi"}}, - errorExpected: true, - name: "Wrong 'numa-node' value", - }, - { - config: []map[string]string{{"numa-node": "0", "type": "not-memory", "limit": "2Gi"}}, - errorExpected: true, - name: "Wrong 'memory' value", - }, - { - config: []map[string]string{{"numa-node": "0", "type": "memory", "limit": "2Gigs"}}, - errorExpected: true, - name: "Wrong 'limit' value", - }, - { - config: []map[string]string{{"numa-node": "-1", "type": "memory", "limit": "2Gigs"}}, - errorExpected: true, - name: "Invalid 'numa-node' number", - }, - } - - for _, test := range testCases { - _, err := parseReservedMemoryConfig(test.config) - if test.errorExpected { - if err == nil { - t.Errorf("%s: error expected", test.name) - } - } else { - if err != nil { - t.Errorf("%s: unexpected error: %v", test.name, err) - } - } - } -} diff --git a/pkg/kubelet/cm/BUILD b/pkg/kubelet/cm/BUILD index 4ac0d10c67b..e90df47ce01 100644 --- a/pkg/kubelet/cm/BUILD +++ b/pkg/kubelet/cm/BUILD @@ -31,6 +31,7 @@ go_library( visibility = ["//visibility:public"], deps = [ "//pkg/features:go_default_library", + "//pkg/kubelet/apis/config:go_default_library", "//pkg/kubelet/cm/cpumanager:go_default_library", "//pkg/kubelet/cm/cpuset:go_default_library", "//pkg/kubelet/cm/memorymanager:go_default_library", @@ -41,7 +42,6 @@ go_library( "//pkg/kubelet/lifecycle:go_default_library", "//pkg/kubelet/pluginmanager/cache:go_default_library", "//pkg/kubelet/status:go_default_library", - "//pkg/kubelet/types:go_default_library", "//pkg/scheduler/framework:go_default_library", "//staging/src/k8s.io/api/core/v1:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/api/resource:go_default_library", @@ -70,6 +70,7 @@ go_library( "//pkg/kubelet/metrics:go_default_library", "//pkg/kubelet/qos:go_default_library", "//pkg/kubelet/stats/pidlimit:go_default_library", + "//pkg/kubelet/types:go_default_library", "//pkg/util/oom:go_default_library", "//pkg/util/procfs:go_default_library", "//pkg/util/sysctl:go_default_library", @@ -130,6 +131,7 @@ go_library( "//pkg/kubelet/metrics:go_default_library", "//pkg/kubelet/qos:go_default_library", "//pkg/kubelet/stats/pidlimit:go_default_library", + "//pkg/kubelet/types:go_default_library", "//pkg/util/oom:go_default_library", "//pkg/util/procfs:go_default_library", "//pkg/util/sysctl:go_default_library", diff --git a/pkg/kubelet/cm/container_manager.go b/pkg/kubelet/cm/container_manager.go index d72c16630ee..e4a71094718 100644 --- a/pkg/kubelet/cm/container_manager.go +++ b/pkg/kubelet/cm/container_manager.go @@ -17,6 +17,9 @@ limitations under the License. package cm import ( + "fmt" + "strconv" + "strings" "time" "k8s.io/apimachinery/pkg/util/sets" @@ -24,6 +27,7 @@ import ( v1 "k8s.io/api/core/v1" internalapi "k8s.io/cri-api/pkg/apis" podresourcesapi "k8s.io/kubelet/pkg/apis/podresources/v1" + kubeletconfig "k8s.io/kubernetes/pkg/kubelet/apis/config" "k8s.io/kubernetes/pkg/kubelet/cm/cpuset" "k8s.io/kubernetes/pkg/kubelet/config" kubecontainer "k8s.io/kubernetes/pkg/kubelet/container" @@ -31,12 +35,7 @@ import ( "k8s.io/kubernetes/pkg/kubelet/lifecycle" "k8s.io/kubernetes/pkg/kubelet/pluginmanager/cache" "k8s.io/kubernetes/pkg/kubelet/status" - kubetypes "k8s.io/kubernetes/pkg/kubelet/types" schedulerframework "k8s.io/kubernetes/pkg/scheduler/framework" - - "fmt" - "strconv" - "strings" ) type ActivePodsFunc func() []*v1.Pod @@ -137,7 +136,7 @@ type NodeConfig struct { ExperimentalTopologyManagerScope string ExperimentalCPUManagerReconcilePeriod time.Duration ExperimentalMemoryManagerPolicy string - ExperimentalMemoryManagerReservedMemory kubetypes.NUMANodeResources + ExperimentalMemoryManagerReservedMemory []kubeletconfig.MemoryReservation ExperimentalPodPidsLimit int64 EnforceCPULimits bool CPUCFSQuotaPeriod time.Duration diff --git a/pkg/util/flag/BUILD b/pkg/util/flag/BUILD index 4e5ac790111..4f8c6b4e94b 100644 --- a/pkg/util/flag/BUILD +++ b/pkg/util/flag/BUILD @@ -11,6 +11,10 @@ go_library( srcs = ["flags.go"], importpath = "k8s.io/kubernetes/pkg/util/flag", deps = [ + "//pkg/apis/core/v1/helper:go_default_library", + "//pkg/kubelet/apis/config:go_default_library", + "//staging/src/k8s.io/api/core/v1:go_default_library", + "//staging/src/k8s.io/apimachinery/pkg/api/resource:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/util/net:go_default_library", "//vendor/github.com/spf13/pflag:go_default_library", "//vendor/k8s.io/utils/net:go_default_library", @@ -34,5 +38,11 @@ go_test( name = "go_default_test", srcs = ["flags_test.go"], embed = [":go_default_library"], - deps = ["//vendor/github.com/spf13/pflag:go_default_library"], + deps = [ + "//pkg/kubelet/apis/config:go_default_library", + "//staging/src/k8s.io/api/core/v1:go_default_library", + "//staging/src/k8s.io/apimachinery/pkg/api/equality:go_default_library", + "//staging/src/k8s.io/apimachinery/pkg/api/resource:go_default_library", + "//vendor/github.com/spf13/pflag:go_default_library", + ], ) diff --git a/pkg/util/flag/flags.go b/pkg/util/flag/flags.go index 7c489e93ab3..9a4241d67bb 100644 --- a/pkg/util/flag/flags.go +++ b/pkg/util/flag/flags.go @@ -19,10 +19,17 @@ package flag import ( "fmt" "net" + "sort" + "strconv" + "strings" "github.com/spf13/pflag" + v1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/resource" utilnet "k8s.io/apimachinery/pkg/util/net" + corev1helper "k8s.io/kubernetes/pkg/apis/core/v1/helper" + kubeletconfig "k8s.io/kubernetes/pkg/kubelet/apis/config" utilsnet "k8s.io/utils/net" ) @@ -32,6 +39,7 @@ var ( _ pflag.Value = &IPVar{} _ pflag.Value = &IPPortVar{} _ pflag.Value = &PortRangeVar{} + _ pflag.Value = &ReservedMemoryVar{} ) // IPVar is used for validating a command line option that represents an IP. It implements the pflag.Value interface @@ -151,3 +159,99 @@ func (v PortRangeVar) String() string { func (v PortRangeVar) Type() string { return "port-range" } + +// ReservedMemoryVar is used for validating a command line option that represents a reserved memory. It implements the pflag.Value interface +type ReservedMemoryVar struct { + Value *[]kubeletconfig.MemoryReservation + initialized bool // set to true after the first Set call +} + +// Set sets the flag value +func (v *ReservedMemoryVar) Set(s string) error { + if v.Value == nil { + return fmt.Errorf("no target (nil pointer to *[]MemoryReservation") + } + + if s == "" { + v.Value = nil + return nil + } + + if !v.initialized || *v.Value == nil { + *v.Value = make([]kubeletconfig.MemoryReservation, 0) + v.initialized = true + } + + if s == "" { + return nil + } + + numaNodeReservation := strings.Split(s, ":") + if len(numaNodeReservation) != 2 { + return fmt.Errorf("the reserved memory has incorrect format, expected numaNodeID:type=quantity[,type=quantity...], got %s", s) + } + + memoryTypeReservations := strings.Split(numaNodeReservation[1], ",") + if len(memoryTypeReservations) < 1 { + return fmt.Errorf("the reserved memory has incorrect format, expected numaNodeID:type=quantity[,type=quantity...], got %s", s) + } + + numaNodeID, err := strconv.Atoi(numaNodeReservation[0]) + if err != nil { + return fmt.Errorf("failed to convert the NUMA node ID, exptected integer, got %s", numaNodeReservation[0]) + } + + memoryReservation := kubeletconfig.MemoryReservation{ + NumaNode: int32(numaNodeID), + Limits: map[v1.ResourceName]resource.Quantity{}, + } + + for _, reservation := range memoryTypeReservations { + limit := strings.Split(reservation, "=") + if len(limit) != 2 { + return fmt.Errorf("the reserved limit has incorrect value, expected type=quantatity, got %s", reservation) + } + + resourceName := v1.ResourceName(limit[0]) + if resourceName != v1.ResourceMemory && !corev1helper.IsHugePageResourceName(resourceName) { + return fmt.Errorf("memory type conversion error, unknown type: %q", resourceName) + } + + q, err := resource.ParseQuantity(limit[1]) + if err != nil { + return fmt.Errorf("failed to parse the quantatity, expected quantatity, got %s", limit[1]) + } + + memoryReservation.Limits[v1.ResourceName(limit[0])] = q + } + + *v.Value = append(*v.Value, memoryReservation) + + return nil +} + +// String returns the flag value +func (v *ReservedMemoryVar) String() string { + if v == nil || v.Value == nil { + return "" + } + + var slices []string + for _, reservedMemory := range *v.Value { + var limits []string + for resourceName, q := range reservedMemory.Limits { + limits = append(limits, fmt.Sprintf("%s=%s", resourceName, q.String())) + } + + sort.Strings(limits) + slices = append(slices, fmt.Sprintf("%d:%s", reservedMemory.NumaNode, strings.Join(limits, ","))) + } + + sort.Strings(slices) + return strings.Join(slices, ",") +} + +// Type gets the flag type +func (v *ReservedMemoryVar) Type() string { + return "reserved-memory" +} diff --git a/pkg/util/flag/flags_test.go b/pkg/util/flag/flags_test.go index b60f8e0ea48..cd1edd0f5e8 100644 --- a/pkg/util/flag/flags_test.go +++ b/pkg/util/flag/flags_test.go @@ -17,10 +17,16 @@ limitations under the License. package flag import ( + "fmt" "strings" "testing" "github.com/spf13/pflag" + + v1 "k8s.io/api/core/v1" + apiequality "k8s.io/apimachinery/pkg/api/equality" + "k8s.io/apimachinery/pkg/api/resource" + kubeletconfig "k8s.io/kubernetes/pkg/kubelet/apis/config" ) func TestIPVar(t *testing.T) { @@ -163,3 +169,121 @@ func TestIPPortVar(t *testing.T) { } } } + +func TestReservedMemoryVar(t *testing.T) { + resourceNameHugepages1Gi := v1.ResourceName(fmt.Sprintf("%s1Gi", v1.ResourceHugePagesPrefix)) + memory1Gi := resource.MustParse("1Gi") + testCases := []struct { + desc string + argc string + expectErr bool + expectVal []kubeletconfig.MemoryReservation + }{ + { + desc: "valid input", + argc: "blah --reserved-memory=0:memory=1Gi", + expectVal: []kubeletconfig.MemoryReservation{ + { + NumaNode: 0, + Limits: v1.ResourceList{ + v1.ResourceMemory: memory1Gi, + }, + }, + }, + }, + { + desc: "valid input with multiple memory types", + argc: "blah --reserved-memory=0:memory=1Gi,hugepages-1Gi=1Gi", + expectVal: []kubeletconfig.MemoryReservation{ + { + NumaNode: 0, + Limits: v1.ResourceList{ + v1.ResourceMemory: memory1Gi, + resourceNameHugepages1Gi: memory1Gi, + }, + }, + }, + }, + { + desc: "valid input with multiple reserved-memory arguments", + argc: "blah --reserved-memory=0:memory=1Gi,hugepages-1Gi=1Gi --reserved-memory=1:memory=1Gi", + expectVal: []kubeletconfig.MemoryReservation{ + { + NumaNode: 0, + Limits: v1.ResourceList{ + v1.ResourceMemory: memory1Gi, + resourceNameHugepages1Gi: memory1Gi, + }, + }, + { + NumaNode: 1, + Limits: v1.ResourceList{ + v1.ResourceMemory: memory1Gi, + }, + }, + }, + }, + { + desc: "invalid input", + argc: "blah --reserved-memory=bad-input", + expectVal: nil, + expectErr: true, + }, + { + desc: "invalid input without memory types", + argc: "blah --reserved-memory=0:", + expectVal: nil, + expectErr: true, + }, + { + desc: "invalid input with non-integer NUMA node", + argc: "blah --reserved-memory=a:memory=1Gi", + expectVal: nil, + expectErr: true, + }, + { + desc: "invalid input with invalid limit", + argc: "blah --reserved-memory=0:memory=", + expectVal: nil, + expectErr: true, + }, + { + desc: "invalid input with invalid memory type", + argc: "blah --reserved-memory=0:type=1Gi", + expectVal: nil, + expectErr: true, + }, + { + desc: "invalid input with invalid quantity", + argc: "blah --reserved-memory=0:memory=1Be", + expectVal: nil, + expectErr: true, + }, + } + for _, tc := range testCases { + fs := pflag.NewFlagSet("blah", pflag.PanicOnError) + + var reservedMemory []kubeletconfig.MemoryReservation + fs.Var(&ReservedMemoryVar{Value: &reservedMemory}, "reserved-memory", "--reserved-memory 0:memory=1Gi,hugepages-1M=2Gi") + + var err error + func() { + defer func() { + if r := recover(); r != nil { + err = r.(error) + } + }() + fs.Parse(strings.Split(tc.argc, " ")) + }() + + if tc.expectErr && err == nil { + t.Fatalf("%q: Did not observe an expected error", tc.desc) + } + if !tc.expectErr && err != nil { + t.Fatalf("%q: Observed an unexpected error: %v", tc.desc, err) + } + if !apiequality.Semantic.DeepEqual(reservedMemory, tc.expectVal) { + t.Fatalf("%q: Unexpected reserved-error: expected %v, saw %v", tc.desc, tc.expectVal, reservedMemory) + } + } +} diff --git a/staging/src/k8s.io/component-base/cli/flag/BUILD b/staging/src/k8s.io/component-base/cli/flag/BUILD index 25d290f7e85..9c52db98989 100644 --- a/staging/src/k8s.io/component-base/cli/flag/BUILD +++ b/staging/src/k8s.io/component-base/cli/flag/BUILD @@ -9,7 +9,6 @@ load( go_test( name = "go_default_test", srcs = [ - "bracket_separated_slice_map_string_string_test.go", "ciphersuites_flag_test.go", "colon_separated_multimap_string_string_test.go", "langle_separated_map_string_string_test.go", @@ -25,7 +24,6 @@ go_test( go_library( name = "go_default_library", srcs = [ - "bracket_separated_slice_map_string_string.go", "ciphersuites_flag.go", "ciphersuites_flag_114.go", "colon_separated_multimap_string_string.go", diff --git a/staging/src/k8s.io/component-base/cli/flag/bracket_separated_slice_map_string_string.go b/staging/src/k8s.io/component-base/cli/flag/bracket_separated_slice_map_string_string.go deleted file mode 100644 index e3a99c872bd..00000000000 --- a/staging/src/k8s.io/component-base/cli/flag/bracket_separated_slice_map_string_string.go +++ /dev/null @@ -1,111 +0,0 @@ -/* -Copyright 2020 The Kubernetes Authors. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package flag - -import ( - "fmt" - "sort" - "strings" -) - -// BracketSeparatedSliceMapStringString can be set from the command line with the format `--flag {key=value, ...}, {...}`. -// Multiple comma-separated key-value pairs in brackets (`{}`) in a single invocation are supported. For example: `--flag {key=value, key=value, ...}`. -// Multiple bracket-separated list of key-value pairs in a single invocation are supported. For example: `--flag {key=value, key=value}, {key=value, key=value}`. -type BracketSeparatedSliceMapStringString struct { - Value *[]map[string]string - initialized bool // set to true after the first Set call -} - -// NewBracketSeparatedSliceMapStringString takes a pointer to a []map[string]string and returns the -// BracketSeparatedSliceMapStringString flag parsing shim for that map -func NewBracketSeparatedSliceMapStringString(m *[]map[string]string) *BracketSeparatedSliceMapStringString { - return &BracketSeparatedSliceMapStringString{Value: m} -} - -// Set implements github.com/spf13/pflag.Value -func (m *BracketSeparatedSliceMapStringString) Set(value string) error { - if m.Value == nil { - return fmt.Errorf("no target (nil pointer to []map[string]string)") - } - if !m.initialized || *m.Value == nil { - *m.Value = make([]map[string]string, 0) - m.initialized = true - } - - value = strings.TrimSpace(value) - - for _, split := range strings.Split(value, ",{") { - split = strings.TrimLeft(split, "{") - split = strings.TrimRight(split, "}") - - if len(split) == 0 { - continue - } - - // now we have "numa-node=1,memory-type=memory,limit=1Gi" - tmpRawMap := make(map[string]string) - - tmpMap := NewMapStringString(&tmpRawMap) - - if err := tmpMap.Set(split); err != nil { - return fmt.Errorf("could not parse String: (%s): %v", value, err) - } - - *m.Value = append(*m.Value, tmpRawMap) - } - - return nil -} - -// String implements github.com/spf13/pflag.Value -func (m *BracketSeparatedSliceMapStringString) String() string { - if m == nil || m.Value == nil { - return "" - } - - var slices []string - - for _, configMap := range *m.Value { - var tmpPairs []string - - var keys []string - for key := range configMap { - keys = append(keys, key) - } - sort.Strings(keys) - - for _, key := range keys { - tmpPairs = append(tmpPairs, fmt.Sprintf("%s=%s", key, configMap[key])) - } - - if len(tmpPairs) != 0 { - slices = append(slices, "{"+strings.Join(tmpPairs, ",")+"}") - } - } - sort.Strings(slices) - return strings.Join(slices, ",") -} - -// Type implements github.com/spf13/pflag.Value -func (*BracketSeparatedSliceMapStringString) Type() string { - return "BracketSeparatedSliceMapStringString" -} - -// Empty implements OmitEmpty -func (m *BracketSeparatedSliceMapStringString) Empty() bool { - return !m.initialized || m.Value == nil || len(*m.Value) == 0 -} diff --git a/staging/src/k8s.io/component-base/cli/flag/bracket_separated_slice_map_string_string_test.go b/staging/src/k8s.io/component-base/cli/flag/bracket_separated_slice_map_string_string_test.go deleted file mode 100644 index 84049d1ebc2..00000000000 --- a/staging/src/k8s.io/component-base/cli/flag/bracket_separated_slice_map_string_string_test.go +++ /dev/null @@ -1,178 +0,0 @@ -/* -Copyright 2020 The Kubernetes Authors. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package flag - -import ( - "reflect" - "testing" -) - -func TestStringBracketSeparatedSliceMapStringString(t *testing.T) { - var nilSliceMap []map[string]string - testCases := []struct { - desc string - m *BracketSeparatedSliceMapStringString - expect string - }{ - {"nil", NewBracketSeparatedSliceMapStringString(&nilSliceMap), ""}, - {"empty", NewBracketSeparatedSliceMapStringString(&[]map[string]string{}), ""}, - {"one key", NewBracketSeparatedSliceMapStringString(&[]map[string]string{{"a": "string"}}), "{a=string}"}, - {"two keys", NewBracketSeparatedSliceMapStringString(&[]map[string]string{{"a": "string", "b": "string"}}), "{a=string,b=string}"}, - } - for _, tc := range testCases { - t.Run(tc.desc, func(t *testing.T) { - str := tc.m.String() - if tc.expect != str { - t.Fatalf("expect %q but got %q", tc.expect, str) - } - }) - } -} - -func TestSetBracketSeparatedSliceMapStringString(t *testing.T) { - var nilMap []map[string]string - testCases := []struct { - desc string - vals []string - start *BracketSeparatedSliceMapStringString - expect *BracketSeparatedSliceMapStringString - err string - }{ - // we initialize the map with a default key that should be cleared by Set - {"clears defaults", []string{""}, - NewBracketSeparatedSliceMapStringString(&[]map[string]string{{"default": ""}}), - &BracketSeparatedSliceMapStringString{ - initialized: true, - Value: &[]map[string]string{}, - }, ""}, - // make sure we still allocate for "initialized" multimaps where Multimap was initially set to a nil map - {"allocates map if currently nil", []string{""}, - &BracketSeparatedSliceMapStringString{initialized: true, Value: &nilMap}, - &BracketSeparatedSliceMapStringString{ - initialized: true, - Value: &[]map[string]string{}, - }, ""}, - // for most cases, we just reuse nilMap, which should be allocated by Set, and is reset before each test case - {"empty", []string{""}, - NewBracketSeparatedSliceMapStringString(&nilMap), - &BracketSeparatedSliceMapStringString{ - initialized: true, - Value: &[]map[string]string{}, - }, ""}, - {"empty bracket", []string{"{}"}, - NewBracketSeparatedSliceMapStringString(&nilMap), - &BracketSeparatedSliceMapStringString{ - initialized: true, - Value: &[]map[string]string{}, - }, ""}, - {"missing bracket", []string{"a=string, b=string"}, - NewBracketSeparatedSliceMapStringString(&nilMap), - &BracketSeparatedSliceMapStringString{ - initialized: true, - Value: &[]map[string]string{{"a": "string", "b": "string"}}, - }, ""}, - {"empty key", []string{"{=string}"}, - NewBracketSeparatedSliceMapStringString(&nilMap), - &BracketSeparatedSliceMapStringString{ - initialized: true, - Value: &[]map[string]string{{"": "string"}}, - }, ""}, - {"one key", []string{"{a=string}"}, - NewBracketSeparatedSliceMapStringString(&nilMap), - &BracketSeparatedSliceMapStringString{ - initialized: true, - Value: &[]map[string]string{{"a": "string"}}, - }, ""}, - {"two keys", []string{"{a=string,b=string}"}, - NewBracketSeparatedSliceMapStringString(&nilMap), - &BracketSeparatedSliceMapStringString{ - initialized: true, - Value: &[]map[string]string{{"a": "string", "b": "string"}}, - }, ""}, - {"two duplicated keys", []string{"{a=string,a=string}"}, - NewBracketSeparatedSliceMapStringString(&nilMap), - &BracketSeparatedSliceMapStringString{ - initialized: true, - Value: &[]map[string]string{{"a": "string"}}, - }, ""}, - {"two keys with spaces", []string{"{a = string, b = string}"}, - NewBracketSeparatedSliceMapStringString(&nilMap), - &BracketSeparatedSliceMapStringString{ - initialized: true, - Value: &[]map[string]string{{"a": "string", "b": "string"}}, - }, ""}, - {"two keys, multiple Set invocations", []string{"{a=string, b=string}", "{a=string, b=string}"}, - NewBracketSeparatedSliceMapStringString(&nilMap), - &BracketSeparatedSliceMapStringString{ - initialized: true, - Value: &[]map[string]string{{"a": "string", "b": "string"}, {"a": "string", "b": "string"}}, - }, ""}, - {"no target", []string{""}, - NewBracketSeparatedSliceMapStringString(nil), - nil, - "no target (nil pointer to []map[string]string)"}, - } - for _, tc := range testCases { - nilMap = nil - t.Run(tc.desc, func(t *testing.T) { - var err error - for _, val := range tc.vals { - err = tc.start.Set(val) - if err != nil { - break - } - } - if tc.err != "" { - if err == nil || err.Error() != tc.err { - t.Fatalf("expect error %s but got %v", tc.err, err) - } - return - } else if err != nil { - t.Fatalf("unexpected error: %v", err) - } - if !reflect.DeepEqual(tc.expect, tc.start) { - t.Fatalf("expect %#v but got %#v", tc.expect, tc.start) - } - }) - } -} - -func TestEmptyBracketSeparatedSliceMapStringString(t *testing.T) { - var nilSliceMap []map[string]string - notEmpty := &BracketSeparatedSliceMapStringString{ - Value: &[]map[string]string{{"a": "int", "b": "string", "c": "string"}}, - initialized: true, - } - - testCases := []struct { - desc string - m *BracketSeparatedSliceMapStringString - expect bool - }{ - {"nil", NewBracketSeparatedSliceMapStringString(&nilSliceMap), true}, - {"empty", NewBracketSeparatedSliceMapStringString(&[]map[string]string{}), true}, - {"populated", notEmpty, false}, - } - for _, tc := range testCases { - t.Run(tc.desc, func(t *testing.T) { - ret := tc.m.Empty() - if ret != tc.expect { - t.Fatalf("expect %t but got %t", tc.expect, ret) - } - }) - } -}