diff --git a/cluster-autoscaler/cloudprovider/clusterapi/README.md b/cluster-autoscaler/cloudprovider/clusterapi/README.md index 6b2b1ec611fa..3f12c397fef4 100644 --- a/cluster-autoscaler/cloudprovider/clusterapi/README.md +++ b/cluster-autoscaler/cloudprovider/clusterapi/README.md @@ -31,6 +31,37 @@ Please note, this example only shows the cloud provider options, you will most likely need other command line flags. For more information you can invoke `cluster-autoscaler --help` to see a full list of options. +## Configuring node group auto discovery + +If you do not configure node group auto discovery, cluster autoscaler will attempt +to match nodes against any scalable resources found in any namespace and belonging +to any Cluster. + +Limiting cluster autoscaler to only match against resources in the blue namespace + +``` +--node-group-auto-discovery=clusterapi:namespace=blue +``` + +Limiting cluster autoscaler to only match against resources belonging to Cluster test1 + +``` +--node-group-auto-discovery=clusterapi:clusterName=test1 +``` + +Limiting cluster autoscaler to only match against resources matching the provided labels + +``` +--node-group-auto-discovery=clusterapi:color=green,shape=square +``` + +These can be mixed and matched in any combination, for example to only match resources +in the staging namespace, belonging to the purple cluster, with the label owner=jim: + +``` +--node-group-auto-discovery=clusterapi:namespace=staging,clusterName=purple,owner=jim +``` + ## Enabling Autoscaling To enable the automatic scaling of components in your cluster-api managed diff --git a/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_autodiscovery.go b/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_autodiscovery.go new file mode 100644 index 000000000000..b54f845a2f47 --- /dev/null +++ b/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_autodiscovery.go @@ -0,0 +1,99 @@ +/* +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 clusterapi + +import ( + "fmt" + "strings" + + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/labels" + "k8s.io/apimachinery/pkg/selection" + + "k8s.io/autoscaler/cluster-autoscaler/utils/errors" +) + +type clusterAPIAutoDiscoveryConfig struct { + clusterName string + namespace string + labelSelector labels.Selector +} + +func parseAutoDiscoverySpec(spec string) (*clusterAPIAutoDiscoveryConfig, error) { + cfg := &clusterAPIAutoDiscoveryConfig{ + labelSelector: labels.NewSelector(), + } + + tokens := strings.Split(spec, ":") + if len(tokens) != 2 { + return cfg, errors.NewAutoscalerError(errors.ConfigurationError, fmt.Sprintf("spec \"%s\" should be discoverer:key=value,key=value", spec)) + } + discoverer := tokens[0] + if discoverer != autoDiscovererTypeClusterAPI { + return cfg, errors.NewAutoscalerError(errors.ConfigurationError, fmt.Sprintf("unsupported discoverer specified: %s", discoverer)) + } + + for _, arg := range strings.Split(tokens[1], ",") { + if len(arg) == 0 { + continue + } + kv := strings.Split(arg, "=") + if len(kv) != 2 { + return cfg, errors.NewAutoscalerError(errors.ConfigurationError, fmt.Sprintf("invalid key=value pair %s", kv)) + } + k, v := kv[0], kv[1] + + switch k { + case autoDiscovererClusterNameKey: + cfg.clusterName = v + case autoDiscovererNamespaceKey: + cfg.namespace = v + default: + req, err := labels.NewRequirement(k, selection.Equals, []string{v}) + if err != nil { + return cfg, errors.NewAutoscalerError(errors.ConfigurationError, fmt.Sprintf("failed to create label selector; %v", err)) + } + cfg.labelSelector = cfg.labelSelector.Add(*req) + } + } + return cfg, nil +} + +func parseAutoDiscovery(specs []string) ([]*clusterAPIAutoDiscoveryConfig, error) { + result := make([]*clusterAPIAutoDiscoveryConfig, 0, len(specs)) + for _, spec := range specs { + autoDiscoverySpec, err := parseAutoDiscoverySpec(spec) + if err != nil { + return result, err + } + result = append(result, autoDiscoverySpec) + } + return result, nil +} + +func allowedByAutoDiscoverySpec(spec *clusterAPIAutoDiscoveryConfig, r *unstructured.Unstructured) bool { + switch { + case spec.namespace != "" && spec.namespace != r.GetNamespace(): + return false + case spec.clusterName != "" && spec.clusterName != clusterNameFromResource(r): + return false + case !spec.labelSelector.Matches(labels.Set(r.GetLabels())): + return false + default: + return true + } +} diff --git a/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_autodiscovery_test.go b/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_autodiscovery_test.go new file mode 100644 index 000000000000..98f4426f3130 --- /dev/null +++ b/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_autodiscovery_test.go @@ -0,0 +1,324 @@ +/* +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 clusterapi + +import ( + "reflect" + "testing" + + "k8s.io/apimachinery/pkg/labels" +) + +func Test_parseAutoDiscoverySpec(t *testing.T) { + for _, tc := range []struct { + name string + spec string + want *clusterAPIAutoDiscoveryConfig + wantErr bool + }{{ + name: "missing ':'", + spec: "foo", + wantErr: true, + }, { + name: "wrong provider given", + spec: "asg:tag=k8s.io/cluster-autoscaler/enabled,k8s.io/cluster-autoscaler/clustername", + wantErr: true, + }, { + name: "invalid key/value pair given", + spec: "clusterapi:invalid", + wantErr: true, + }, { + name: "no attributes specified", + spec: "clusterapi:", + want: &clusterAPIAutoDiscoveryConfig{ + labelSelector: labels.NewSelector(), + }, + wantErr: false, + }, { + name: "only clusterName given", + spec: "clusterapi:clusterName=foo", + want: &clusterAPIAutoDiscoveryConfig{ + clusterName: "foo", + labelSelector: labels.NewSelector(), + }, + wantErr: false, + }, { + name: "only namespace given", + spec: "clusterapi:namespace=default", + want: &clusterAPIAutoDiscoveryConfig{ + namespace: "default", + labelSelector: labels.NewSelector(), + }, + wantErr: false, + }, { + name: "no clustername or namespace given, key provided without value", + spec: "clusterapi:mylabel=", + want: &clusterAPIAutoDiscoveryConfig{ + labelSelector: labels.SelectorFromSet(labels.Set{"mylabel": ""}), + }, + wantErr: false, + }, { + name: "no clustername or namespace given, single key/value pair for labels", + spec: "clusterapi:mylabel=myval", + want: &clusterAPIAutoDiscoveryConfig{ + labelSelector: labels.SelectorFromSet(labels.Set{"mylabel": "myval"}), + }, + wantErr: false, + }, { + name: "no clustername or namespace given, multiple key/value pair for labels", + spec: "clusterapi:color=blue,shape=square", + want: &clusterAPIAutoDiscoveryConfig{ + labelSelector: labels.SelectorFromSet(labels.Set{"color": "blue", "shape": "square"}), + }, + wantErr: false, + }, { + name: "no clustername given, multiple key/value pair for labels", + spec: "clusterapi:namespace=test,color=blue,shape=square", + want: &clusterAPIAutoDiscoveryConfig{ + namespace: "test", + labelSelector: labels.SelectorFromSet(labels.Set{"color": "blue", "shape": "square"}), + }, + wantErr: false, + }, { + name: "no clustername given, single key/value pair for labels", + spec: "clusterapi:namespace=test,color=blue", + want: &clusterAPIAutoDiscoveryConfig{ + namespace: "test", + labelSelector: labels.SelectorFromSet(labels.Set{"color": "blue"}), + }, + wantErr: false, + }, { + name: "no namespace given, multiple key/value pair for labels", + spec: "clusterapi:clusterName=foo,color=blue,shape=square", + want: &clusterAPIAutoDiscoveryConfig{ + clusterName: "foo", + labelSelector: labels.SelectorFromSet(labels.Set{"color": "blue", "shape": "square"}), + }, + wantErr: false, + }, { + name: "no namespace given, single key/value pair for labels", + spec: "clusterapi:clusterName=foo,shape=square", + want: &clusterAPIAutoDiscoveryConfig{ + clusterName: "foo", + labelSelector: labels.SelectorFromSet(labels.Set{"shape": "square"}), + }, + wantErr: false, + }, { + name: "clustername, namespace, multiple key/value pair for labels provided", + spec: "clusterapi:namespace=test,color=blue,shape=square,clusterName=foo", + want: &clusterAPIAutoDiscoveryConfig{ + namespace: "test", + clusterName: "foo", + labelSelector: labels.SelectorFromSet(labels.Set{"color": "blue", "shape": "square"}), + }, + wantErr: false, + }, { + name: "clustername, namespace, single key/value pair for labels provided", + spec: "clusterapi:namespace=test,color=blue,clusterName=foo", + want: &clusterAPIAutoDiscoveryConfig{ + namespace: "test", + clusterName: "foo", + labelSelector: labels.SelectorFromSet(labels.Set{"color": "blue"}), + }, + wantErr: false, + }} { + t.Run(tc.name, func(t *testing.T) { + got, err := parseAutoDiscoverySpec(tc.spec) + if (err != nil) != tc.wantErr { + t.Errorf("parseAutoDiscoverySpec() error = %v, wantErr %v", err, tc.wantErr) + return + } + if err == nil && !reflect.DeepEqual(got, tc.want) { + t.Errorf("parseAutoDiscoverySpec() got = %v, want %v", got, tc.want) + } + }) + } +} + +func Test_parseAutoDiscovery(t *testing.T) { + for _, tc := range []struct { + name string + spec []string + want []*clusterAPIAutoDiscoveryConfig + wantErr bool + }{{ + name: "contains invalid spec", + spec: []string{"foo", "clusterapi:color=green"}, + wantErr: true, + }, { + name: "clustername, namespace, single key/value pair for labels provided", + spec: []string{ + "clusterapi:namespace=test,color=blue,clusterName=foo", + "clusterapi:namespace=default,color=green,clusterName=bar", + }, + want: []*clusterAPIAutoDiscoveryConfig{ + { + namespace: "test", + clusterName: "foo", + labelSelector: labels.SelectorFromSet(labels.Set{"color": "blue"}), + }, + { + namespace: "default", + clusterName: "bar", + labelSelector: labels.SelectorFromSet(labels.Set{"color": "green"}), + }, + }, + wantErr: false, + }} { + t.Run(tc.name, func(t *testing.T) { + got, err := parseAutoDiscovery(tc.spec) + if (err != nil) != tc.wantErr { + t.Errorf("parseAutoDiscoverySpec() error = %v, wantErr %v", err, tc.wantErr) + return + } + if len(got) != len(tc.want) { + t.Errorf("parseAutoDiscoverySpec() expected length of got to be = %v, got %v", len(tc.want), len(got)) + } + if err == nil && !reflect.DeepEqual(got, tc.want) { + t.Errorf("parseAutoDiscoverySpec() got = %v, want %v", got, tc.want) + } + }) + } +} + +func Test_allowedByAutoDiscoverySpec(t *testing.T) { + for _, tc := range []struct { + name string + testSpec testSpec + autoDiscoveryConfig *clusterAPIAutoDiscoveryConfig + additionalLabels map[string]string + shouldMatch bool + }{{ + name: "no clustername, namespace, or label selector specified should match any MachineSet", + testSpec: createTestSpec(RandomString(6), RandomString(6), RandomString(6), 1, false, nil), + autoDiscoveryConfig: &clusterAPIAutoDiscoveryConfig{labelSelector: labels.NewSelector()}, + shouldMatch: true, + }, { + name: "no clustername, namespace, or label selector specified should match any MachineDeployment", + testSpec: createTestSpec(RandomString(6), RandomString(6), RandomString(6), 1, true, nil), + autoDiscoveryConfig: &clusterAPIAutoDiscoveryConfig{labelSelector: labels.NewSelector()}, + shouldMatch: true, + }, { + name: "clustername specified does not match MachineSet, namespace matches, no labels specified", + testSpec: createTestSpec("default", RandomString(6), RandomString(6), 1, false, nil), + autoDiscoveryConfig: &clusterAPIAutoDiscoveryConfig{ + clusterName: "foo", + namespace: "default", + labelSelector: labels.NewSelector(), + }, + shouldMatch: false, + }, { + name: "clustername specified does not match MachineDeployment, namespace matches, no labels specified", + testSpec: createTestSpec("default", RandomString(6), RandomString(6), 1, true, nil), + autoDiscoveryConfig: &clusterAPIAutoDiscoveryConfig{ + clusterName: "foo", + namespace: "default", + labelSelector: labels.NewSelector(), + }, + shouldMatch: false, + }, { + name: "namespace specified does not match MachineSet, clusterName matches, no labels specified", + testSpec: createTestSpec(RandomString(6), "foo", RandomString(6), 1, false, nil), + autoDiscoveryConfig: &clusterAPIAutoDiscoveryConfig{ + clusterName: "foo", + namespace: "default", + labelSelector: labels.NewSelector(), + }, + shouldMatch: false, + }, { + name: "clustername specified does not match MachineDeployment, namespace matches, no labels specified", + testSpec: createTestSpec(RandomString(6), "foo", RandomString(6), 1, true, nil), + autoDiscoveryConfig: &clusterAPIAutoDiscoveryConfig{ + clusterName: "foo", + namespace: "default", + labelSelector: labels.NewSelector(), + }, + shouldMatch: false, + }, { + name: "namespace and clusterName matches MachineSet, no labels specified", + testSpec: createTestSpec("default", "foo", RandomString(6), 1, false, nil), + autoDiscoveryConfig: &clusterAPIAutoDiscoveryConfig{ + clusterName: "foo", + namespace: "default", + labelSelector: labels.NewSelector(), + }, + shouldMatch: true, + }, { + name: "namespace and clusterName matches MachineDeployment, no labels specified", + testSpec: createTestSpec("default", "foo", RandomString(6), 1, true, nil), + autoDiscoveryConfig: &clusterAPIAutoDiscoveryConfig{ + clusterName: "foo", + namespace: "default", + labelSelector: labels.NewSelector(), + }, + shouldMatch: true, + }, { + name: "namespace and clusterName matches MachineSet, does not match label selector", + testSpec: createTestSpec("default", "foo", RandomString(6), 1, false, nil), + autoDiscoveryConfig: &clusterAPIAutoDiscoveryConfig{ + clusterName: "foo", + namespace: "default", + labelSelector: labels.SelectorFromSet(labels.Set{"color": "green"}), + }, + shouldMatch: false, + }, { + name: "namespace and clusterName matches MachineDeployment, does not match label selector", + testSpec: createTestSpec("default", "foo", RandomString(6), 1, true, nil), + autoDiscoveryConfig: &clusterAPIAutoDiscoveryConfig{ + clusterName: "foo", + namespace: "default", + labelSelector: labels.SelectorFromSet(labels.Set{"color": "green"}), + }, + shouldMatch: false, + }, { + name: "namespace, clusterName, and label selector matches MachineSet", + testSpec: createTestSpec("default", "foo", RandomString(6), 1, false, nil), + additionalLabels: map[string]string{"color": "green"}, + autoDiscoveryConfig: &clusterAPIAutoDiscoveryConfig{ + clusterName: "foo", + namespace: "default", + labelSelector: labels.SelectorFromSet(labels.Set{"color": "green"}), + }, + shouldMatch: true, + }, { + name: "namespace, clusterName, and label selector matches MachineDeployment", + testSpec: createTestSpec("default", "foo", RandomString(6), 1, true, nil), + additionalLabels: map[string]string{"color": "green"}, + autoDiscoveryConfig: &clusterAPIAutoDiscoveryConfig{ + clusterName: "foo", + namespace: "default", + labelSelector: labels.SelectorFromSet(labels.Set{"color": "green"}), + }, + shouldMatch: true, + }} { + t.Run(tc.name, func(t *testing.T) { + testConfigs := createTestConfigs(tc.testSpec) + resource := testConfigs[0].machineSet + if tc.testSpec.rootIsMachineDeployment { + resource = testConfigs[0].machineDeployment + } + if tc.additionalLabels != nil { + resource.SetLabels(labels.Merge(resource.GetLabels(), tc.additionalLabels)) + } + got := allowedByAutoDiscoverySpec(tc.autoDiscoveryConfig, resource) + + if got != tc.shouldMatch { + t.Errorf("allowedByAutoDiscoverySpec got = %v, want %v", got, tc.shouldMatch) + } + }) + } +} diff --git a/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_controller.go b/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_controller.go index 473d47a9e5cc..1bffec3e372d 100644 --- a/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_controller.go +++ b/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_controller.go @@ -37,6 +37,8 @@ import ( "k8s.io/client-go/scale" "k8s.io/client-go/tools/cache" klog "k8s.io/klog/v2" + + "k8s.io/autoscaler/cluster-autoscaler/cloudprovider" ) const ( @@ -52,6 +54,9 @@ const ( machineDeploymentKind = "MachineDeployment" machineSetKind = "MachineSet" machineKind = "Machine" + autoDiscovererTypeClusterAPI = "clusterapi" + autoDiscovererClusterNameKey = "clusterName" + autoDiscovererNamespaceKey = "namespace" ) // machineController watches for Nodes, Machines, MachineSets and @@ -72,6 +77,7 @@ type machineController struct { machineDeploymentResource schema.GroupVersionResource machineDeploymentsAvailable bool accessLock sync.Mutex + autoDiscoverySpecs []*clusterAPIAutoDiscoveryConfig } func indexMachineByProviderID(obj interface{}) ([]string, error) { @@ -80,7 +86,7 @@ func indexMachineByProviderID(obj interface{}) ([]string, error) { return nil, nil } - providerID, found, err := unstructured.NestedString(u.Object, "spec", "providerID") + providerID, found, err := unstructured.NestedString(u.UnstructuredContent(), "spec", "providerID") if err != nil || !found { return nil, nil } @@ -102,18 +108,18 @@ func indexNodeByProviderID(obj interface{}) ([]string, error) { } func (c *machineController) findMachine(id string) (*unstructured.Unstructured, error) { - return findResourceByKey(c.machineInformer.Informer().GetStore(), id) + return c.findResourceByKey(c.machineInformer.Informer().GetStore(), id) } func (c *machineController) findMachineSet(id string) (*unstructured.Unstructured, error) { - return findResourceByKey(c.machineSetInformer.Informer().GetStore(), id) + return c.findResourceByKey(c.machineSetInformer.Informer().GetStore(), id) } func (c *machineController) findMachineDeployment(id string) (*unstructured.Unstructured, error) { - return findResourceByKey(c.machineDeploymentInformer.Informer().GetStore(), id) + return c.findResourceByKey(c.machineDeploymentInformer.Informer().GetStore(), id) } -func findResourceByKey(store cache.Store, key string) (*unstructured.Unstructured, error) { +func (c *machineController) findResourceByKey(store cache.Store, key string) (*unstructured.Unstructured, error) { item, exists, err := store.GetByKey(key) if err != nil { return nil, err @@ -128,6 +134,11 @@ func findResourceByKey(store cache.Store, key string) (*unstructured.Unstructure return nil, fmt.Errorf("internal error; unexpected type: %T", item) } + // Verify the resource is allowed by the autodiscovery configuration + if !c.allowedByAutoDiscoverySpecs(u) { + return nil, nil + } + return u.DeepCopy(), nil } @@ -300,10 +311,16 @@ func newMachineController( workloadClient kubeclient.Interface, managementDiscoveryClient discovery.DiscoveryInterface, managementScaleClient scale.ScalesGetter, + discoveryOpts cloudprovider.NodeGroupDiscoveryOptions, ) (*machineController, error) { workloadInformerFactory := kubeinformers.NewSharedInformerFactory(workloadClient, 0) managementInformerFactory := dynamicinformer.NewFilteredDynamicSharedInformerFactory(managementClient, 0, metav1.NamespaceAll, nil) + autoDiscoverySpecs, err := parseAutoDiscovery(discoveryOpts.NodeGroupAutoDiscoverySpecs) + if err != nil { + return nil, fmt.Errorf("failed to parse auto discovery configuration: %v", err) + } + CAPIGroup := getCAPIGroup() CAPIVersion, err := getAPIGroupPreferredVersion(managementDiscoveryClient, CAPIGroup) if err != nil { @@ -363,6 +380,7 @@ func newMachineController( } return &machineController{ + autoDiscoverySpecs: autoDiscoverySpecs, workloadInformerFactory: workloadInformerFactory, managementInformerFactory: managementInformerFactory, machineDeploymentInformer: machineDeploymentInformer, @@ -416,7 +434,7 @@ func (c *machineController) scalableResourceProviderIDs(scalableResource *unstru var providerIDs []string for _, machine := range machines { - providerID, found, err := unstructured.NestedString(machine.Object, "spec", "providerID") + providerID, found, err := unstructured.NestedString(machine.UnstructuredContent(), "spec", "providerID") if err != nil { return nil, err } @@ -446,7 +464,7 @@ func (c *machineController) scalableResourceProviderIDs(scalableResource *unstru continue } - _, found, err = unstructured.NestedFieldCopy(machine.Object, "status", "nodeRef") + _, found, err = unstructured.NestedFieldCopy(machine.UnstructuredContent(), "status", "nodeRef") if err != nil { return nil, err } @@ -456,7 +474,7 @@ func (c *machineController) scalableResourceProviderIDs(scalableResource *unstru continue } - nodeRefKind, found, err := unstructured.NestedString(machine.Object, "status", "nodeRef", "kind") + nodeRefKind, found, err := unstructured.NestedString(machine.UnstructuredContent(), "status", "nodeRef", "kind") if err != nil { return nil, err } @@ -466,7 +484,7 @@ func (c *machineController) scalableResourceProviderIDs(scalableResource *unstru continue } - nodeRefName, found, err := unstructured.NestedString(machine.Object, "status", "nodeRef", "name") + nodeRefName, found, err := unstructured.NestedString(machine.UnstructuredContent(), "status", "nodeRef", "name") if err != nil { return nil, err } @@ -497,28 +515,13 @@ func (c *machineController) nodeGroups() ([]*nodegroup, error) { nodegroups := make([]*nodegroup, 0, len(scalableResources)) for _, r := range scalableResources { - ng, err := newNodegroupFromScalableResource(c, r) + ng, err := newNodeGroupFromScalableResource(c, r) if err != nil { return nil, err } - // add nodegroup iff it has the capacity to scale - if ng.MaxSize()-ng.MinSize() > 0 { - // if the node group can scale from zero there is no need to check the replicas against 0, it should be added - if ng.scalableResource.CanScaleFromZero() { - nodegroups = append(nodegroups, ng) - continue - } - - // since this is relying on a scale resource we should never get a nil here - replicas, found, err := unstructured.NestedInt64(r.Object, "spec", "replicas") - if err != nil { - return nil, err - } - - if found && replicas > 0 { - nodegroups = append(nodegroups, ng) - } + if ng != nil { + nodegroups = append(nodegroups, ng) } } return nodegroups, nil @@ -533,12 +536,14 @@ func (c *machineController) nodeGroupForNode(node *corev1.Node) (*nodegroup, err return nil, nil } - nodegroup, err := newNodegroupFromScalableResource(c, scalableResource) + nodegroup, err := newNodeGroupFromScalableResource(c, scalableResource) if err != nil { return nil, fmt.Errorf("failed to build nodegroup for node %q: %v", node.Name, err) } - if nodegroup.MaxSize()-nodegroup.MinSize() < 1 { + // the nodegroup will be nil if it doesn't match the autodiscovery configuration + // or if it doesn't meet the scaling requirements + if nodegroup == nil { return nil, nil } @@ -573,7 +578,7 @@ func (c *machineController) findNodeByProviderID(providerID normalizedProviderID func (c *machineController) listMachinesForScalableResource(r *unstructured.Unstructured) ([]*unstructured.Unstructured, error) { switch r.GetKind() { case machineSetKind, machineDeploymentKind: - unstructuredSelector, found, err := unstructured.NestedMap(r.Object, "spec", "selector") + unstructuredSelector, found, err := unstructured.NestedMap(r.UnstructuredContent(), "spec", "selector") if err != nil { return nil, err } @@ -592,7 +597,7 @@ func (c *machineController) listMachinesForScalableResource(r *unstructured.Unst return nil, err } - return listResources(c.machineInformer.Lister().ByNamespace(r.GetNamespace()), selector) + return listResources(c.machineInformer.Lister().ByNamespace(r.GetNamespace()), clusterNameFromResource(r), selector) default: return nil, fmt.Errorf("unknown scalable resource kind %s", r.GetKind()) } @@ -616,10 +621,31 @@ func (c *machineController) listScalableResources() ([]*unstructured.Unstructure } func (c *machineController) listResources(lister cache.GenericLister) ([]*unstructured.Unstructured, error) { - return listResources(lister.ByNamespace(metav1.NamespaceAll), labels.Everything()) + if len(c.autoDiscoverySpecs) == 0 { + return listResources(lister.ByNamespace(metav1.NamespaceAll), "", labels.Everything()) + } + + var results []*unstructured.Unstructured + tracker := map[string]bool{} + for _, spec := range c.autoDiscoverySpecs { + resources, err := listResources(lister.ByNamespace(spec.namespace), spec.clusterName, spec.labelSelector) + if err != nil { + return nil, err + } + for i := range resources { + r := resources[i] + key := fmt.Sprintf("%s-%s-%s", r.GetKind(), r.GetNamespace(), r.GetName()) + if _, ok := tracker[key]; !ok { + results = append(results, r) + tracker[key] = true + } + } + } + + return results, nil } -func listResources(lister cache.GenericNamespaceLister, selector labels.Selector) ([]*unstructured.Unstructured, error) { +func listResources(lister cache.GenericNamespaceLister, clusterName string, selector labels.Selector) ([]*unstructured.Unstructured, error) { objs, err := lister.List(selector) if err != nil { return nil, err @@ -632,6 +658,11 @@ func listResources(lister cache.GenericNamespaceLister, selector labels.Selector return nil, fmt.Errorf("expected unstructured resource from lister, not %T", x) } + // if clusterName is not empty and the clusterName does not match the resource, do not return it as part of the results + if clusterName != "" && clusterNameFromResource(u) != clusterName { + continue + } + // if we are listing MachineSets, do not return MachineSets that are owned by a MachineDeployment if u.GetKind() == machineSetKind && machineSetHasMachineDeploymentOwnerRef(u) { continue @@ -642,3 +673,18 @@ func listResources(lister cache.GenericNamespaceLister, selector labels.Selector return results, nil } + +func (c *machineController) allowedByAutoDiscoverySpecs(r *unstructured.Unstructured) bool { + // If no autodiscovery configuration fall back to previous behavior of allowing all + if len(c.autoDiscoverySpecs) == 0 { + return true + } + + for _, spec := range c.autoDiscoverySpecs { + if allowedByAutoDiscoverySpec(spec, r) { + return true + } + } + + return false +} diff --git a/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_controller_test.go b/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_controller_test.go index 2913e833273c..6a841e42b908 100644 --- a/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_controller_test.go +++ b/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_controller_test.go @@ -43,12 +43,16 @@ import ( fakekube "k8s.io/client-go/kubernetes/fake" fakescale "k8s.io/client-go/scale/fake" clientgotesting "k8s.io/client-go/testing" + + "k8s.io/autoscaler/cluster-autoscaler/cloudprovider" ) type testControllerShutdownFunc func() type testConfig struct { spec *testSpec + clusterName string + namespace string machineDeployment *unstructured.Unstructured machineSet *unstructured.Unstructured machines []*unstructured.Unstructured @@ -59,6 +63,7 @@ type testSpec struct { annotations map[string]string machineDeploymentName string machineSetName string + clusterName string namespace string nodeCount int rootIsMachineDeployment bool @@ -162,7 +167,7 @@ func mustCreateTestController(t *testing.T, testConfigs ...*testConfig) (*machin return true, nil, err } - replicas, found, err := unstructured.NestedInt64(u.Object, "spec", "replicas") + replicas, found, err := unstructured.NestedInt64(u.UnstructuredContent(), "spec", "replicas") if err != nil { return true, nil, err } @@ -214,7 +219,7 @@ func mustCreateTestController(t *testing.T, testConfigs ...*testConfig) (*machin } scaleClient.AddReactor("*", "*", scaleReactor) - controller, err := newMachineController(dynamicClientset, kubeclientSet, discoveryClient, scaleClient) + controller, err := newMachineController(dynamicClientset, kubeclientSet, discoveryClient, scaleClient, cloudprovider.NodeGroupDiscoveryOptions{}) if err != nil { t.Fatal("failed to create test controller") } @@ -229,50 +234,58 @@ func mustCreateTestController(t *testing.T, testConfigs ...*testConfig) (*machin } } -func createMachineSetTestConfig(namespace, namePrefix string, nodeCount int, annotations map[string]string) *testConfig { - return createTestConfigs(createTestSpecs(namespace, namePrefix, 1, nodeCount, false, annotations)...)[0] +func createMachineSetTestConfig(namespace, clusterName, namePrefix string, nodeCount int, annotations map[string]string) *testConfig { + return createTestConfigs(createTestSpecs(namespace, clusterName, namePrefix, 1, nodeCount, false, annotations)...)[0] } -func createMachineSetTestConfigs(namespace, namePrefix string, configCount, nodeCount int, annotations map[string]string) []*testConfig { - return createTestConfigs(createTestSpecs(namespace, namePrefix, configCount, nodeCount, false, annotations)...) +func createMachineSetTestConfigs(namespace, clusterName, namePrefix string, configCount, nodeCount int, annotations map[string]string) []*testConfig { + return createTestConfigs(createTestSpecs(namespace, clusterName, namePrefix, configCount, nodeCount, false, annotations)...) } -func createMachineDeploymentTestConfig(namespace, namePrefix string, nodeCount int, annotations map[string]string) *testConfig { - return createTestConfigs(createTestSpecs(namespace, namePrefix, 1, nodeCount, true, annotations)...)[0] +func createMachineDeploymentTestConfig(namespace, clusterName, namePrefix string, nodeCount int, annotations map[string]string) *testConfig { + return createTestConfigs(createTestSpecs(namespace, clusterName, namePrefix, 1, nodeCount, true, annotations)...)[0] } -func createMachineDeploymentTestConfigs(namespace, namePrefix string, configCount, nodeCount int, annotations map[string]string) []*testConfig { - return createTestConfigs(createTestSpecs(namespace, namePrefix, configCount, nodeCount, true, annotations)...) +func createMachineDeploymentTestConfigs(namespace, clusterName, namePrefix string, configCount, nodeCount int, annotations map[string]string) []*testConfig { + return createTestConfigs(createTestSpecs(namespace, clusterName, namePrefix, configCount, nodeCount, true, annotations)...) } -func createTestSpecs(namespace, namePrefix string, scalableResourceCount, nodeCount int, isMachineDeployment bool, annotations map[string]string) []testSpec { +func createTestSpecs(namespace, clusterName, namePrefix string, scalableResourceCount, nodeCount int, isMachineDeployment bool, annotations map[string]string) []testSpec { var specs []testSpec for i := 0; i < scalableResourceCount; i++ { - specs = append(specs, testSpec{ - annotations: annotations, - machineDeploymentName: fmt.Sprintf("%s-%d", namePrefix, i), - machineSetName: fmt.Sprintf("%s-%d", namePrefix, i), - namespace: namespace, - nodeCount: nodeCount, - rootIsMachineDeployment: isMachineDeployment, - }) + specs = append(specs, createTestSpec(namespace, clusterName, fmt.Sprintf("%s-%d", namePrefix, i), nodeCount, isMachineDeployment, annotations)) } return specs } +func createTestSpec(namespace, clusterName, name string, nodeCount int, isMachineDeployment bool, annotations map[string]string) testSpec { + return testSpec{ + annotations: annotations, + machineDeploymentName: name, + machineSetName: name, + clusterName: clusterName, + namespace: namespace, + nodeCount: nodeCount, + rootIsMachineDeployment: isMachineDeployment, + } +} + func createTestConfigs(specs ...testSpec) []*testConfig { result := make([]*testConfig, 0, len(specs)) for i, spec := range specs { config := &testConfig{ - spec: &specs[i], - nodes: make([]*corev1.Node, spec.nodeCount), - machines: make([]*unstructured.Unstructured, spec.nodeCount), + spec: &specs[i], + namespace: spec.namespace, + clusterName: spec.clusterName, + nodes: make([]*corev1.Node, spec.nodeCount), + machines: make([]*unstructured.Unstructured, spec.nodeCount), } machineSetLabels := map[string]string{ + "clusterName": spec.clusterName, "machineSetName": spec.machineSetName, } @@ -286,7 +299,8 @@ func createTestConfigs(specs ...testSpec) []*testConfig { "uid": spec.machineSetName, }, "spec": map[string]interface{}{ - "replicas": int64(spec.nodeCount), + "clusterName": spec.clusterName, + "replicas": int64(spec.nodeCount), }, "status": map[string]interface{}{}, }, @@ -300,6 +314,7 @@ func createTestConfigs(specs ...testSpec) []*testConfig { machineSetLabels["machineDeploymentName"] = spec.machineDeploymentName machineDeploymentLabels := map[string]string{ + "clusterName": spec.clusterName, "machineDeploymentName": spec.machineDeploymentName, } @@ -313,7 +328,8 @@ func createTestConfigs(specs ...testSpec) []*testConfig { "uid": spec.machineDeploymentName, }, "spec": map[string]interface{}{ - "replicas": int64(spec.nodeCount), + "clusterName": spec.clusterName, + "replicas": int64(spec.nodeCount), }, "status": map[string]interface{}{}, }, @@ -341,7 +357,7 @@ func createTestConfigs(specs ...testSpec) []*testConfig { } for j := 0; j < spec.nodeCount; j++ { - config.nodes[j], config.machines[j] = makeLinkedNodeAndMachine(j, spec.namespace, machineOwner, machineSetLabels) + config.nodes[j], config.machines[j] = makeLinkedNodeAndMachine(j, spec.namespace, spec.clusterName, machineOwner, machineSetLabels) } result = append(result, config) @@ -353,7 +369,7 @@ func createTestConfigs(specs ...testSpec) []*testConfig { // makeLinkedNodeAndMachine creates a node and machine. The machine // has its NodeRef set to the new node and the new machine's owner // reference is set to owner. -func makeLinkedNodeAndMachine(i int, namespace string, owner metav1.OwnerReference, machineLabels map[string]string) (*corev1.Node, *unstructured.Unstructured) { +func makeLinkedNodeAndMachine(i int, namespace, clusterName string, owner metav1.OwnerReference, machineLabels map[string]string) (*corev1.Node, *unstructured.Unstructured) { node := &corev1.Node{ TypeMeta: metav1.TypeMeta{ Kind: "Node", @@ -378,7 +394,8 @@ func makeLinkedNodeAndMachine(i int, namespace string, owner metav1.OwnerReferen "namespace": namespace, }, "spec": map[string]interface{}{ - "providerID": fmt.Sprintf("test:////%s-%s-nodeid-%d", namespace, owner.Name, i), + "clusterName": clusterName, + "providerID": fmt.Sprintf("test:////%s-%s-nodeid-%d", namespace, owner.Name, i), }, "status": map[string]interface{}{ "nodeRef": map[string]interface{}{ @@ -422,24 +439,6 @@ func addTestConfigs(t *testing.T, controller *machineController, testConfigs ... return nil } -func selectorFromScalableResource(u *unstructured.Unstructured) (labels.Selector, error) { - unstructuredSelector, found, err := unstructured.NestedMap(u.Object, "spec", "selector") - if err != nil { - return nil, err - } - - if !found { - return nil, fmt.Errorf("expected field spec.selector on scalable resource type") - } - - labelSelector := &metav1.LabelSelector{} - if err := runtime.DefaultUnstructuredConverter.FromUnstructured(unstructuredSelector, labelSelector); err != nil { - return nil, err - } - - return metav1.LabelSelectorAsSelector(labelSelector) -} - func createResource(client dynamic.Interface, informer informers.GenericInformer, gvr schema.GroupVersionResource, resource *unstructured.Unstructured) error { if _, err := client.Resource(gvr).Namespace(resource.GetNamespace()).Create(context.TODO(), resource, metav1.CreateOptions{}); err != nil { return err @@ -560,7 +559,7 @@ func TestControllerFindMachine(t *testing.T) { for _, tc := range testCases { t.Run(tc.description, func(t *testing.T) { - testConfig := createMachineSetTestConfig(RandomString(6), RandomString(6), 1, map[string]string{ + testConfig := createMachineSetTestConfig(RandomString(6), RandomString(6), RandomString(6), 1, map[string]string{ nodeGroupMinSizeAnnotationKey: "1", nodeGroupMaxSizeAnnotationKey: "10", }) @@ -577,7 +576,7 @@ func TestControllerFindMachine(t *testing.T) { } func TestControllerFindMachineOwner(t *testing.T) { - testConfig := createMachineSetTestConfig(RandomString(6), RandomString(6), 1, map[string]string{ + testConfig := createMachineSetTestConfig(RandomString(6), RandomString(6), RandomString(6), 1, map[string]string{ nodeGroupMinSizeAnnotationKey: "1", nodeGroupMaxSizeAnnotationKey: "10", }) @@ -626,7 +625,7 @@ func TestControllerFindMachineOwner(t *testing.T) { } func TestControllerFindMachineByProviderID(t *testing.T) { - testConfig := createMachineSetTestConfig(RandomString(6), RandomString(6), 1, map[string]string{ + testConfig := createMachineSetTestConfig(RandomString(6), RandomString(6), RandomString(6), 1, map[string]string{ nodeGroupMinSizeAnnotationKey: "1", nodeGroupMaxSizeAnnotationKey: "10", }) @@ -688,7 +687,7 @@ func TestControllerFindMachineByProviderID(t *testing.T) { } func TestControllerFindNodeByNodeName(t *testing.T) { - testConfig := createMachineSetTestConfig(RandomString(6), RandomString(6), 1, map[string]string{ + testConfig := createMachineSetTestConfig(RandomString(6), RandomString(6), RandomString(6), 1, map[string]string{ nodeGroupMinSizeAnnotationKey: "1", nodeGroupMaxSizeAnnotationKey: "10", }) @@ -787,7 +786,8 @@ func TestControllerListMachinesForScalableResource(t *testing.T) { t.Run("MachineSet", func(t *testing.T) { namespace := RandomString(6) - testConfig1 := createMachineSetTestConfig(namespace, RandomString(6), 5, map[string]string{ + clusterName := RandomString(6) + testConfig1 := createMachineSetTestConfig(namespace, clusterName, RandomString(6), 5, map[string]string{ nodeGroupMinSizeAnnotationKey: "1", nodeGroupMaxSizeAnnotationKey: "10", }) @@ -796,7 +796,7 @@ func TestControllerListMachinesForScalableResource(t *testing.T) { // nodes and the additional machineset to the existing set of // test objects in the controller. This gives us two // machinesets, each with their own machines and linked nodes. - testConfig2 := createMachineSetTestConfig(namespace, RandomString(6), 5, map[string]string{ + testConfig2 := createMachineSetTestConfig(namespace, clusterName, RandomString(6), 5, map[string]string{ nodeGroupMinSizeAnnotationKey: "1", nodeGroupMaxSizeAnnotationKey: "10", }) @@ -806,7 +806,8 @@ func TestControllerListMachinesForScalableResource(t *testing.T) { t.Run("MachineDeployment", func(t *testing.T) { namespace := RandomString(6) - testConfig1 := createMachineDeploymentTestConfig(namespace, RandomString(6), 5, map[string]string{ + clusterName := RandomString(6) + testConfig1 := createMachineDeploymentTestConfig(namespace, clusterName, RandomString(6), 5, map[string]string{ nodeGroupMinSizeAnnotationKey: "1", nodeGroupMaxSizeAnnotationKey: "10", }) @@ -815,7 +816,7 @@ func TestControllerListMachinesForScalableResource(t *testing.T) { // nodes, machineset, and the additional machineset to the existing set of // test objects in the controller. This gives us two // machinedeployments, each with their own machineSet, machines and linked nodes. - testConfig2 := createMachineDeploymentTestConfig(namespace, RandomString(6), 5, map[string]string{ + testConfig2 := createMachineDeploymentTestConfig(namespace, clusterName, RandomString(6), 5, map[string]string{ nodeGroupMinSizeAnnotationKey: "1", nodeGroupMaxSizeAnnotationKey: "10", }) @@ -846,7 +847,7 @@ func TestControllerLookupNodeGroupForNonExistentNode(t *testing.T) { } t.Run("MachineSet", func(t *testing.T) { - testConfig := createMachineSetTestConfig(RandomString(6), RandomString(6), 1, map[string]string{ + testConfig := createMachineSetTestConfig(RandomString(6), RandomString(6), RandomString(6), 1, map[string]string{ nodeGroupMinSizeAnnotationKey: "1", nodeGroupMaxSizeAnnotationKey: "10", }) @@ -854,7 +855,7 @@ func TestControllerLookupNodeGroupForNonExistentNode(t *testing.T) { }) t.Run("MachineDeployment", func(t *testing.T) { - testConfig := createMachineDeploymentTestConfig(RandomString(6), RandomString(6), 1, map[string]string{ + testConfig := createMachineDeploymentTestConfig(RandomString(6), RandomString(6), RandomString(6), 1, map[string]string{ nodeGroupMinSizeAnnotationKey: "1", nodeGroupMaxSizeAnnotationKey: "10", }) @@ -885,7 +886,7 @@ func TestControllerNodeGroupForNodeWithMissingMachineOwner(t *testing.T) { } t.Run("MachineSet", func(t *testing.T) { - testConfig := createMachineSetTestConfig(RandomString(6), RandomString(6), 1, map[string]string{ + testConfig := createMachineSetTestConfig(RandomString(6), RandomString(6), RandomString(6), 1, map[string]string{ nodeGroupMinSizeAnnotationKey: "1", nodeGroupMaxSizeAnnotationKey: "10", }) @@ -893,7 +894,7 @@ func TestControllerNodeGroupForNodeWithMissingMachineOwner(t *testing.T) { }) t.Run("MachineDeployment", func(t *testing.T) { - testConfig := createMachineDeploymentTestConfig(RandomString(6), RandomString(6), 1, map[string]string{ + testConfig := createMachineDeploymentTestConfig(RandomString(6), RandomString(6), RandomString(6), 1, map[string]string{ nodeGroupMinSizeAnnotationKey: "1", nodeGroupMaxSizeAnnotationKey: "10", }) @@ -902,7 +903,7 @@ func TestControllerNodeGroupForNodeWithMissingMachineOwner(t *testing.T) { } func TestControllerNodeGroupForNodeWithMissingSetMachineOwner(t *testing.T) { - testConfig := createMachineDeploymentTestConfig(RandomString(6), RandomString(6), 1, map[string]string{ + testConfig := createMachineDeploymentTestConfig(RandomString(6), RandomString(6), RandomString(6), 1, map[string]string{ nodeGroupMinSizeAnnotationKey: "1", nodeGroupMaxSizeAnnotationKey: "10", }) @@ -944,7 +945,7 @@ func TestControllerNodeGroupForNodeWithPositiveScalingBounds(t *testing.T) { } t.Run("MachineSet", func(t *testing.T) { - testConfig := createMachineSetTestConfig(RandomString(6), RandomString(6), 1, map[string]string{ + testConfig := createMachineSetTestConfig(RandomString(6), RandomString(6), RandomString(6), 1, map[string]string{ nodeGroupMinSizeAnnotationKey: "1", nodeGroupMaxSizeAnnotationKey: "1", }) @@ -952,7 +953,7 @@ func TestControllerNodeGroupForNodeWithPositiveScalingBounds(t *testing.T) { }) t.Run("MachineDeployment", func(t *testing.T) { - testConfig := createMachineDeploymentTestConfig(RandomString(6), RandomString(6), 1, map[string]string{ + testConfig := createMachineDeploymentTestConfig(RandomString(6), RandomString(6), RandomString(6), 1, map[string]string{ nodeGroupMinSizeAnnotationKey: "1", nodeGroupMaxSizeAnnotationKey: "1", }) @@ -981,19 +982,20 @@ func TestControllerNodeGroups(t *testing.T) { defer stop() namespace := RandomString(6) + clusterName := RandomString(6) // Test #1: zero nodegroups assertNodegroupLen(t, controller, 0) // Test #2: add 5 machineset-based nodegroups - machineSetConfigs := createMachineSetTestConfigs(namespace, RandomString(6), 5, 1, annotations) + machineSetConfigs := createMachineSetTestConfigs(namespace, clusterName, RandomString(6), 5, 1, annotations) if err := addTestConfigs(t, controller, machineSetConfigs...); err != nil { t.Fatalf("unexpected error: %v", err) } assertNodegroupLen(t, controller, 5) // Test #2: add 2 machinedeployment-based nodegroups - machineDeploymentConfigs := createMachineDeploymentTestConfigs(namespace, RandomString(6), 2, 1, annotations) + machineDeploymentConfigs := createMachineDeploymentTestConfigs(namespace, clusterName, RandomString(6), 2, 1, annotations) if err := addTestConfigs(t, controller, machineDeploymentConfigs...); err != nil { t.Fatalf("unexpected error: %v", err) } @@ -1017,14 +1019,14 @@ func TestControllerNodeGroups(t *testing.T) { } // Test #5: machineset with no scaling bounds results in no nodegroups - machineSetConfigs = createMachineSetTestConfigs(namespace, RandomString(6), 5, 1, annotations) + machineSetConfigs = createMachineSetTestConfigs(namespace, clusterName, RandomString(6), 5, 1, annotations) if err := addTestConfigs(t, controller, machineSetConfigs...); err != nil { t.Fatalf("unexpected error: %v", err) } assertNodegroupLen(t, controller, 0) // Test #6: machinedeployment with no scaling bounds results in no nodegroups - machineDeploymentConfigs = createMachineDeploymentTestConfigs(namespace, RandomString(6), 2, 1, annotations) + machineDeploymentConfigs = createMachineDeploymentTestConfigs(namespace, clusterName, RandomString(6), 2, 1, annotations) if err := addTestConfigs(t, controller, machineDeploymentConfigs...); err != nil { t.Fatalf("unexpected error: %v", err) } @@ -1036,7 +1038,7 @@ func TestControllerNodeGroups(t *testing.T) { } // Test #7: machineset with bad scaling bounds results in an error and no nodegroups - machineSetConfigs = createMachineSetTestConfigs(namespace, RandomString(6), 5, 1, annotations) + machineSetConfigs = createMachineSetTestConfigs(namespace, clusterName, RandomString(6), 5, 1, annotations) if err := addTestConfigs(t, controller, machineSetConfigs...); err != nil { t.Fatalf("unexpected error: %v", err) } @@ -1049,7 +1051,7 @@ func TestControllerNodeGroups(t *testing.T) { assertNodegroupLen(t, controller, 0) // Test #8: machinedeployment with bad scaling bounds results in an error and no nodegroups - machineDeploymentConfigs = createMachineDeploymentTestConfigs(namespace, RandomString(6), 2, 1, annotations) + machineDeploymentConfigs = createMachineDeploymentTestConfigs(namespace, clusterName, RandomString(6), 2, 1, annotations) if err := addTestConfigs(t, controller, machineDeploymentConfigs...); err != nil { t.Fatalf("unexpected error: %v", err) } @@ -1113,19 +1115,19 @@ func TestControllerNodeGroupsNodeCount(t *testing.T) { t.Run("MachineSet", func(t *testing.T) { for _, tc := range testCases { - test(t, tc, createMachineSetTestConfigs(RandomString(6), RandomString(6), tc.nodeGroups, tc.nodesPerGroup, annotations)) + test(t, tc, createMachineSetTestConfigs(RandomString(6), RandomString(6), RandomString(6), tc.nodeGroups, tc.nodesPerGroup, annotations)) } }) t.Run("MachineDeployment", func(t *testing.T) { for _, tc := range testCases { - test(t, tc, createMachineDeploymentTestConfigs(RandomString(6), RandomString(6), tc.nodeGroups, tc.nodesPerGroup, annotations)) + test(t, tc, createMachineDeploymentTestConfigs(RandomString(6), RandomString(6), RandomString(6), tc.nodeGroups, tc.nodesPerGroup, annotations)) } }) } func TestControllerFindMachineFromNodeAnnotation(t *testing.T) { - testConfig := createMachineSetTestConfig(RandomString(6), RandomString(6), 1, map[string]string{ + testConfig := createMachineSetTestConfig(RandomString(6), RandomString(6), RandomString(6), 1, map[string]string{ nodeGroupMinSizeAnnotationKey: "1", nodeGroupMaxSizeAnnotationKey: "10", }) @@ -1173,7 +1175,7 @@ func TestControllerFindMachineFromNodeAnnotation(t *testing.T) { } func TestControllerMachineSetNodeNamesWithoutLinkage(t *testing.T) { - testConfig := createMachineSetTestConfig(RandomString(6), RandomString(6), 3, map[string]string{ + testConfig := createMachineSetTestConfig(RandomString(6), RandomString(6), RandomString(6), 3, map[string]string{ nodeGroupMinSizeAnnotationKey: "1", nodeGroupMaxSizeAnnotationKey: "10", }) @@ -1215,7 +1217,7 @@ func TestControllerMachineSetNodeNamesWithoutLinkage(t *testing.T) { } func TestControllerMachineSetNodeNamesUsingProviderID(t *testing.T) { - testConfig := createMachineSetTestConfig(RandomString(6), RandomString(6), 3, map[string]string{ + testConfig := createMachineSetTestConfig(RandomString(6), RandomString(6), RandomString(6), 3, map[string]string{ nodeGroupMinSizeAnnotationKey: "1", nodeGroupMaxSizeAnnotationKey: "10", }) @@ -1267,7 +1269,7 @@ func TestControllerMachineSetNodeNamesUsingProviderID(t *testing.T) { } func TestControllerMachineSetNodeNamesUsingStatusNodeRefName(t *testing.T) { - testConfig := createMachineSetTestConfig(RandomString(6), RandomString(6), 3, map[string]string{ + testConfig := createMachineSetTestConfig(RandomString(6), RandomString(6), RandomString(6), 3, map[string]string{ nodeGroupMinSizeAnnotationKey: "1", nodeGroupMaxSizeAnnotationKey: "10", }) @@ -1338,7 +1340,7 @@ func TestControllerGetAPIVersionGroup(t *testing.T) { } func TestControllerGetAPIVersionGroupWithMachineDeployments(t *testing.T) { - testConfig := createMachineDeploymentTestConfig(RandomString(6), RandomString(6), 1, map[string]string{ + testConfig := createMachineDeploymentTestConfig(RandomString(6), RandomString(6), RandomString(6), 1, map[string]string{ nodeGroupMinSizeAnnotationKey: "1", nodeGroupMaxSizeAnnotationKey: "1", }) @@ -1573,3 +1575,430 @@ func RandomString(n int) string { } return string(result) } + +func Test_machineController_allowedByAutoDiscoverySpecs(t *testing.T) { + for _, tc := range []struct { + name string + testSpec testSpec + autoDiscoverySpecs []*clusterAPIAutoDiscoveryConfig + additionalLabels map[string]string + shouldMatch bool + }{{ + name: "autodiscovery specs includes permissive spec that should match any MachineSet", + testSpec: createTestSpec(RandomString(6), RandomString(6), RandomString(6), 1, false, nil), + autoDiscoverySpecs: []*clusterAPIAutoDiscoveryConfig{ + {labelSelector: labels.NewSelector()}, + {clusterName: "foo", namespace: "bar", labelSelector: labels.Nothing()}, + }, + shouldMatch: true, + }, { + name: "autodiscovery specs includes permissive spec that should match any MachineDeployment", + testSpec: createTestSpec(RandomString(6), RandomString(6), RandomString(6), 1, true, nil), + autoDiscoverySpecs: []*clusterAPIAutoDiscoveryConfig{ + {labelSelector: labels.NewSelector()}, + {clusterName: "foo", namespace: "bar", labelSelector: labels.Nothing()}, + }, + shouldMatch: true, + }, { + name: "autodiscovery specs includes a restrictive spec that should match specific MachineSet", + testSpec: createTestSpec("default", "foo", RandomString(6), 1, false, nil), + additionalLabels: map[string]string{"color": "green"}, + autoDiscoverySpecs: []*clusterAPIAutoDiscoveryConfig{ + {clusterName: "foo", namespace: "default", labelSelector: labels.SelectorFromSet(labels.Set{"color": "green"})}, + {clusterName: "wombat", namespace: "bar", labelSelector: labels.Nothing()}, + }, + shouldMatch: true, + }, { + name: "autodiscovery specs includes a restrictive spec that should match specific MachineDeployment", + testSpec: createTestSpec("default", "foo", RandomString(6), 1, true, nil), + additionalLabels: map[string]string{"color": "green"}, + autoDiscoverySpecs: []*clusterAPIAutoDiscoveryConfig{ + {clusterName: "foo", namespace: "default", labelSelector: labels.SelectorFromSet(labels.Set{"color": "green"})}, + {clusterName: "wombat", namespace: "bar", labelSelector: labels.Nothing()}, + }, + shouldMatch: true, + }, { + name: "autodiscovery specs does not include any specs that should match specific MachineSet", + testSpec: createTestSpec("default", "foo", RandomString(6), 1, false, nil), + additionalLabels: map[string]string{"color": "green"}, + autoDiscoverySpecs: []*clusterAPIAutoDiscoveryConfig{ + {clusterName: "test", namespace: "default", labelSelector: labels.SelectorFromSet(labels.Set{"color": "blue"})}, + {clusterName: "wombat", namespace: "bar", labelSelector: labels.Nothing()}, + }, + shouldMatch: false, + }, { + name: "autodiscovery specs does not include any specs that should match specific MachineDeployment", + testSpec: createTestSpec("default", "foo", RandomString(6), 1, true, nil), + additionalLabels: map[string]string{"color": "green"}, + autoDiscoverySpecs: []*clusterAPIAutoDiscoveryConfig{ + {clusterName: "test", namespace: "default", labelSelector: labels.SelectorFromSet(labels.Set{"color": "blue"})}, + {clusterName: "wombat", namespace: "bar", labelSelector: labels.Nothing()}, + }, + shouldMatch: false, + }} { + t.Run(tc.name, func(t *testing.T) { + testConfigs := createTestConfigs(tc.testSpec) + resource := testConfigs[0].machineSet + if tc.testSpec.rootIsMachineDeployment { + resource = testConfigs[0].machineDeployment + } + if tc.additionalLabels != nil { + resource.SetLabels(labels.Merge(resource.GetLabels(), tc.additionalLabels)) + } + c := &machineController{ + autoDiscoverySpecs: tc.autoDiscoverySpecs, + } + + got := c.allowedByAutoDiscoverySpecs(resource) + if got != tc.shouldMatch { + t.Errorf("allowedByAutoDiscoverySpecs got = %v, want %v", got, tc.shouldMatch) + } + }) + } +} + +func Test_machineController_listScalableResources(t *testing.T) { + uniqueMDConfig := createMachineDeploymentTestConfig(RandomString(6), RandomString(6), RandomString(6), 1, nil) + + mdTestConfigs := createMachineDeploymentTestConfigs(RandomString(6), RandomString(6), RandomString(6), 5, 1, nil) + mdTestConfigs = append(mdTestConfigs, uniqueMDConfig) + + allMachineDeployments := make([]*unstructured.Unstructured, 0, len(mdTestConfigs)) + for i := range mdTestConfigs { + allMachineDeployments = append(allMachineDeployments, mdTestConfigs[i].machineDeployment) + } + + uniqueMSConfig := createMachineSetTestConfig(RandomString(6), RandomString(6), RandomString(6), 1, nil) + + msTestConfigs := createMachineSetTestConfigs(RandomString(6), RandomString(6), RandomString(6), 5, 1, nil) + msTestConfigs = append(msTestConfigs, uniqueMSConfig) + + allMachineSets := make([]*unstructured.Unstructured, 0, len(msTestConfigs)) + for i := range msTestConfigs { + allMachineSets = append(allMachineSets, msTestConfigs[i].machineSet) + } + + allTestConfigs := append(mdTestConfigs, msTestConfigs...) + allScalableResources := append(allMachineDeployments, allMachineSets...) + + for _, tc := range []struct { + name string + autoDiscoverySpecs []*clusterAPIAutoDiscoveryConfig + want []*unstructured.Unstructured + wantErr bool + }{{ + name: "undefined autodiscovery results in returning all scalable resources", + autoDiscoverySpecs: nil, + want: allScalableResources, + wantErr: false, + }, { + name: "autodiscovery configuration to match against unique MachineSet only returns that MachineSet", + autoDiscoverySpecs: []*clusterAPIAutoDiscoveryConfig{ + {namespace: uniqueMSConfig.namespace, clusterName: uniqueMSConfig.clusterName, labelSelector: labels.SelectorFromSet(uniqueMSConfig.machineSet.GetLabels())}, + }, + want: []*unstructured.Unstructured{uniqueMSConfig.machineSet}, + wantErr: false, + }, { + name: "autodiscovery configuration to match against unique MachineDeployment only returns that MachineDeployment", + autoDiscoverySpecs: []*clusterAPIAutoDiscoveryConfig{ + {namespace: uniqueMDConfig.namespace, clusterName: uniqueMDConfig.clusterName, labelSelector: labels.SelectorFromSet(uniqueMDConfig.machineDeployment.GetLabels())}, + }, + want: []*unstructured.Unstructured{uniqueMDConfig.machineDeployment}, + wantErr: false, + }, { + name: "autodiscovery configuration to match against both unique MachineDeployment and unique MachineSet only returns those scalable resources", + autoDiscoverySpecs: []*clusterAPIAutoDiscoveryConfig{ + {namespace: uniqueMDConfig.namespace, clusterName: uniqueMDConfig.clusterName, labelSelector: labels.SelectorFromSet(uniqueMDConfig.machineDeployment.GetLabels())}, + {namespace: uniqueMSConfig.namespace, clusterName: uniqueMSConfig.clusterName, labelSelector: labels.SelectorFromSet(uniqueMSConfig.machineSet.GetLabels())}, + }, + want: []*unstructured.Unstructured{uniqueMDConfig.machineDeployment, uniqueMSConfig.machineSet}, + wantErr: false, + }, { + name: "autodiscovery configuration to match against both unique MachineDeployment, unique MachineSet, and a permissive config returns all scalable resources", + autoDiscoverySpecs: []*clusterAPIAutoDiscoveryConfig{ + {namespace: uniqueMDConfig.namespace, clusterName: uniqueMDConfig.clusterName, labelSelector: labels.SelectorFromSet(uniqueMDConfig.machineDeployment.GetLabels())}, + {namespace: uniqueMSConfig.namespace, clusterName: uniqueMSConfig.clusterName, labelSelector: labels.SelectorFromSet(uniqueMSConfig.machineSet.GetLabels())}, + {labelSelector: labels.NewSelector()}, + }, + want: allScalableResources, + wantErr: false, + }, { + name: "autodiscovery configuration to match against both unique MachineDeployment, unique MachineSet, and a restrictive returns unique scalable resources", + autoDiscoverySpecs: []*clusterAPIAutoDiscoveryConfig{ + {namespace: uniqueMDConfig.namespace, clusterName: uniqueMDConfig.clusterName, labelSelector: labels.SelectorFromSet(uniqueMDConfig.machineDeployment.GetLabels())}, + {namespace: uniqueMSConfig.namespace, clusterName: uniqueMSConfig.clusterName, labelSelector: labels.SelectorFromSet(uniqueMSConfig.machineSet.GetLabels())}, + {namespace: RandomString(6), clusterName: RandomString(6), labelSelector: labels.Nothing()}, + }, + want: []*unstructured.Unstructured{uniqueMDConfig.machineDeployment, uniqueMSConfig.machineSet}, + wantErr: false, + }, { + name: "autodiscovery configuration to match against a restrictive config returns no scalable resources", + autoDiscoverySpecs: []*clusterAPIAutoDiscoveryConfig{ + {namespace: RandomString(6), clusterName: RandomString(6), labelSelector: labels.Nothing()}, + }, + want: nil, + wantErr: false, + }} { + t.Run(tc.name, func(t *testing.T) { + c, stop := mustCreateTestController(t, allTestConfigs...) + defer stop() + c.autoDiscoverySpecs = tc.autoDiscoverySpecs + + got, err := c.listScalableResources() + if (err != nil) != tc.wantErr { + t.Errorf("listScalableRsources() error = %v, wantErr %v", err, tc.wantErr) + return + } + + if len(got) != len(tc.want) { + t.Errorf("listScalableRsources() expected length of got to be = %v, got %v", len(tc.want), len(got)) + } + + // Sort results as order is not guaranteed. + sort.Slice(got, func(i, j int) bool { + return got[i].GetName() < got[j].GetName() + }) + sort.Slice(tc.want, func(i, j int) bool { + return tc.want[i].GetName() < tc.want[j].GetName() + }) + + if err == nil && !reflect.DeepEqual(got, tc.want) { + t.Errorf("listScalableRsources() got = %v, want %v", got, tc.want) + } + }) + } +} + +func Test_machineController_nodeGroupForNode(t *testing.T) { + uniqueMDConfig := createMachineDeploymentTestConfig(RandomString(6), RandomString(6), RandomString(6), 1, map[string]string{ + nodeGroupMinSizeAnnotationKey: "1", + nodeGroupMaxSizeAnnotationKey: "10", + }) + + mdTestConfigs := createMachineDeploymentTestConfigs(RandomString(6), RandomString(6), RandomString(6), 5, 1, map[string]string{ + nodeGroupMinSizeAnnotationKey: "1", + nodeGroupMaxSizeAnnotationKey: "10", + }) + mdTestConfigs = append(mdTestConfigs, uniqueMDConfig) + + allMachineDeployments := make([]*unstructured.Unstructured, 0, len(mdTestConfigs)) + for i := range mdTestConfigs { + allMachineDeployments = append(allMachineDeployments, mdTestConfigs[i].machineDeployment) + } + + uniqueMSConfig := createMachineSetTestConfig(RandomString(6), RandomString(6), RandomString(6), 1, map[string]string{ + nodeGroupMinSizeAnnotationKey: "1", + nodeGroupMaxSizeAnnotationKey: "10", + }) + + msTestConfigs := createMachineSetTestConfigs(RandomString(6), RandomString(6), RandomString(6), 5, 1, map[string]string{ + nodeGroupMinSizeAnnotationKey: "1", + nodeGroupMaxSizeAnnotationKey: "10", + }) + msTestConfigs = append(msTestConfigs, uniqueMSConfig) + + allMachineSets := make([]*unstructured.Unstructured, 0, len(msTestConfigs)) + for i := range msTestConfigs { + allMachineSets = append(allMachineSets, msTestConfigs[i].machineSet) + } + + allTestConfigs := append(mdTestConfigs, msTestConfigs...) + + for _, tc := range []struct { + name string + autoDiscoverySpecs []*clusterAPIAutoDiscoveryConfig + node *corev1.Node + scalableResource *unstructured.Unstructured + wantErr bool + }{{ + name: "undefined autodiscovery results in returning MachineSet resource for given node", + autoDiscoverySpecs: nil, + node: msTestConfigs[0].nodes[0], + scalableResource: msTestConfigs[0].machineSet, + wantErr: false, + }, { + name: "undefined autodiscovery results in returning MachineDeployment resource for given node", + autoDiscoverySpecs: nil, + node: mdTestConfigs[0].nodes[0], + scalableResource: mdTestConfigs[0].machineDeployment, + wantErr: false, + }, { + name: "autodiscovery configuration to match against a restrictive config does not return a nodegroup", + autoDiscoverySpecs: []*clusterAPIAutoDiscoveryConfig{ + {namespace: RandomString(6), clusterName: RandomString(6), labelSelector: labels.Nothing()}, + }, + node: msTestConfigs[0].nodes[0], + scalableResource: nil, + wantErr: false, + }, { + name: "autodiscovery configuration to match against unique MachineSet returns nodegroup for that MachineSet", + autoDiscoverySpecs: []*clusterAPIAutoDiscoveryConfig{ + {namespace: uniqueMSConfig.namespace, clusterName: uniqueMSConfig.clusterName, labelSelector: labels.SelectorFromSet(uniqueMSConfig.machineSet.GetLabels())}, + }, + node: uniqueMSConfig.nodes[0], + scalableResource: uniqueMSConfig.machineSet, + wantErr: false, + }, { + name: "autodiscovery configuration to match against unique MachineDeployment returns nodegroup for that MachineDeployment", + autoDiscoverySpecs: []*clusterAPIAutoDiscoveryConfig{ + {namespace: uniqueMDConfig.namespace, clusterName: uniqueMDConfig.clusterName, labelSelector: labels.SelectorFromSet(uniqueMDConfig.machineDeployment.GetLabels())}, + }, + node: uniqueMDConfig.nodes[0], + scalableResource: uniqueMDConfig.machineDeployment, + wantErr: false, + }} { + t.Run(tc.name, func(t *testing.T) { + c, stop := mustCreateTestController(t, allTestConfigs...) + defer stop() + c.autoDiscoverySpecs = tc.autoDiscoverySpecs + + got, err := c.nodeGroupForNode(tc.node) + if (err != nil) != tc.wantErr { + t.Errorf("nodeGroupForNode() error = %v, wantErr %v", err, tc.wantErr) + return + } + + if got == nil && tc.scalableResource != nil { + t.Error("expected a node group to be returned, got nil") + return + } + + if tc.scalableResource == nil && got != nil { + t.Errorf("expected nil node group, got: %v", got) + return + } + + if tc.scalableResource != nil && !reflect.DeepEqual(got.scalableResource.unstructured, tc.scalableResource) { + t.Errorf("nodeGroupForNode() got = %v, want node group for scalable resource %v", got, tc.scalableResource) + } + }) + } +} + +func Test_machineController_nodeGroups(t *testing.T) { + uniqueMDConfig := createMachineDeploymentTestConfig(RandomString(6), RandomString(6), RandomString(6), 1, map[string]string{ + nodeGroupMinSizeAnnotationKey: "1", + nodeGroupMaxSizeAnnotationKey: "10", + }) + + mdTestConfigs := createMachineDeploymentTestConfigs(RandomString(6), RandomString(6), RandomString(6), 5, 1, map[string]string{ + nodeGroupMinSizeAnnotationKey: "1", + nodeGroupMaxSizeAnnotationKey: "10", + }) + mdTestConfigs = append(mdTestConfigs, uniqueMDConfig) + + allMachineDeployments := make([]*unstructured.Unstructured, 0, len(mdTestConfigs)) + for i := range mdTestConfigs { + allMachineDeployments = append(allMachineDeployments, mdTestConfigs[i].machineDeployment) + } + + uniqueMSConfig := createMachineSetTestConfig(RandomString(6), RandomString(6), RandomString(6), 1, map[string]string{ + nodeGroupMinSizeAnnotationKey: "1", + nodeGroupMaxSizeAnnotationKey: "10", + }) + + msTestConfigs := createMachineSetTestConfigs(RandomString(6), RandomString(6), RandomString(6), 5, 1, map[string]string{ + nodeGroupMinSizeAnnotationKey: "1", + nodeGroupMaxSizeAnnotationKey: "10", + }) + msTestConfigs = append(msTestConfigs, uniqueMSConfig) + + allMachineSets := make([]*unstructured.Unstructured, 0, len(msTestConfigs)) + for i := range msTestConfigs { + allMachineSets = append(allMachineSets, msTestConfigs[i].machineSet) + } + + allTestConfigs := append(mdTestConfigs, msTestConfigs...) + allScalableResources := append(allMachineDeployments, allMachineSets...) + + for _, tc := range []struct { + name string + autoDiscoverySpecs []*clusterAPIAutoDiscoveryConfig + expectedScalableResources []*unstructured.Unstructured + wantErr bool + }{{ + name: "undefined autodiscovery results in returning nodegroups for all scalable resources", + autoDiscoverySpecs: nil, + expectedScalableResources: allScalableResources, + wantErr: false, + }, { + name: "autodiscovery configuration to match against unique MachineSet only returns nodegroup for that MachineSet", + autoDiscoverySpecs: []*clusterAPIAutoDiscoveryConfig{ + {namespace: uniqueMSConfig.namespace, clusterName: uniqueMSConfig.clusterName, labelSelector: labels.SelectorFromSet(uniqueMSConfig.machineSet.GetLabels())}, + }, + expectedScalableResources: []*unstructured.Unstructured{uniqueMSConfig.machineSet}, + wantErr: false, + }, { + name: "autodiscovery configuration to match against unique MachineDeployment only returns nodegroup for that MachineDeployment", + autoDiscoverySpecs: []*clusterAPIAutoDiscoveryConfig{ + {namespace: uniqueMDConfig.namespace, clusterName: uniqueMDConfig.clusterName, labelSelector: labels.SelectorFromSet(uniqueMDConfig.machineDeployment.GetLabels())}, + }, + expectedScalableResources: []*unstructured.Unstructured{uniqueMDConfig.machineDeployment}, + wantErr: false, + }, { + name: "autodiscovery configuration to match against both unique MachineDeployment and unique MachineSet only returns nodegroups for those scalable resources", + autoDiscoverySpecs: []*clusterAPIAutoDiscoveryConfig{ + {namespace: uniqueMDConfig.namespace, clusterName: uniqueMDConfig.clusterName, labelSelector: labels.SelectorFromSet(uniqueMDConfig.machineDeployment.GetLabels())}, + {namespace: uniqueMSConfig.namespace, clusterName: uniqueMSConfig.clusterName, labelSelector: labels.SelectorFromSet(uniqueMSConfig.machineSet.GetLabels())}, + }, + expectedScalableResources: []*unstructured.Unstructured{uniqueMDConfig.machineDeployment, uniqueMSConfig.machineSet}, + wantErr: false, + }, { + name: "autodiscovery configuration to match against both unique MachineDeployment, unique MachineSet, and a permissive config returns nodegroups for all scalable resources", + autoDiscoverySpecs: []*clusterAPIAutoDiscoveryConfig{ + {namespace: uniqueMDConfig.namespace, clusterName: uniqueMDConfig.clusterName, labelSelector: labels.SelectorFromSet(uniqueMDConfig.machineDeployment.GetLabels())}, + {namespace: uniqueMSConfig.namespace, clusterName: uniqueMSConfig.clusterName, labelSelector: labels.SelectorFromSet(uniqueMSConfig.machineSet.GetLabels())}, + {labelSelector: labels.NewSelector()}, + }, + expectedScalableResources: allScalableResources, + wantErr: false, + }, { + name: "autodiscovery configuration to match against both unique MachineDeployment, unique MachineSet, and a restrictive returns nodegroups for unique scalable resources", + autoDiscoverySpecs: []*clusterAPIAutoDiscoveryConfig{ + {namespace: uniqueMDConfig.namespace, clusterName: uniqueMDConfig.clusterName, labelSelector: labels.SelectorFromSet(uniqueMDConfig.machineDeployment.GetLabels())}, + {namespace: uniqueMSConfig.namespace, clusterName: uniqueMSConfig.clusterName, labelSelector: labels.SelectorFromSet(uniqueMSConfig.machineSet.GetLabels())}, + {namespace: RandomString(6), clusterName: RandomString(6), labelSelector: labels.Nothing()}, + }, + expectedScalableResources: []*unstructured.Unstructured{uniqueMDConfig.machineDeployment, uniqueMSConfig.machineSet}, + wantErr: false, + }, { + name: "autodiscovery configuration to match against a restrictive config returns no nodegroups", + autoDiscoverySpecs: []*clusterAPIAutoDiscoveryConfig{ + {namespace: RandomString(6), clusterName: RandomString(6), labelSelector: labels.Nothing()}, + }, + expectedScalableResources: nil, + wantErr: false, + }} { + t.Run(tc.name, func(t *testing.T) { + c, stop := mustCreateTestController(t, allTestConfigs...) + defer stop() + c.autoDiscoverySpecs = tc.autoDiscoverySpecs + + got, err := c.nodeGroups() + if (err != nil) != tc.wantErr { + t.Errorf("nodeGroups() error = %v, wantErr %v", err, tc.wantErr) + return + } + + if len(got) != len(tc.expectedScalableResources) { + t.Errorf("nodeGroups() expected length of got to be = %v, got %v", len(tc.expectedScalableResources), len(got)) + } + + // Sort results as order is not guaranteed. + sort.Slice(got, func(i, j int) bool { + return got[i].scalableResource.Name() < got[j].scalableResource.Name() + }) + sort.Slice(tc.expectedScalableResources, func(i, j int) bool { + return tc.expectedScalableResources[i].GetName() < tc.expectedScalableResources[j].GetName() + }) + + if err == nil { + for i := range got { + if !reflect.DeepEqual(got[i].scalableResource.unstructured, tc.expectedScalableResources[i]) { + t.Errorf("nodeGroups() got = %v, expected to consist of nodegroups for scalable resources: %v", got, tc.expectedScalableResources) + } + } + } + }) + } +} diff --git a/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_nodegroup.go b/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_nodegroup.go index b47e2eb5021c..ef1f2545b4ab 100644 --- a/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_nodegroup.go +++ b/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_nodegroup.go @@ -62,11 +62,7 @@ func (ng *nodegroup) MaxSize() int { // (new nodes finish startup and registration or removed nodes are // deleted completely). Implementation required. func (ng *nodegroup) TargetSize() (int, error) { - size, err := ng.scalableResource.Replicas() - if err != nil { - return 0, err - } - return int(size), nil + return ng.scalableResource.Replicas() } // IncreaseSize increases the size of the node group. To delete a node @@ -82,12 +78,8 @@ func (ng *nodegroup) IncreaseSize(delta int) error { if err != nil { return err } - intSize := int(size) - if intSize+delta > ng.MaxSize() { - return fmt.Errorf("size increase too large - desired:%d max:%d", intSize+delta, ng.MaxSize()) - } - return ng.scalableResource.SetSize(int32(intSize + delta)) + return ng.scalableResource.SetSize(size + delta) } // DeleteNodes deletes nodes from this node group. Error is returned @@ -127,7 +119,7 @@ func (ng *nodegroup) DeleteNodes(nodes []*corev1.Node) error { // Step 2: if deleting len(nodes) would make the replica count // < minSize, then the request to delete that many nodes is bogus // and we fail fast. - if replicas-int32(len(nodes)) < int32(ng.MinSize()) { + if replicas-len(nodes) < ng.MinSize() { return fmt.Errorf("unable to delete %d machines in %q, machine replicas are %q, minSize is %q ", len(nodes), ng.Id(), replicas, ng.MinSize()) } @@ -196,7 +188,7 @@ func (ng *nodegroup) DecreaseTargetSize(delta int) error { size, delta, len(nodes)) } - return ng.scalableResource.SetSize(int32(size + delta)) + return ng.scalableResource.SetSize(size + delta) } // Id returns an unique identifier of the node group. @@ -362,12 +354,33 @@ func (ng *nodegroup) Autoprovisioned() bool { return false } -func newNodegroupFromScalableResource(controller *machineController, unstructuredScalableResource *unstructured.Unstructured) (*nodegroup, error) { +func newNodeGroupFromScalableResource(controller *machineController, unstructuredScalableResource *unstructured.Unstructured) (*nodegroup, error) { + // Ensure that the resulting node group would be allowed based on the autodiscovery specs if defined + if !controller.allowedByAutoDiscoverySpecs(unstructuredScalableResource) { + return nil, nil + } + scalableResource, err := newUnstructuredScalableResource(controller, unstructuredScalableResource) if err != nil { return nil, err } + replicas, found, err := unstructured.NestedInt64(unstructuredScalableResource.UnstructuredContent(), "spec", "replicas") + if err != nil { + return nil, err + } + + // We don't scale from 0 so nodes must belong to a nodegroup + // that has a scale size of at least 1. + if found && replicas == 0 { + return nil, nil + } + + // Ensure the node group would have the capacity to scale + if scalableResource.MaxSize()-scalableResource.MinSize() < 1 { + return nil, nil + } + return &nodegroup{ machineController: controller, scalableResource: scalableResource, diff --git a/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_nodegroup_test.go b/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_nodegroup_test.go index 3ee8a98118b8..fcc2eee231a0 100644 --- a/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_nodegroup_test.go +++ b/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_nodegroup_test.go @@ -47,6 +47,7 @@ func TestNodeGroupNewNodeGroupConstructor(t *testing.T) { minSize int maxSize int nodeCount int + expectNil bool } var testCases = []testCase{{ @@ -83,15 +84,17 @@ func TestNodeGroupNewNodeGroupConstructor(t *testing.T) { maxSize: 0, replicas: 0, errors: false, + expectNil: true, }, { description: "no error: min=0, max=1", annotations: map[string]string{ nodeGroupMaxSizeAnnotationKey: "1", }, - minSize: 0, - maxSize: 1, - replicas: 0, - errors: false, + minSize: 0, + maxSize: 1, + replicas: 0, + errors: false, + expectNil: true, }, { description: "no error: min=1, max=10, replicas=5", annotations: map[string]string{ @@ -103,13 +106,14 @@ func TestNodeGroupNewNodeGroupConstructor(t *testing.T) { replicas: 5, nodeCount: 5, errors: false, + expectNil: true, }} newNodeGroup := func(controller *machineController, testConfig *testConfig) (*nodegroup, error) { if testConfig.machineDeployment != nil { - return newNodegroupFromScalableResource(controller, testConfig.machineDeployment) + return newNodeGroupFromScalableResource(controller, testConfig.machineDeployment) } - return newNodegroupFromScalableResource(controller, testConfig.machineSet) + return newNodeGroupFromScalableResource(controller, testConfig.machineSet) } test := func(t *testing.T, tc testCase, testConfig *testConfig) { @@ -121,16 +125,18 @@ func TestNodeGroupNewNodeGroupConstructor(t *testing.T) { t.Fatal("expected an error") } - if !tc.errors && ng == nil { - t.Fatalf("test case logic error: %v", err) - } - if tc.errors { // if the test case is expected to error then // don't assert the remainder return } + if tc.expectNil && ng == nil { + // if the test case is expected to return nil then + // don't assert the remainder + return + } + if ng == nil { t.Fatal("expected nodegroup to be non-nil") } @@ -194,7 +200,7 @@ func TestNodeGroupNewNodeGroupConstructor(t *testing.T) { t.Run("MachineSet", func(t *testing.T) { for _, tc := range testCases { t.Run(tc.description, func(t *testing.T) { - test(t, tc, createMachineSetTestConfig(RandomString(6), RandomString(6), tc.nodeCount, tc.annotations)) + test(t, tc, createMachineSetTestConfig(RandomString(6), RandomString(6), RandomString(6), tc.nodeCount, tc.annotations)) }) } }) @@ -202,7 +208,7 @@ func TestNodeGroupNewNodeGroupConstructor(t *testing.T) { t.Run("MachineDeployment", func(t *testing.T) { for _, tc := range testCases { t.Run(tc.description, func(t *testing.T) { - test(t, tc, createMachineDeploymentTestConfig(RandomString(6), RandomString(6), tc.nodeCount, tc.annotations)) + test(t, tc, createMachineDeploymentTestConfig(RandomString(6), RandomString(6), RandomString(6), tc.nodeCount, tc.annotations)) }) } }) @@ -286,7 +292,7 @@ func TestNodeGroupIncreaseSizeErrors(t *testing.T) { nodeGroupMinSizeAnnotationKey: "1", nodeGroupMaxSizeAnnotationKey: "10", } - test(t, &tc, createMachineSetTestConfig(RandomString(6), RandomString(6), int(tc.initial), annotations)) + test(t, &tc, createMachineSetTestConfig(RandomString(6), RandomString(6), RandomString(6), int(tc.initial), annotations)) }) } }) @@ -298,7 +304,7 @@ func TestNodeGroupIncreaseSizeErrors(t *testing.T) { nodeGroupMinSizeAnnotationKey: "1", nodeGroupMaxSizeAnnotationKey: "10", } - test(t, &tc, createMachineDeploymentTestConfig(RandomString(6), RandomString(6), int(tc.initial), annotations)) + test(t, &tc, createMachineDeploymentTestConfig(RandomString(6), RandomString(6), RandomString(6), int(tc.initial), annotations)) }) } }) @@ -365,7 +371,7 @@ func TestNodeGroupIncreaseSize(t *testing.T) { expected: 4, delta: 1, } - test(t, &tc, createMachineSetTestConfig(RandomString(6), RandomString(6), int(tc.initial), annotations)) + test(t, &tc, createMachineSetTestConfig(RandomString(6), RandomString(6), RandomString(6), int(tc.initial), annotations)) }) t.Run("MachineDeployment", func(t *testing.T) { @@ -375,7 +381,7 @@ func TestNodeGroupIncreaseSize(t *testing.T) { expected: 4, delta: 1, } - test(t, &tc, createMachineDeploymentTestConfig(RandomString(6), RandomString(6), int(tc.initial), annotations)) + test(t, &tc, createMachineDeploymentTestConfig(RandomString(6), RandomString(6), RandomString(6), int(tc.initial), annotations)) }) } @@ -471,7 +477,7 @@ func TestNodeGroupDecreaseTargetSize(t *testing.T) { delta: -1, expectedError: true, } - test(t, &tc, createMachineSetTestConfig(RandomString(6), RandomString(6), int(tc.initial), annotations)) + test(t, &tc, createMachineSetTestConfig(RandomString(6), RandomString(6), RandomString(6), int(tc.initial), annotations)) }) t.Run("MachineSet", func(t *testing.T) { @@ -482,7 +488,7 @@ func TestNodeGroupDecreaseTargetSize(t *testing.T) { expected: 3, delta: -1, } - test(t, &tc, createMachineSetTestConfig(RandomString(6), RandomString(6), int(tc.initial), annotations)) + test(t, &tc, createMachineSetTestConfig(RandomString(6), RandomString(6), RandomString(6), int(tc.initial), annotations)) }) t.Run("MachineDeployment", func(t *testing.T) { @@ -494,7 +500,7 @@ func TestNodeGroupDecreaseTargetSize(t *testing.T) { delta: -1, expectedError: true, } - test(t, &tc, createMachineDeploymentTestConfig(RandomString(6), RandomString(6), int(tc.initial), annotations)) + test(t, &tc, createMachineDeploymentTestConfig(RandomString(6), RandomString(6), RandomString(6), int(tc.initial), annotations)) }) } @@ -576,7 +582,7 @@ func TestNodeGroupDecreaseSizeErrors(t *testing.T) { nodeGroupMinSizeAnnotationKey: "1", nodeGroupMaxSizeAnnotationKey: "10", } - test(t, &tc, createMachineSetTestConfig(RandomString(6), RandomString(6), int(tc.initial), annotations)) + test(t, &tc, createMachineSetTestConfig(RandomString(6), RandomString(6), RandomString(6), int(tc.initial), annotations)) }) } }) @@ -588,7 +594,7 @@ func TestNodeGroupDecreaseSizeErrors(t *testing.T) { nodeGroupMinSizeAnnotationKey: "1", nodeGroupMaxSizeAnnotationKey: "10", } - test(t, &tc, createMachineDeploymentTestConfig(RandomString(6), RandomString(6), int(tc.initial), annotations)) + test(t, &tc, createMachineDeploymentTestConfig(RandomString(6), RandomString(6), RandomString(6), int(tc.initial), annotations)) }) } }) @@ -666,14 +672,14 @@ func TestNodeGroupDeleteNodes(t *testing.T) { // sorting and the expected semantics in test() will fail. t.Run("MachineSet", func(t *testing.T) { - test(t, createMachineSetTestConfig(RandomString(6), RandomString(6), 10, map[string]string{ + test(t, createMachineSetTestConfig(RandomString(6), RandomString(6), RandomString(6), 10, map[string]string{ nodeGroupMinSizeAnnotationKey: "1", nodeGroupMaxSizeAnnotationKey: "10", })) }) t.Run("MachineDeployment", func(t *testing.T) { - test(t, createMachineDeploymentTestConfig(RandomString(6), RandomString(6), 10, map[string]string{ + test(t, createMachineDeploymentTestConfig(RandomString(6), RandomString(6), RandomString(6), 10, map[string]string{ nodeGroupMinSizeAnnotationKey: "1", nodeGroupMaxSizeAnnotationKey: "10", })) @@ -746,15 +752,17 @@ func TestNodeGroupMachineSetDeleteNodesWithMismatchedNodes(t *testing.T) { t.Run("MachineSet", func(t *testing.T) { namespace := RandomString(6) - testConfig0 := createMachineSetTestConfigs(namespace, RandomString(6), 1, 2, annotations) - testConfig1 := createMachineSetTestConfigs(namespace, RandomString(6), 1, 2, annotations) + clusterName := RandomString(6) + testConfig0 := createMachineSetTestConfigs(namespace, clusterName, RandomString(6), 1, 2, annotations) + testConfig1 := createMachineSetTestConfigs(namespace, clusterName, RandomString(6), 1, 2, annotations) test(t, 2, append(testConfig0, testConfig1...)) }) t.Run("MachineDeployment", func(t *testing.T) { namespace := RandomString(6) - testConfig0 := createMachineDeploymentTestConfigs(namespace, RandomString(6), 1, 2, annotations) - testConfig1 := createMachineDeploymentTestConfigs(namespace, RandomString(6), 1, 2, annotations) + clusterName := RandomString(6) + testConfig0 := createMachineDeploymentTestConfigs(namespace, clusterName, RandomString(6), 1, 2, annotations) + testConfig1 := createMachineDeploymentTestConfigs(namespace, clusterName, RandomString(6), 1, 2, annotations) test(t, 2, append(testConfig0, testConfig1...)) }) } @@ -924,14 +932,14 @@ func TestNodeGroupDeleteNodesTwice(t *testing.T) { // sorting and the expected semantics in test() will fail. t.Run("MachineSet", func(t *testing.T) { - test(t, createMachineSetTestConfig(RandomString(6), RandomString(6), 10, map[string]string{ + test(t, createMachineSetTestConfig(RandomString(6), RandomString(6), RandomString(6), 10, map[string]string{ nodeGroupMinSizeAnnotationKey: "1", nodeGroupMaxSizeAnnotationKey: "10", })) }) t.Run("MachineDeployment", func(t *testing.T) { - test(t, createMachineDeploymentTestConfig(RandomString(6), RandomString(6), 10, map[string]string{ + test(t, createMachineDeploymentTestConfig(RandomString(6), RandomString(6), RandomString(6), 10, map[string]string{ nodeGroupMinSizeAnnotationKey: "1", nodeGroupMaxSizeAnnotationKey: "10", })) @@ -1005,14 +1013,14 @@ func TestNodeGroupWithFailedMachine(t *testing.T) { // sorting and the expected semantics in test() will fail. t.Run("MachineSet", func(t *testing.T) { - test(t, createMachineSetTestConfig(RandomString(6), RandomString(6), 10, map[string]string{ + test(t, createMachineSetTestConfig(RandomString(6), RandomString(6), RandomString(6), 10, map[string]string{ nodeGroupMinSizeAnnotationKey: "1", nodeGroupMaxSizeAnnotationKey: "10", })) }) t.Run("MachineDeployment", func(t *testing.T) { - test(t, createMachineDeploymentTestConfig(RandomString(6), RandomString(6), 10, map[string]string{ + test(t, createMachineDeploymentTestConfig(RandomString(6), RandomString(6), RandomString(6), 10, map[string]string{ nodeGroupMinSizeAnnotationKey: "1", nodeGroupMaxSizeAnnotationKey: "10", })) diff --git a/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_provider.go b/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_provider.go index a0a017cad67a..fbac1fd89f32 100644 --- a/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_provider.go +++ b/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_provider.go @@ -171,7 +171,7 @@ func BuildClusterAPI(opts config.AutoscalingOptions, do cloudprovider.NodeGroupD klog.Fatalf("create scale client failed: %v", err) } - controller, err := newMachineController(managementClient, workloadClient, managementDiscoveryClient, managementScaleClient) + controller, err := newMachineController(managementClient, workloadClient, managementDiscoveryClient, managementScaleClient, do) if err != nil { klog.Fatal(err) } diff --git a/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_unstructured.go b/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_unstructured.go index 30a488baf0d3..166eb97fe979 100644 --- a/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_unstructured.go +++ b/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_unstructured.go @@ -81,7 +81,7 @@ func (r unstructuredScalableResource) ProviderIDs() ([]string, error) { return providerIds, nil } -func (r unstructuredScalableResource) Replicas() (int32, error) { +func (r unstructuredScalableResource) Replicas() (int, error) { gvr, err := r.GroupVersionResource() if err != nil { return 0, err @@ -94,10 +94,17 @@ func (r unstructuredScalableResource) Replicas() (int32, error) { if s == nil { return 0, fmt.Errorf("unknown %s %s/%s", r.Kind(), r.Namespace(), r.Name()) } - return s.Spec.Replicas, nil + return int(s.Spec.Replicas), nil } -func (r unstructuredScalableResource) SetSize(nreplicas int32) error { +func (r unstructuredScalableResource) SetSize(nreplicas int) error { + switch { + case nreplicas > r.maxSize: + return fmt.Errorf("size increase too large - desired:%d max:%d", nreplicas, r.maxSize) + case nreplicas < r.minSize: + return fmt.Errorf("size decrease too large - desired:%d min:%d", nreplicas, r.minSize) + } + gvr, err := r.GroupVersionResource() if err != nil { return err @@ -112,7 +119,7 @@ func (r unstructuredScalableResource) SetSize(nreplicas int32) error { return fmt.Errorf("unknown %s %s/%s", r.Kind(), r.Namespace(), r.Name()) } - s.Spec.Replicas = nreplicas + s.Spec.Replicas = int32(nreplicas) _, updateErr := r.controller.managementScaleClient.Scales(r.Namespace()).Update(context.TODO(), gvr.GroupResource(), s, metav1.UpdateOptions{}) return updateErr } diff --git a/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_unstructured_test.go b/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_unstructured_test.go index 56fb4d13ab95..a8eb88a655af 100644 --- a/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_unstructured_test.go +++ b/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_unstructured_test.go @@ -27,7 +27,7 @@ import ( func TestSetSize(t *testing.T) { initialReplicas := 1 - updatedReplicas := int32(5) + updatedReplicas := 5 test := func(t *testing.T, testConfig *testConfig) { controller, stop := mustCreateTestController(t, testConfig) @@ -56,23 +56,39 @@ func TestSetSize(t *testing.T) { s, err := sr.controller.managementScaleClient.Scales(testResource.GetNamespace()). Get(context.TODO(), gvr.GroupResource(), testResource.GetName(), metav1.GetOptions{}) - if s.Spec.Replicas != updatedReplicas { + if s.Spec.Replicas != int32(updatedReplicas) { t.Errorf("expected %v, got: %v", updatedReplicas, s.Spec.Replicas) } } t.Run("MachineSet", func(t *testing.T) { - test(t, createMachineSetTestConfig(RandomString(6), RandomString(6), initialReplicas, nil)) + test(t, createMachineSetTestConfig( + RandomString(6), + RandomString(6), + RandomString(6), + initialReplicas, map[string]string{ + nodeGroupMinSizeAnnotationKey: "1", + nodeGroupMaxSizeAnnotationKey: "10", + }, + )) }) t.Run("MachineDeployment", func(t *testing.T) { - test(t, createMachineDeploymentTestConfig(RandomString(6), RandomString(6), initialReplicas, nil)) + test(t, createMachineDeploymentTestConfig( + RandomString(6), + RandomString(6), + RandomString(6), + initialReplicas, map[string]string{ + nodeGroupMinSizeAnnotationKey: "1", + nodeGroupMaxSizeAnnotationKey: "10", + }, + )) }) } func TestReplicas(t *testing.T) { initialReplicas := 1 - updatedReplicas := int32(5) + updatedReplicas := 5 test := func(t *testing.T, testConfig *testConfig) { controller, stop := mustCreateTestController(t, testConfig) @@ -98,7 +114,7 @@ func TestReplicas(t *testing.T) { t.Fatal(err) } - if i != int32(initialReplicas) { + if i != initialReplicas { t.Errorf("expected %v, got: %v", initialReplicas, i) } @@ -109,7 +125,7 @@ func TestReplicas(t *testing.T) { t.Fatal(err) } - s.Spec.Replicas = updatedReplicas + s.Spec.Replicas = int32(updatedReplicas) _, err = sr.controller.managementScaleClient.Scales(testResource.GetNamespace()). Update(context.TODO(), gvr.GroupResource(), s, metav1.UpdateOptions{}) @@ -128,17 +144,17 @@ func TestReplicas(t *testing.T) { } t.Run("MachineSet", func(t *testing.T) { - test(t, createMachineSetTestConfig(RandomString(6), RandomString(6), initialReplicas, nil)) + test(t, createMachineSetTestConfig(RandomString(6), RandomString(6), RandomString(6), initialReplicas, nil)) }) t.Run("MachineDeployment", func(t *testing.T) { - test(t, createMachineDeploymentTestConfig(RandomString(6), RandomString(6), initialReplicas, nil)) + test(t, createMachineDeploymentTestConfig(RandomString(6), RandomString(6), RandomString(6), initialReplicas, nil)) }) } func TestSetSizeAndReplicas(t *testing.T) { initialReplicas := 1 - updatedReplicas := int32(5) + updatedReplicas := 5 test := func(t *testing.T, testConfig *testConfig) { controller, stop := mustCreateTestController(t, testConfig) @@ -159,7 +175,7 @@ func TestSetSizeAndReplicas(t *testing.T) { t.Fatal(err) } - if i != int32(initialReplicas) { + if i != initialReplicas { t.Errorf("expected %v, got: %v", initialReplicas, i) } @@ -179,11 +195,27 @@ func TestSetSizeAndReplicas(t *testing.T) { } t.Run("MachineSet", func(t *testing.T) { - test(t, createMachineSetTestConfig(RandomString(6), RandomString(6), initialReplicas, nil)) + test(t, createMachineSetTestConfig( + RandomString(6), + RandomString(6), + RandomString(6), + initialReplicas, map[string]string{ + nodeGroupMinSizeAnnotationKey: "1", + nodeGroupMaxSizeAnnotationKey: "10", + }, + )) }) t.Run("MachineDeployment", func(t *testing.T) { - test(t, createMachineDeploymentTestConfig(RandomString(6), RandomString(6), initialReplicas, nil)) + test(t, createMachineDeploymentTestConfig( + RandomString(6), + RandomString(6), + RandomString(6), + initialReplicas, map[string]string{ + nodeGroupMinSizeAnnotationKey: "1", + nodeGroupMaxSizeAnnotationKey: "10", + }, + )) }) } @@ -241,7 +273,7 @@ func TestAnnotations(t *testing.T) { } t.Run("MachineSet", func(t *testing.T) { - test(t, createMachineSetTestConfig(RandomString(6), RandomString(6), 1, annotations)) + test(t, createMachineSetTestConfig(RandomString(6), RandomString(6), RandomString(6), 1, annotations)) }) } @@ -282,7 +314,7 @@ func TestCanScaleFromZero(t *testing.T) { for _, tc := range testConfigs { t.Run(tc.name, func(t *testing.T) { - msTestConfig := createMachineSetTestConfig(RandomString(6), RandomString(6), 1, tc.annotations) + msTestConfig := createMachineSetTestConfig(RandomString(6), RandomString(6), RandomString(6), 1, tc.annotations) controller, stop := mustCreateTestController(t, msTestConfig) defer stop() diff --git a/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_utils.go b/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_utils.go index fca0354b2721..6a160401c4af 100644 --- a/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_utils.go +++ b/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_utils.go @@ -31,6 +31,8 @@ import ( const ( nodeGroupMinSizeAnnotationKey = "machine.openshift.io/cluster-api-autoscaler-node-group-min-size" nodeGroupMaxSizeAnnotationKey = "machine.openshift.io/cluster-api-autoscaler-node-group-max-size" + clusterNameLabel = "cluster.x-k8s.io/cluster-name" + deprecatedClusterNameLabel = "cluster.k8s.io/cluster-name" cpuKey = "machine.openshift.io/vCPU" memoryKey = "machine.openshift.io/memoryMb" @@ -192,3 +194,35 @@ func parseGPUCapacity(annotations map[string]string) (resource.Quantity, error) func parseMaxPodsCapacity(annotations map[string]string) (resource.Quantity, error) { return parseKey(annotations, maxPodsKey) } + +func clusterNameFromResource(r *unstructured.Unstructured) string { + // Use Spec.ClusterName if defined (only available on v1alpha3+ types) + clusterName, found, err := unstructured.NestedString(r.Object, "spec", "clusterName") + if err != nil { + return "" + } + + if found { + return clusterName + } + + // Fallback to value of clusterNameLabel + if clusterName, ok := r.GetLabels()[clusterNameLabel]; ok { + return clusterName + } + + // fallback for backward compatibility for deprecatedClusterNameLabel + if clusterName, ok := r.GetLabels()[deprecatedClusterNameLabel]; ok { + return clusterName + } + + // fallback for cluster-api v1alpha1 cluster linking + templateLabels, found, err := unstructured.NestedStringMap(r.UnstructuredContent(), "spec", "template", "metadata", "labels") + if found { + if clusterName, ok := templateLabels[deprecatedClusterNameLabel]; ok { + return clusterName + } + } + + return "" +} diff --git a/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_utils_test.go b/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_utils_test.go index 88fb4322342f..a1aa09660c77 100644 --- a/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_utils_test.go +++ b/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_utils_test.go @@ -642,3 +642,249 @@ func TestParseMaxPodsCapacity(t *testing.T) { }) } } + +func Test_clusterNameFromResource(t *testing.T) { + for _, tc := range []struct { + name string + resource *unstructured.Unstructured + want string + }{{ + name: "cluster name not set, v1alpha1 MachineSet", + resource: &unstructured.Unstructured{ + Object: map[string]interface{}{ + "kind": machineSetKind, + "apiVersion": "cluster.k8s.io/v1alpha1", + "metadata": map[string]interface{}{ + "name": "foo", + "namespace": "default", + }, + "spec": map[string]interface{}{ + "replicas": int64(1), + }, + "status": map[string]interface{}{}, + }, + }, + want: "", + }, { + name: "cluster name not set, v1alpha1 MachineDeployment", + resource: &unstructured.Unstructured{ + Object: map[string]interface{}{ + "kind": machineDeploymentKind, + "apiVersion": "cluster.k8s.io/v1alpha1", + "metadata": map[string]interface{}{ + "name": "foo", + "namespace": "default", + }, + "spec": map[string]interface{}{ + "replicas": int64(1), + }, + "status": map[string]interface{}{}, + }, + }, + want: "", + }, { + name: "cluster name set in MachineSet labels, v1alpha1 MachineSet", + resource: &unstructured.Unstructured{ + Object: map[string]interface{}{ + "kind": machineSetKind, + "apiVersion": "cluster.k8s.io/v1alpha1", + "metadata": map[string]interface{}{ + "name": "foo", + "namespace": "default", + "labels": map[string]interface{}{ + deprecatedClusterNameLabel: "bar", + }, + }, + "spec": map[string]interface{}{ + "replicas": int64(1), + }, + "status": map[string]interface{}{}, + }, + }, + want: "bar", + }, { + name: "cluster name set in MachineDeployment, v1alpha1 MachineDeployment", + resource: &unstructured.Unstructured{ + Object: map[string]interface{}{ + "kind": machineDeploymentKind, + "apiVersion": "cluster.k8s.io/v1alpha1", + "metadata": map[string]interface{}{ + "name": "foo", + "namespace": "default", + "labels": map[string]interface{}{ + deprecatedClusterNameLabel: "bar", + }, + }, + "spec": map[string]interface{}{ + "replicas": int64(1), + }, + "status": map[string]interface{}{}, + }, + }, + want: "bar", + }, { + name: "cluster name set in Machine template labels, v1alpha1 MachineSet", + resource: &unstructured.Unstructured{ + Object: map[string]interface{}{ + "kind": machineSetKind, + "apiVersion": "cluster.k8s.io/v1alpha1", + "metadata": map[string]interface{}{ + "name": "foo", + "namespace": "default", + }, + "spec": map[string]interface{}{ + "replicas": int64(1), + "template": map[string]interface{}{ + "metadata": map[string]interface{}{ + "labels": map[string]interface{}{ + deprecatedClusterNameLabel: "bar", + }, + }, + }, + }, + "status": map[string]interface{}{}, + }, + }, + want: "bar", + }, { + name: "cluster name set in Machine template, v1alpha1 MachineDeployment", + resource: &unstructured.Unstructured{ + Object: map[string]interface{}{ + "kind": machineDeploymentKind, + "apiVersion": "cluster.k8s.io/v1alpha1", + "metadata": map[string]interface{}{ + "name": "foo", + "namespace": "default", + }, + "spec": map[string]interface{}{ + "replicas": int64(1), + "template": map[string]interface{}{ + "metadata": map[string]interface{}{ + "labels": map[string]interface{}{ + deprecatedClusterNameLabel: "bar", + }, + }, + }, + }, + "status": map[string]interface{}{}, + }, + }, + want: "bar", + }, { + name: "cluster name not set, v1alpha2 MachineSet", + resource: &unstructured.Unstructured{ + Object: map[string]interface{}{ + "kind": machineSetKind, + "apiVersion": "cluster.x-k8s.io/v1alpha2", + "metadata": map[string]interface{}{ + "name": "foo", + "namespace": "default", + }, + "spec": map[string]interface{}{ + "replicas": int64(1), + }, + "status": map[string]interface{}{}, + }, + }, + want: "", + }, { + name: "cluster name not set, v1alpha2 MachineDeployment", + resource: &unstructured.Unstructured{ + Object: map[string]interface{}{ + "kind": machineDeploymentKind, + "apiVersion": "cluster.x-k8s.io/v1alpha2", + "metadata": map[string]interface{}{ + "name": "foo", + "namespace": "default", + }, + "spec": map[string]interface{}{ + "replicas": int64(1), + }, + "status": map[string]interface{}{}, + }, + }, + want: "", + }, { + name: "cluster name set in MachineSet labels, v1alpha2 MachineSet", + resource: &unstructured.Unstructured{ + Object: map[string]interface{}{ + "kind": machineSetKind, + "apiVersion": "cluster.x-k8s.io/v1alpha2", + "metadata": map[string]interface{}{ + "name": "foo", + "namespace": "default", + "labels": map[string]interface{}{ + clusterNameLabel: "bar", + }, + }, + "spec": map[string]interface{}{ + "replicas": int64(1), + }, + "status": map[string]interface{}{}, + }, + }, + want: "bar", + }, { + name: "cluster name set in MachineDeployment, v1alpha2 MachineDeployment", + resource: &unstructured.Unstructured{ + Object: map[string]interface{}{ + "kind": machineDeploymentKind, + "apiVersion": "cluster.x-k8s.io/v1alpha2", + "metadata": map[string]interface{}{ + "name": "foo", + "namespace": "default", + "labels": map[string]interface{}{ + clusterNameLabel: "bar", + }, + }, + "spec": map[string]interface{}{ + "replicas": int64(1), + }, + "status": map[string]interface{}{}, + }, + }, + want: "bar", + }, { + name: "cluster name set in spec, v1alpha3 MachineSet", + resource: &unstructured.Unstructured{ + Object: map[string]interface{}{ + "kind": machineSetKind, + "apiVersion": "cluster.x-k8s.io/v1alpha3", + "metadata": map[string]interface{}{ + "name": "foo", + "namespace": "default", + }, + "spec": map[string]interface{}{ + "clusterName": "bar", + "replicas": int64(1), + }, + "status": map[string]interface{}{}, + }, + }, + want: "bar", + }, { + name: "cluster name set in spec, v1alpha3 MachineDeployment", + resource: &unstructured.Unstructured{ + Object: map[string]interface{}{ + "kind": machineDeploymentKind, + "apiVersion": "cluster.x-k8s.io/v1alpha3", + "metadata": map[string]interface{}{ + "name": "foo", + "namespace": "default", + }, + "spec": map[string]interface{}{ + "clusterName": "bar", + "replicas": int64(1), + }, + "status": map[string]interface{}{}, + }, + }, + want: "bar", + }} { + t.Run(tc.name, func(t *testing.T) { + if got := clusterNameFromResource(tc.resource); got != tc.want { + t.Errorf("clusterNameFromResource() = %v, want %v", got, tc.want) + } + }) + } +}