From 9b96e0d2eac9b1dd3df55040f9cdf7d896ab816c Mon Sep 17 00:00:00 2001 From: Jason DeTiberus Date: Fri, 10 Jul 2020 12:09:25 -0400 Subject: [PATCH] Convert clusterapi provider to use unstructured --- .../clusterapi/clusterapi_controller.go | 514 ++++++------- .../clusterapi/clusterapi_controller_test.go | 728 ++++++++++++------ .../clusterapi/clusterapi_converters.go | 196 ----- .../clusterapi_machinedeployment.go | 152 ---- .../clusterapi/clusterapi_machineset.go | 138 ---- .../clusterapi/clusterapi_machineset_test.go | 150 ---- .../clusterapi/clusterapi_nodegroup.go | 46 +- .../clusterapi/clusterapi_nodegroup_test.go | 357 ++++----- .../clusterapi/clusterapi_provider.go | 39 +- .../clusterapi/clusterapi_provider_test.go | 7 +- .../clusterapi/clusterapi_scalableresource.go | 74 -- .../clusterapi/clusterapi_unstructured.go | 169 ++++ .../clusterapi_unstructured_test.go | 186 +++++ .../clusterapi/clusterapi_utils.go | 35 +- .../clusterapi/clusterapi_utils_test.go | 338 ++++---- .../cloudprovider/clusterapi/machine_types.go | 92 --- .../clusterapi/machinedeployment_types.go | 59 -- .../clusterapi/machineset_types.go | 81 -- .../clusterapi/zz_generated.deepcopy.go | 360 --------- 19 files changed, 1446 insertions(+), 2275 deletions(-) delete mode 100644 cluster-autoscaler/cloudprovider/clusterapi/clusterapi_converters.go delete mode 100644 cluster-autoscaler/cloudprovider/clusterapi/clusterapi_machinedeployment.go delete mode 100644 cluster-autoscaler/cloudprovider/clusterapi/clusterapi_machineset.go delete mode 100644 cluster-autoscaler/cloudprovider/clusterapi/clusterapi_machineset_test.go delete mode 100644 cluster-autoscaler/cloudprovider/clusterapi/clusterapi_scalableresource.go create mode 100644 cluster-autoscaler/cloudprovider/clusterapi/clusterapi_unstructured.go create mode 100644 cluster-autoscaler/cloudprovider/clusterapi/clusterapi_unstructured_test.go delete mode 100644 cluster-autoscaler/cloudprovider/clusterapi/machine_types.go delete mode 100644 cluster-autoscaler/cloudprovider/clusterapi/machinedeployment_types.go delete mode 100644 cluster-autoscaler/cloudprovider/clusterapi/machineset_types.go delete mode 100644 cluster-autoscaler/cloudprovider/clusterapi/zz_generated.deepcopy.go diff --git a/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_controller.go b/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_controller.go index 6430b485c731..834fa5da1779 100644 --- a/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_controller.go +++ b/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_controller.go @@ -17,7 +17,6 @@ limitations under the License. package clusterapi import ( - "context" "fmt" "os" "strings" @@ -27,6 +26,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/labels" + "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/client-go/discovery" "k8s.io/client-go/dynamic" @@ -34,9 +34,9 @@ import ( "k8s.io/client-go/informers" kubeinformers "k8s.io/client-go/informers" kubeclient "k8s.io/client-go/kubernetes" + "k8s.io/client-go/scale" "k8s.io/client-go/tools/cache" klog "k8s.io/klog/v2" - "k8s.io/utils/pointer" ) const ( @@ -49,6 +49,9 @@ const ( resourceNameMachineSet = "machinesets" resourceNameMachineDeployment = "machinedeployments" failedMachinePrefix = "failed-machine-" + machineDeploymentKind = "MachineDeployment" + machineSetKind = "MachineSet" + machineKind = "Machine" ) // machineController watches for Nodes, Machines, MachineSets and @@ -56,21 +59,21 @@ const ( // cluster. Additionally, it adds indices to the node informers to // satisfy lookup by node.Spec.ProviderID. type machineController struct { - kubeInformerFactory kubeinformers.SharedInformerFactory - machineInformerFactory dynamicinformer.DynamicSharedInformerFactory - machineDeploymentInformer informers.GenericInformer - machineInformer informers.GenericInformer - machineSetInformer informers.GenericInformer - nodeInformer cache.SharedIndexInformer - dynamicclient dynamic.Interface - machineSetResource *schema.GroupVersionResource - machineResource *schema.GroupVersionResource - machineDeploymentResource *schema.GroupVersionResource - accessLock sync.Mutex + workloadInformerFactory kubeinformers.SharedInformerFactory + managementInformerFactory dynamicinformer.DynamicSharedInformerFactory + machineDeploymentInformer informers.GenericInformer + machineInformer informers.GenericInformer + machineSetInformer informers.GenericInformer + nodeInformer cache.SharedIndexInformer + managementClient dynamic.Interface + managementScaleClient scale.ScalesGetter + machineSetResource schema.GroupVersionResource + machineResource schema.GroupVersionResource + machineDeploymentResource schema.GroupVersionResource + machineDeploymentsAvailable bool + accessLock sync.Mutex } -type machineSetFilterFunc func(machineSet *MachineSet) error - func indexMachineByProviderID(obj interface{}) ([]string, error) { u, ok := obj.(*unstructured.Unstructured) if !ok { @@ -98,31 +101,20 @@ func indexNodeByProviderID(obj interface{}) ([]string, error) { return []string{}, nil } -func (c *machineController) findMachine(id string) (*Machine, error) { - item, exists, err := c.machineInformer.Informer().GetStore().GetByKey(id) - if err != nil { - return nil, err - } - - if !exists { - return nil, nil - } - - u, ok := item.(*unstructured.Unstructured) - if !ok { - return nil, fmt.Errorf("internal error; unexpected type: %T", item) - } +func (c *machineController) findMachine(id string) (*unstructured.Unstructured, error) { + return findResourceByKey(c.machineInformer.Informer().GetStore(), id) +} - machine := newMachineFromUnstructured(u.DeepCopy()) - if machine == nil { - return nil, nil - } +func (c *machineController) findMachineSet(id string) (*unstructured.Unstructured, error) { + return findResourceByKey(c.machineSetInformer.Informer().GetStore(), id) +} - return machine, nil +func (c *machineController) findMachineDeployment(id string) (*unstructured.Unstructured, error) { + return findResourceByKey(c.machineDeploymentInformer.Informer().GetStore(), id) } -func (c *machineController) findMachineDeployment(id string) (*MachineDeployment, error) { - item, exists, err := c.machineDeploymentInformer.Informer().GetStore().GetByKey(id) +func findResourceByKey(store cache.Store, key string) (*unstructured.Unstructured, error) { + item, exists, err := store.GetByKey(key) if err != nil { return nil, err } @@ -136,62 +128,45 @@ func (c *machineController) findMachineDeployment(id string) (*MachineDeployment return nil, fmt.Errorf("internal error; unexpected type: %T", item) } - machineDeployment := newMachineDeploymentFromUnstructured(u.DeepCopy()) - if machineDeployment == nil { - return nil, nil - } - - return machineDeployment, nil + return u.DeepCopy(), nil } // findMachineOwner returns the machine set owner for machine, or nil // if there is no owner. A DeepCopy() of the object is returned on // success. -func (c *machineController) findMachineOwner(machine *Machine) (*MachineSet, error) { +func (c *machineController) findMachineOwner(machine *unstructured.Unstructured) (*unstructured.Unstructured, error) { machineOwnerRef := machineOwnerRef(machine) if machineOwnerRef == nil { return nil, nil } - store := c.machineSetInformer.Informer().GetStore() - item, exists, err := store.GetByKey(fmt.Sprintf("%s/%s", machine.Namespace, machineOwnerRef.Name)) - if err != nil { - return nil, err - } - if !exists { - return nil, nil - } - - u, ok := item.(*unstructured.Unstructured) - if !ok { - return nil, fmt.Errorf("internal error; unexpected type: %T", item) - } - - u = u.DeepCopy() - machineSet := newMachineSetFromUnstructured(u) - if machineSet == nil { - return nil, nil - } + return c.findMachineSet(fmt.Sprintf("%s/%s", machine.GetNamespace(), machineOwnerRef.Name)) +} - if !machineIsOwnedByMachineSet(machine, machineSet) { +// findMachineSetOwner returns the owner for the machineSet, or nil +// if there is no owner. A DeepCopy() of the object is returned on +// success. +func (c *machineController) findMachineSetOwner(machineSet *unstructured.Unstructured) (*unstructured.Unstructured, error) { + machineSetOwnerRef := machineSetOwnerRef(machineSet) + if machineSetOwnerRef == nil { return nil, nil } - return machineSet, nil + return c.findMachineDeployment(fmt.Sprintf("%s/%s", machineSet.GetNamespace(), machineSetOwnerRef.Name)) } // run starts shared informers and waits for the informer cache to // synchronize. func (c *machineController) run(stopCh <-chan struct{}) error { - c.kubeInformerFactory.Start(stopCh) - c.machineInformerFactory.Start(stopCh) + c.workloadInformerFactory.Start(stopCh) + c.managementInformerFactory.Start(stopCh) syncFuncs := []cache.InformerSynced{ c.nodeInformer.HasSynced, c.machineInformer.Informer().HasSynced, c.machineSetInformer.Informer().HasSynced, } - if c.machineDeploymentResource != nil { + if c.machineDeploymentsAvailable { syncFuncs = append(syncFuncs, c.machineDeploymentInformer.Informer().HasSynced) } @@ -203,9 +178,42 @@ func (c *machineController) run(stopCh <-chan struct{}) error { return nil } +func (c *machineController) findScalableResourceByProviderID(providerID normalizedProviderID) (*unstructured.Unstructured, error) { + machine, err := c.findMachineByProviderID(providerID) + if err != nil { + return nil, err + } + + if machine == nil { + return nil, nil + } + + scalableResource, err := c.findMachineOwner(machine) + if err != nil { + return nil, err + } + + if scalableResource == nil { + return nil, nil + } + + if c.machineDeploymentsAvailable { + machineDeployment, err := c.findMachineSetOwner(scalableResource) + if err != nil { + return nil, err + } + + // If a machineDeployment was found return a new nodegroup for the machineDeployment + if machineDeployment != nil { + scalableResource = machineDeployment + } + } + return scalableResource, nil +} + // findMachineByProviderID finds machine matching providerID. A // DeepCopy() of the object is returned on success. -func (c *machineController) findMachineByProviderID(providerID normalizedProviderID) (*Machine, error) { +func (c *machineController) findMachineByProviderID(providerID normalizedProviderID) (*unstructured.Unstructured, error) { objs, err := c.machineInformer.Informer().GetIndexer().ByIndex(machineProviderIDIndex, string(providerID)) if err != nil { return nil, err @@ -219,20 +227,11 @@ func (c *machineController) findMachineByProviderID(providerID normalizedProvide if !ok { return nil, fmt.Errorf("internal error; unexpected type %T", objs[0]) } - machine := newMachineFromUnstructured(u.DeepCopy()) - if machine != nil { - return machine, nil - } + return u.DeepCopy(), nil } if isFailedMachineProviderID(providerID) { - machine, err := c.findMachine(machineKeyFromFailedProviderID(providerID)) - if err != nil { - return nil, err - } - if machine != nil { - return machine.DeepCopy(), nil - } + return c.findMachine(machineKeyFromFailedProviderID(providerID)) } // If the machine object has no providerID--maybe actuator @@ -280,29 +279,6 @@ func (c *machineController) findNodeByNodeName(name string) (*corev1.Node, error return node.DeepCopy(), nil } -// machinesInMachineSet returns all the machines that belong to -// machineSet. For each machine in the set a DeepCopy() of the object -// is returned. -func (c *machineController) machinesInMachineSet(machineSet *MachineSet) ([]*Machine, error) { - machines, err := c.listMachines(machineSet.Namespace, labels.SelectorFromSet(machineSet.Labels)) - if err != nil { - return nil, err - } - if machines == nil { - return nil, nil - } - - var result []*Machine - - for _, machine := range machines { - if machineIsOwnedByMachineSet(machine, machineSet) { - result = append(result, machine) - } - } - - return result, nil -} - // getCAPIGroup returns a string that specifies the group for the API. // It will return either the value from the // CAPI_GROUP environment variable, or the default value i.e cluster.x-k8s.io. @@ -319,57 +295,58 @@ func getCAPIGroup() string { // Machines and MachineSet as they are added, updated and deleted on // the cluster. func newMachineController( - dynamicclient dynamic.Interface, - kubeclient kubeclient.Interface, - discoveryclient discovery.DiscoveryInterface, + managementClient dynamic.Interface, + workloadClient kubeclient.Interface, + managementDiscoveryClient discovery.DiscoveryInterface, + managementScaleClient scale.ScalesGetter, ) (*machineController, error) { - kubeInformerFactory := kubeinformers.NewSharedInformerFactory(kubeclient, 0) - informerFactory := dynamicinformer.NewFilteredDynamicSharedInformerFactory(dynamicclient, 0, metav1.NamespaceAll, nil) + workloadInformerFactory := kubeinformers.NewSharedInformerFactory(workloadClient, 0) + managementInformerFactory := dynamicinformer.NewFilteredDynamicSharedInformerFactory(managementClient, 0, metav1.NamespaceAll, nil) CAPIGroup := getCAPIGroup() - CAPIVersion, err := getAPIGroupPreferredVersion(discoveryclient, CAPIGroup) + CAPIVersion, err := getAPIGroupPreferredVersion(managementDiscoveryClient, CAPIGroup) if err != nil { return nil, fmt.Errorf("could not find preferred version for CAPI group %q: %v", CAPIGroup, err) } klog.Infof("Using version %q for API group %q", CAPIVersion, CAPIGroup) - var gvrMachineDeployment *schema.GroupVersionResource + var gvrMachineDeployment schema.GroupVersionResource var machineDeploymentInformer informers.GenericInformer - machineDeployment, err := groupVersionHasResource(discoveryclient, + machineDeploymentAvailable, err := groupVersionHasResource(managementDiscoveryClient, fmt.Sprintf("%s/%s", CAPIGroup, CAPIVersion), resourceNameMachineDeployment) if err != nil { return nil, fmt.Errorf("failed to validate if resource %q is available for group %q: %v", resourceNameMachineDeployment, fmt.Sprintf("%s/%s", CAPIGroup, CAPIVersion), err) } - if machineDeployment { - gvrMachineDeployment = &schema.GroupVersionResource{ + if machineDeploymentAvailable { + gvrMachineDeployment = schema.GroupVersionResource{ Group: CAPIGroup, Version: CAPIVersion, Resource: resourceNameMachineDeployment, } - machineDeploymentInformer = informerFactory.ForResource(*gvrMachineDeployment) + machineDeploymentInformer = managementInformerFactory.ForResource(gvrMachineDeployment) machineDeploymentInformer.Informer().AddEventHandler(cache.ResourceEventHandlerFuncs{}) } - gvrMachineSet := &schema.GroupVersionResource{ + gvrMachineSet := schema.GroupVersionResource{ Group: CAPIGroup, Version: CAPIVersion, Resource: resourceNameMachineSet, } - machineSetInformer := informerFactory.ForResource(*gvrMachineSet) + machineSetInformer := managementInformerFactory.ForResource(gvrMachineSet) machineSetInformer.Informer().AddEventHandler(cache.ResourceEventHandlerFuncs{}) - gvrMachine := &schema.GroupVersionResource{ + gvrMachine := schema.GroupVersionResource{ Group: CAPIGroup, Version: CAPIVersion, Resource: resourceNameMachine, } - machineInformer := informerFactory.ForResource(*gvrMachine) + machineInformer := managementInformerFactory.ForResource(gvrMachine) machineInformer.Informer().AddEventHandler(cache.ResourceEventHandlerFuncs{}) - nodeInformer := kubeInformerFactory.Core().V1().Nodes().Informer() + nodeInformer := workloadInformerFactory.Core().V1().Nodes().Informer() nodeInformer.AddEventHandler(cache.ResourceEventHandlerFuncs{}) if err := machineInformer.Informer().GetIndexer().AddIndexers(cache.Indexers{ @@ -385,16 +362,18 @@ func newMachineController( } return &machineController{ - kubeInformerFactory: kubeInformerFactory, - machineInformerFactory: informerFactory, - machineDeploymentInformer: machineDeploymentInformer, - machineInformer: machineInformer, - machineSetInformer: machineSetInformer, - nodeInformer: nodeInformer, - dynamicclient: dynamicclient, - machineSetResource: gvrMachineSet, - machineResource: gvrMachine, - machineDeploymentResource: gvrMachineDeployment, + workloadInformerFactory: workloadInformerFactory, + managementInformerFactory: managementInformerFactory, + machineDeploymentInformer: machineDeploymentInformer, + machineInformer: machineInformer, + machineSetInformer: machineSetInformer, + nodeInformer: nodeInformer, + managementClient: managementClient, + managementScaleClient: managementScaleClient, + machineSetResource: gvrMachineSet, + machineResource: gvrMachine, + machineDeploymentResource: gvrMachineDeployment, + machineDeploymentsAvailable: machineDeploymentAvailable, }, nil } @@ -428,179 +407,125 @@ func getAPIGroupPreferredVersion(client discovery.DiscoveryInterface, APIGroup s return "", fmt.Errorf("failed to find API group %q", APIGroup) } -func (c *machineController) machineSetProviderIDs(machineSet *MachineSet) ([]string, error) { - machines, err := c.machinesInMachineSet(machineSet) +func (c *machineController) scalableResourceProviderIDs(scalableResource *unstructured.Unstructured) ([]string, error) { + machines, err := c.listMachinesForScalableResource(scalableResource) if err != nil { return nil, fmt.Errorf("error listing machines: %v", err) } var providerIDs []string for _, machine := range machines { - if machine.Spec.ProviderID == nil || *machine.Spec.ProviderID == "" { - klog.Warningf("Machine %q has no providerID", machine.Name) + providerID, found, err := unstructured.NestedString(machine.Object, "spec", "providerID") + if err != nil { + return nil, err } - if machine.Spec.ProviderID != nil && *machine.Spec.ProviderID != "" { - providerIDs = append(providerIDs, *machine.Spec.ProviderID) - continue + if found { + if providerID != "" { + providerIDs = append(providerIDs, providerID) + continue + } + } + + klog.Warningf("Machine %q has no providerID", machine.GetName()) + + failureMessage, found, err := unstructured.NestedString(machine.Object, "status", "failureMessage") + if err != nil { + return nil, err } - if machine.Status.FailureMessage != nil { - klog.V(4).Infof("Status.FailureMessage of machine %q is %q", machine.Name, *machine.Status.FailureMessage) + if found { + klog.V(4).Infof("Status.FailureMessage of machine %q is %q", machine.GetName(), failureMessage) // Provide a fake ID to allow the autoscaler to track machines that will never // become nodes and mark the nodegroup unhealthy after maxNodeProvisionTime. // Fake ID needs to be recognised later and converted into a machine key. // Use an underscore as a separator between namespace and name as it is not a // valid character within a namespace name. - providerIDs = append(providerIDs, fmt.Sprintf("%s%s_%s", failedMachinePrefix, machine.Namespace, machine.Name)) + providerIDs = append(providerIDs, fmt.Sprintf("%s%s_%s", failedMachinePrefix, machine.GetNamespace(), machine.GetName())) continue } - if machine.Status.NodeRef == nil { - klog.V(4).Infof("Status.NodeRef of machine %q is currently nil", machine.Name) - continue + _, found, err = unstructured.NestedFieldCopy(machine.Object, "status", "nodeRef") + if err != nil { + return nil, err } - if machine.Status.NodeRef.Kind != "Node" { - klog.Errorf("Status.NodeRef of machine %q does not reference a node (rather %q)", machine.Name, machine.Status.NodeRef.Kind) + if !found { + klog.V(4).Infof("Status.NodeRef of machine %q is currently nil", machine.GetName()) continue } - node, err := c.findNodeByNodeName(machine.Status.NodeRef.Name) + nodeRefKind, found, err := unstructured.NestedString(machine.Object, "status", "nodeRef", "kind") if err != nil { - return nil, fmt.Errorf("unknown node %q", machine.Status.NodeRef.Name) + return nil, err } - if node != nil { - providerIDs = append(providerIDs, node.Spec.ProviderID) + if found && nodeRefKind != "Node" { + klog.Errorf("Status.NodeRef of machine %q does not reference a node (rather %q)", machine.GetName(), nodeRefKind) + continue } - } - - klog.V(4).Infof("nodegroup %s has nodes %v", machineSet.Name, providerIDs) - return providerIDs, nil -} -func (c *machineController) filterAllMachineSets(f machineSetFilterFunc) error { - return c.filterMachineSets(metav1.NamespaceAll, f) -} - -func (c *machineController) filterMachineSets(namespace string, f machineSetFilterFunc) error { - machineSets, err := c.listMachineSets(namespace, labels.Everything()) - if err != nil { - return nil - } - for _, machineSet := range machineSets { - if err := f(machineSet); err != nil { - return err + nodeRefName, found, err := unstructured.NestedString(machine.Object, "status", "nodeRef", "name") + if err != nil { + return nil, err } - } - return nil -} -func (c *machineController) machineSetNodeGroups() ([]*nodegroup, error) { - var nodegroups []*nodegroup + if found { + node, err := c.findNodeByNodeName(nodeRefName) + if err != nil { + return nil, fmt.Errorf("unknown node %q", nodeRefName) + } - if err := c.filterAllMachineSets(func(machineSet *MachineSet) error { - if machineSetHasMachineDeploymentOwnerRef(machineSet) { - return nil - } - ng, err := newNodegroupFromMachineSet(c, machineSet) - if err != nil { - return err - } - if ng.MaxSize()-ng.MinSize() > 0 && pointer.Int32PtrDerefOr(machineSet.Spec.Replicas, 0) > 0 { - nodegroups = append(nodegroups, ng) + if node != nil { + providerIDs = append(providerIDs, node.Spec.ProviderID) + } } - return nil - }); err != nil { - return nil, err } - return nodegroups, nil + klog.V(4).Infof("nodegroup %s has nodes %v", scalableResource.GetName(), providerIDs) + + return providerIDs, nil } -func (c *machineController) machineDeploymentNodeGroups() ([]*nodegroup, error) { - machineDeployments, err := c.listMachineDeployments(metav1.NamespaceAll, labels.Everything()) +func (c *machineController) nodeGroups() ([]*nodegroup, error) { + scalableResources, err := c.listScalableResources() if err != nil { return nil, err } - var nodegroups []*nodegroup + nodegroups := make([]*nodegroup, 0, len(scalableResources)) - for _, md := range machineDeployments { - ng, err := newNodegroupFromMachineDeployment(c, md) + for _, r := range scalableResources { + 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 && pointer.Int32PtrDerefOr(md.Spec.Replicas, 0) > 0 { - nodegroups = append(nodegroups, ng) - } - } - return nodegroups, nil -} - -func (c *machineController) nodeGroups() ([]*nodegroup, error) { - machineSets, err := c.machineSetNodeGroups() - if err != nil { - return nil, err - } + // add nodegroup iff it has the capacity to scale + if ng.MaxSize()-ng.MinSize() > 0 { + replicas, found, err := unstructured.NestedInt64(r.Object, "spec", "replicas") + if err != nil { + return nil, err + } - if c.machineDeploymentResource != nil { - machineDeployments, err := c.machineDeploymentNodeGroups() - if err != nil { - return nil, err + if found && replicas > 0 { + nodegroups = append(nodegroups, ng) + } } - machineSets = append(machineSets, machineDeployments...) } - - return machineSets, nil + return nodegroups, nil } func (c *machineController) nodeGroupForNode(node *corev1.Node) (*nodegroup, error) { - machine, err := c.findMachineByProviderID(normalizedProviderString(node.Spec.ProviderID)) - if err != nil { - return nil, err - } - if machine == nil { - return nil, nil - } - - machineSet, err := c.findMachineOwner(machine) + scalableResource, err := c.findScalableResourceByProviderID(normalizedProviderString(node.Spec.ProviderID)) if err != nil { return nil, err } - - if machineSet == nil { + if scalableResource == nil { return nil, nil } - if c.machineDeploymentResource != nil { - if ref := machineSetMachineDeploymentRef(machineSet); ref != nil { - key := fmt.Sprintf("%s/%s", machineSet.Namespace, ref.Name) - machineDeployment, err := c.findMachineDeployment(key) - if err != nil { - return nil, fmt.Errorf("unknown MachineDeployment %q: %v", key, err) - } - if machineDeployment == nil { - return nil, fmt.Errorf("unknown MachineDeployment %q", key) - } - nodegroup, err := newNodegroupFromMachineDeployment(c, machineDeployment) - if err != nil { - return nil, fmt.Errorf("failed to build nodegroup for node %q: %v", node.Name, err) - } - // We don't scale from 0 so nodes must belong - // to a nodegroup that has a scale size of at - // least 1. - if nodegroup.MaxSize()-nodegroup.MinSize() < 1 { - return nil, nil - } - return nodegroup, nil - } - } - - nodegroup, err := newNodegroupFromMachineSet(c, machineSet) + nodegroup, err := newNodegroupFromScalableResource(c, scalableResource) if err != nil { return nil, fmt.Errorf("failed to build nodegroup for node %q: %v", node.Name, err) } @@ -611,7 +536,7 @@ func (c *machineController) nodeGroupForNode(node *corev1.Node) (*nodegroup, err return nil, nil } - klog.V(4).Infof("node %q is in nodegroup %q", node.Name, machineSet.Name) + klog.V(4).Infof("node %q is in nodegroup %q", node.Name, nodegroup.Id()) return nodegroup, nil } @@ -639,80 +564,75 @@ func (c *machineController) findNodeByProviderID(providerID normalizedProviderID return node.DeepCopy(), nil } -func (c *machineController) getMachine(namespace, name string, options metav1.GetOptions) (*Machine, error) { - u, err := c.dynamicclient.Resource(*c.machineResource).Namespace(namespace).Get(context.TODO(), name, options) - if err != nil { - return nil, err - } - return newMachineFromUnstructured(u.DeepCopy()), nil -} - -func (c *machineController) getMachineSet(namespace, name string, options metav1.GetOptions) (*MachineSet, error) { - u, err := c.dynamicclient.Resource(*c.machineSetResource).Namespace(namespace).Get(context.TODO(), name, options) - if err != nil { - return nil, err - } - return newMachineSetFromUnstructured(u.DeepCopy()), nil -} - -func (c *machineController) getMachineDeployment(namespace, name string, options metav1.GetOptions) (*MachineDeployment, error) { - u, err := c.dynamicclient.Resource(*c.machineDeploymentResource).Namespace(namespace).Get(context.TODO(), name, options) - if err != nil { - return nil, err - } - return newMachineDeploymentFromUnstructured(u.DeepCopy()), nil -} +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") + if err != nil { + return nil, err + } -func (c *machineController) listMachines(namespace string, selector labels.Selector) ([]*Machine, error) { - objs, err := c.machineInformer.Lister().ByNamespace(namespace).List(selector) - if err != nil { - return nil, err - } + if !found { + return nil, fmt.Errorf("expected field spec.selector on scalable resource type") + } - var machines []*Machine + labelSelector := &metav1.LabelSelector{} + if err := runtime.DefaultUnstructuredConverter.FromUnstructured(unstructuredSelector, labelSelector); err != nil { + return nil, err + } - for _, x := range objs { - u := x.(*unstructured.Unstructured).DeepCopy() - if machine := newMachineFromUnstructured(u); machine != nil { - machines = append(machines, machine) + selector, err := metav1.LabelSelectorAsSelector(labelSelector) + if err != nil { + return nil, err } - } - return machines, nil + return listResources(c.machineInformer.Lister().ByNamespace(r.GetNamespace()), selector) + default: + return nil, fmt.Errorf("unknown scalable resource kind %s", r.GetKind()) + } } -func (c *machineController) listMachineSets(namespace string, selector labels.Selector) ([]*MachineSet, error) { - objs, err := c.machineSetInformer.Lister().ByNamespace(namespace).List(selector) +func (c *machineController) listScalableResources() ([]*unstructured.Unstructured, error) { + scalableResources, err := c.listResources(c.machineSetInformer.Lister()) if err != nil { return nil, err } - var machineSets []*MachineSet - - for _, x := range objs { - u := x.(*unstructured.Unstructured).DeepCopy() - if machineSet := newMachineSetFromUnstructured(u); machineSet != nil { - machineSets = append(machineSets, machineSet) + if c.machineDeploymentsAvailable { + machineDeployments, err := c.listResources(c.machineDeploymentInformer.Lister()) + if err != nil { + return nil, err } + + scalableResources = append(scalableResources, machineDeployments...) } + return scalableResources, nil +} - return machineSets, nil +func (c *machineController) listResources(lister cache.GenericLister) ([]*unstructured.Unstructured, error) { + return listResources(lister.ByNamespace(metav1.NamespaceAll), labels.Everything()) } -func (c *machineController) listMachineDeployments(namespace string, selector labels.Selector) ([]*MachineDeployment, error) { - objs, err := c.machineDeploymentInformer.Lister().ByNamespace(namespace).List(selector) +func listResources(lister cache.GenericNamespaceLister, selector labels.Selector) ([]*unstructured.Unstructured, error) { + objs, err := lister.List(selector) if err != nil { return nil, err } - var machineDeployments []*MachineDeployment - + results := make([]*unstructured.Unstructured, 0, len(objs)) for _, x := range objs { - u := x.(*unstructured.Unstructured).DeepCopy() - if machineDeployment := newMachineDeploymentFromUnstructured(u); machineDeployment != nil { - machineDeployments = append(machineDeployments, machineDeployment) + u, ok := x.(*unstructured.Unstructured) + if !ok { + return nil, fmt.Errorf("expected unstructured resource from lister, not %T", x) + } + + // if we are listing MachineSets, do not return MachineSets that are owned by a MachineDeployment + if u.GetKind() == machineSetKind && machineSetHasMachineDeploymentOwnerRef(u) { + continue } + + results = append(results, u.DeepCopy()) } - return machineDeployments, nil + return results, nil } diff --git a/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_controller_test.go b/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_controller_test.go index f498ca24cf20..0f146cf7383b 100644 --- a/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_controller_test.go +++ b/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_controller_test.go @@ -17,33 +17,39 @@ limitations under the License. package clusterapi import ( + "context" "fmt" + "math/rand" "os" "path" "reflect" "sort" - "strings" "testing" + "time" + autoscalingv1 "k8s.io/api/autoscaling/v1" corev1 "k8s.io/api/core/v1" - v1 "k8s.io/apimachinery/pkg/apis/meta/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/runtime" - "k8s.io/apimachinery/pkg/types" + "k8s.io/apimachinery/pkg/runtime/schema" fakediscovery "k8s.io/client-go/discovery/fake" + "k8s.io/client-go/dynamic" fakedynamic "k8s.io/client-go/dynamic/fake" + "k8s.io/client-go/informers" fakekube "k8s.io/client-go/kubernetes/fake" + fakescale "k8s.io/client-go/scale/fake" clientgotesting "k8s.io/client-go/testing" - "k8s.io/utils/pointer" ) type testControllerShutdownFunc func() type testConfig struct { spec *testSpec - machineDeployment *MachineDeployment - machineSet *MachineSet - machines []*Machine + machineDeployment *unstructured.Unstructured + machineSet *unstructured.Unstructured + machines []*unstructured.Unstructured nodes []*corev1.Node } @@ -70,12 +76,12 @@ func mustCreateTestController(t *testing.T, testConfigs ...*testConfig) (*machin } for i := range config.machines { - machineObjects = append(machineObjects, newUnstructuredFromMachine(config.machines[i])) + machineObjects = append(machineObjects, config.machines[i]) } - machineObjects = append(machineObjects, newUnstructuredFromMachineSet(config.machineSet)) + machineObjects = append(machineObjects, config.machineSet) if config.machineDeployment != nil { - machineObjects = append(machineObjects, newUnstructuredFromMachineDeployment(config.machineDeployment)) + machineObjects = append(machineObjects, config.machineDeployment) } } @@ -83,10 +89,10 @@ func mustCreateTestController(t *testing.T, testConfigs ...*testConfig) (*machin dynamicClientset := fakedynamic.NewSimpleDynamicClient(runtime.NewScheme(), machineObjects...) discoveryClient := &fakediscovery.FakeDiscovery{ Fake: &clientgotesting.Fake{ - Resources: []*v1.APIResourceList{ + Resources: []*metav1.APIResourceList{ { GroupVersion: fmt.Sprintf("%s/v1beta1", customCAPIGroup), - APIResources: []v1.APIResource{ + APIResources: []metav1.APIResource{ { Name: resourceNameMachineDeployment, }, @@ -100,7 +106,7 @@ func mustCreateTestController(t *testing.T, testConfigs ...*testConfig) (*machin }, { GroupVersion: fmt.Sprintf("%s/v1alpha3", defaultCAPIGroup), - APIResources: []v1.APIResource{ + APIResources: []metav1.APIResource{ { Name: resourceNameMachineDeployment, }, @@ -115,7 +121,98 @@ func mustCreateTestController(t *testing.T, testConfigs ...*testConfig) (*machin }, }, } - controller, err := newMachineController(dynamicClientset, kubeclientSet, discoveryClient) + + scaleClient := &fakescale.FakeScaleClient{Fake: clientgotesting.Fake{}} + scaleReactor := func(action clientgotesting.Action) (bool, runtime.Object, error) { + resource := action.GetResource().Resource + if resource != resourceNameMachineSet && resource != resourceNameMachineDeployment { + // Do not attempt to react to resources that are not MachineSet or MachineDeployment + return false, nil, nil + } + + subresource := action.GetSubresource() + if subresource != "scale" { + // Handle a bug in the client-go fakeNamespaceScaleClient, where the action namespace and subresource are + // switched for update actions + if action.GetVerb() == "update" && action.GetNamespace() == "scale" { + subresource = "scale" + } else { + // Do not attempt to respond to anything but scale subresource requests + return false, nil, nil + } + } + + gvr := schema.GroupVersionResource{ + Group: action.GetResource().Group, + Version: "v1alpha3", + Resource: resource, + } + + switch action.GetVerb() { + case "get": + action, ok := action.(clientgotesting.GetAction) + if !ok { + return true, nil, fmt.Errorf("failed to convert Action to GetAction: %T", action) + } + + u, err := dynamicClientset.Resource(gvr).Namespace(action.GetNamespace()).Get(context.TODO(), action.GetName(), metav1.GetOptions{}) + if err != nil { + return true, nil, err + } + + replicas, found, err := unstructured.NestedInt64(u.Object, "spec", "replicas") + if err != nil { + return true, nil, err + } + + if !found { + replicas = 0 + } + + result := &autoscalingv1.Scale{ + ObjectMeta: metav1.ObjectMeta{ + Name: u.GetName(), + Namespace: u.GetNamespace(), + }, + Spec: autoscalingv1.ScaleSpec{ + Replicas: int32(replicas), + }, + } + + return true, result, nil + case "update": + action, ok := action.(clientgotesting.UpdateAction) + if !ok { + return true, nil, fmt.Errorf("failed to convert Action to UpdateAction: %T", action) + } + + s, ok := action.GetObject().(*autoscalingv1.Scale) + if !ok { + return true, nil, fmt.Errorf("failed to convert Resource to Scale: %T", s) + } + + u, err := dynamicClientset.Resource(gvr).Namespace(s.Namespace).Get(context.TODO(), s.Name, metav1.GetOptions{}) + if err != nil { + return true, nil, fmt.Errorf("failed to fetch underlying %s resource: %s/%s", resource, s.Namespace, s.Name) + } + + if err := unstructured.SetNestedField(u.Object, int64(s.Spec.Replicas), "spec", "replicas"); err != nil { + return true, nil, err + } + + _, err = dynamicClientset.Resource(gvr).Namespace(s.Namespace).Update(context.TODO(), u, metav1.UpdateOptions{}) + if err != nil { + return true, nil, err + } + + return true, s, nil + default: + return true, nil, fmt.Errorf("unknown verb: %v", action.GetVerb()) + } + } + scaleClient.AddReactor("*", "*", scaleReactor) + + controller, err := newMachineController(dynamicClientset, kubeclientSet, discoveryClient, scaleClient) if err != nil { t.Fatal("failed to create test controller") } @@ -130,31 +227,31 @@ func mustCreateTestController(t *testing.T, testConfigs ...*testConfig) (*machin } } -func createMachineSetTestConfig(namespace string, nodeCount int, annotations map[string]string) *testConfig { - return createTestConfigs(createTestSpecs(namespace, 1, nodeCount, false, annotations)...)[0] +func createMachineSetTestConfig(namespace, namePrefix string, nodeCount int, annotations map[string]string) *testConfig { + return createTestConfigs(createTestSpecs(namespace, namePrefix, 1, nodeCount, false, annotations)...)[0] } -func createMachineSetTestConfigs(namespace string, configCount, nodeCount int, annotations map[string]string) []*testConfig { - return createTestConfigs(createTestSpecs(namespace, configCount, nodeCount, false, annotations)...) +func createMachineSetTestConfigs(namespace, namePrefix string, configCount, nodeCount int, annotations map[string]string) []*testConfig { + return createTestConfigs(createTestSpecs(namespace, namePrefix, configCount, nodeCount, false, annotations)...) } -func createMachineDeploymentTestConfig(namespace string, nodeCount int, annotations map[string]string) *testConfig { - return createTestConfigs(createTestSpecs(namespace, 1, nodeCount, true, annotations)...)[0] +func createMachineDeploymentTestConfig(namespace, namePrefix string, nodeCount int, annotations map[string]string) *testConfig { + return createTestConfigs(createTestSpecs(namespace, namePrefix, 1, nodeCount, true, annotations)...)[0] } -func createMachineDeploymentTestConfigs(namespace string, configCount, nodeCount int, annotations map[string]string) []*testConfig { - return createTestConfigs(createTestSpecs(namespace, configCount, nodeCount, true, annotations)...) +func createMachineDeploymentTestConfigs(namespace, namePrefix string, configCount, nodeCount int, annotations map[string]string) []*testConfig { + return createTestConfigs(createTestSpecs(namespace, namePrefix, configCount, nodeCount, true, annotations)...) } -func createTestSpecs(namespace string, scalableResourceCount, nodeCount int, isMachineDeployment bool, annotations map[string]string) []testSpec { +func createTestSpecs(namespace, 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("machinedeployment-%d", i), - machineSetName: fmt.Sprintf("machineset-%d", i), - namespace: strings.ToLower(namespace), + machineDeploymentName: fmt.Sprintf("%s-%d", namePrefix, i), + machineSetName: fmt.Sprintf("%s-%d", namePrefix, i), + namespace: namespace, nodeCount: nodeCount, rootIsMachineDeployment: isMachineDeployment, }) @@ -164,63 +261,85 @@ func createTestSpecs(namespace string, scalableResourceCount, nodeCount int, isM } func createTestConfigs(specs ...testSpec) []*testConfig { - var result []*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([]*Machine, spec.nodeCount), + machines: make([]*unstructured.Unstructured, spec.nodeCount), } - config.machineSet = &MachineSet{ - TypeMeta: v1.TypeMeta{ - APIVersion: fmt.Sprintf("%s/v1alpha3", defaultCAPIGroup), - Kind: "MachineSet", - }, - ObjectMeta: v1.ObjectMeta{ - Name: spec.machineSetName, - Namespace: spec.namespace, - UID: types.UID(spec.machineSetName), + machineSetLabels := map[string]string{ + "machineSetName": spec.machineSetName, + } + + config.machineSet = &unstructured.Unstructured{ + Object: map[string]interface{}{ + "kind": machineSetKind, + "apiVersion": "cluster.x-k8s.io/v1alpha3", + "metadata": map[string]interface{}{ + "name": spec.machineSetName, + "namespace": spec.namespace, + "uid": spec.machineSetName, + }, + "spec": map[string]interface{}{ + "replicas": int64(spec.nodeCount), + }, + "status": map[string]interface{}{}, }, } + config.machineSet.SetAnnotations(make(map[string]string)) + if !spec.rootIsMachineDeployment { - config.machineSet.ObjectMeta.Annotations = spec.annotations - config.machineSet.Spec.Replicas = int32ptr(int32(spec.nodeCount)) + config.machineSet.SetAnnotations(spec.annotations) } else { - config.machineDeployment = &MachineDeployment{ - TypeMeta: v1.TypeMeta{ - APIVersion: fmt.Sprintf("%s/v1alpha3", defaultCAPIGroup), - Kind: "MachineDeployment", - }, - ObjectMeta: v1.ObjectMeta{ - Name: spec.machineDeploymentName, - Namespace: spec.namespace, - UID: types.UID(spec.machineDeploymentName), - Annotations: spec.annotations, - }, - Spec: MachineDeploymentSpec{ - Replicas: int32ptr(int32(spec.nodeCount)), + machineSetLabels["machineDeploymentName"] = spec.machineDeploymentName + + machineDeploymentLabels := map[string]string{ + "machineDeploymentName": spec.machineDeploymentName, + } + + config.machineDeployment = &unstructured.Unstructured{ + Object: map[string]interface{}{ + "kind": machineDeploymentKind, + "apiVersion": "cluster.x-k8s.io/v1alpha3", + "metadata": map[string]interface{}{ + "name": spec.machineDeploymentName, + "namespace": spec.namespace, + "uid": spec.machineDeploymentName, + }, + "spec": map[string]interface{}{ + "replicas": int64(spec.nodeCount), + }, + "status": map[string]interface{}{}, }, } + config.machineDeployment.SetAnnotations(spec.annotations) + config.machineDeployment.SetLabels(machineDeploymentLabels) + unstructured.SetNestedStringMap(config.machineDeployment.Object, machineDeploymentLabels, "spec", "selector", "matchLabels") - config.machineSet.OwnerReferences = make([]v1.OwnerReference, 1) - config.machineSet.OwnerReferences[0] = v1.OwnerReference{ - Name: config.machineDeployment.Name, - Kind: config.machineDeployment.Kind, - UID: config.machineDeployment.UID, + ownerRefs := []metav1.OwnerReference{ + { + Name: config.machineDeployment.GetName(), + Kind: config.machineDeployment.GetKind(), + UID: config.machineDeployment.GetUID(), + }, } + config.machineSet.SetOwnerReferences(ownerRefs) } + config.machineSet.SetLabels(machineSetLabels) + unstructured.SetNestedStringMap(config.machineSet.Object, machineSetLabels, "spec", "selector", "matchLabels") - machineOwner := v1.OwnerReference{ - Name: config.machineSet.Name, - Kind: config.machineSet.Kind, - UID: config.machineSet.UID, + machineOwner := metav1.OwnerReference{ + Name: config.machineSet.GetName(), + Kind: config.machineSet.GetKind(), + UID: config.machineSet.GetUID(), } for j := 0; j < spec.nodeCount; j++ { - config.nodes[j], config.machines[j] = makeLinkedNodeAndMachine(j, spec.namespace, machineOwner) + config.nodes[j], config.machines[j] = makeLinkedNodeAndMachine(j, spec.namespace, machineOwner, machineSetLabels) } result = append(result, config) @@ -232,12 +351,12 @@ 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 v1.OwnerReference) (*corev1.Node, *Machine) { +func makeLinkedNodeAndMachine(i int, namespace string, owner metav1.OwnerReference, machineLabels map[string]string) (*corev1.Node, *unstructured.Unstructured) { node := &corev1.Node{ - TypeMeta: v1.TypeMeta{ + TypeMeta: metav1.TypeMeta{ Kind: "Node", }, - ObjectMeta: v1.ObjectMeta{ + ObjectMeta: metav1.ObjectMeta{ Name: fmt.Sprintf("%s-%s-node-%d", namespace, owner.Name, i), Annotations: map[string]string{ machineAnnotationKey: fmt.Sprintf("%s/%s-%s-machine-%d", namespace, namespace, owner.Name, i), @@ -248,56 +367,50 @@ func makeLinkedNodeAndMachine(i int, namespace string, owner v1.OwnerReference) }, } - machine := &Machine{ - TypeMeta: v1.TypeMeta{ - APIVersion: fmt.Sprintf("%s/v1alpha3", defaultCAPIGroup), - Kind: "Machine", - }, - ObjectMeta: v1.ObjectMeta{ - Name: fmt.Sprintf("%s-%s-machine-%d", namespace, owner.Name, i), - Namespace: namespace, - OwnerReferences: []v1.OwnerReference{{ - Name: owner.Name, - Kind: owner.Kind, - UID: owner.UID, - }}, - }, - Spec: MachineSpec{ - ProviderID: pointer.StringPtr(fmt.Sprintf("test:////%s-%s-nodeid-%d", namespace, owner.Name, i)), - }, - Status: MachineStatus{ - NodeRef: &corev1.ObjectReference{ - Kind: node.Kind, - Name: node.Name, + machine := &unstructured.Unstructured{ + Object: map[string]interface{}{ + "kind": machineKind, + "apiVersion": "cluster.x-k8s.io/v1alpha3", + "metadata": map[string]interface{}{ + "name": fmt.Sprintf("%s-%s-machine-%d", namespace, owner.Name, i), + "namespace": namespace, + }, + "spec": map[string]interface{}{ + "providerID": fmt.Sprintf("test:////%s-%s-nodeid-%d", namespace, owner.Name, i), + }, + "status": map[string]interface{}{ + "nodeRef": map[string]interface{}{ + "kind": node.Kind, + "name": node.Name, + }, }, }, } + machine.SetOwnerReferences([]metav1.OwnerReference{owner}) + machine.SetLabels(machineLabels) return node, machine } -func int32ptr(v int32) *int32 { - return &v -} - func addTestConfigs(t *testing.T, controller *machineController, testConfigs ...*testConfig) error { t.Helper() for _, config := range testConfigs { if config.machineDeployment != nil { - - if err := controller.machineDeploymentInformer.Informer().GetStore().Add(newUnstructuredFromMachineDeployment(config.machineDeployment)); err != nil { + if err := createResource(controller.managementClient, controller.machineDeploymentInformer, controller.machineDeploymentResource, config.machineDeployment); err != nil { return err } } - if err := controller.machineSetInformer.Informer().GetStore().Add(newUnstructuredFromMachineSet(config.machineSet)); err != nil { + if err := createResource(controller.managementClient, controller.machineSetInformer, controller.machineSetResource, config.machineSet); err != nil { return err } + for i := range config.machines { - if err := controller.machineInformer.Informer().GetStore().Add(newUnstructuredFromMachine(config.machines[i])); err != nil { + if err := createResource(controller.managementClient, controller.machineInformer, controller.machineResource, config.machines[i]); err != nil { return err } } + for i := range config.nodes { if err := controller.nodeInformer.GetStore().Add(config.nodes[i]); err != nil { return err @@ -307,6 +420,45 @@ 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.DeepCopy(), metav1.CreateOptions{}); err != nil { + return err + } + return informer.Informer().GetStore().Add(resource.DeepCopy()) +} + +func updateResource(client dynamic.Interface, informer informers.GenericInformer, gvr schema.GroupVersionResource, resource *unstructured.Unstructured) error { + if _, err := client.Resource(gvr).Namespace(resource.GetNamespace()).Update(context.TODO(), resource.DeepCopy(), metav1.UpdateOptions{}); err != nil { + return err + } + return informer.Informer().GetStore().Update(resource.DeepCopy()) +} + +func deleteResource(client dynamic.Interface, informer informers.GenericInformer, gvr schema.GroupVersionResource, resource *unstructured.Unstructured) error { + if err := client.Resource(gvr).Namespace(resource.GetNamespace()).Delete(context.TODO(), resource.GetName(), metav1.DeleteOptions{}); err != nil { + return err + } + return informer.Informer().GetStore().Delete(resource) +} + func deleteTestConfigs(t *testing.T, controller *machineController, testConfigs ...*testConfig) error { t.Helper() @@ -333,7 +485,7 @@ func deleteTestConfigs(t *testing.T, controller *machineController, testConfigs return nil } -func TestControllerFindMachineByID(t *testing.T) { +func TestControllerFindMachine(t *testing.T) { type testCase struct { description string name string @@ -369,26 +521,26 @@ func TestControllerFindMachineByID(t *testing.T) { } if tc.lookupSucceeds && machine != nil { - if machine.Name != tc.name { - t.Errorf("expected %q, got %q", tc.name, machine.Name) + if machine.GetName() != tc.name { + t.Errorf("expected %q, got %q", tc.name, machine.GetName()) } - if machine.Namespace != tc.namespace { - t.Errorf("expected %q, got %q", tc.namespace, machine.Namespace) + if machine.GetNamespace() != tc.namespace { + t.Errorf("expected %q, got %q", tc.namespace, machine.GetNamespace()) } } } for _, tc := range testCases { t.Run(tc.description, func(t *testing.T) { - testConfig := createMachineSetTestConfig(testNamespace, 1, map[string]string{ + testConfig := createMachineSetTestConfig(RandomString(6), RandomString(6), 1, map[string]string{ nodeGroupMinSizeAnnotationKey: "1", nodeGroupMaxSizeAnnotationKey: "10", }) if tc.name == "" { - tc.name = testConfig.machines[0].Name + tc.name = testConfig.machines[0].GetName() } if tc.namespace == "" { - tc.namespace = testConfig.machines[0].Namespace + tc.namespace = testConfig.machines[0].GetNamespace() } test(t, tc, testConfig) }) @@ -396,7 +548,7 @@ func TestControllerFindMachineByID(t *testing.T) { } func TestControllerFindMachineOwner(t *testing.T) { - testConfig := createMachineSetTestConfig(testNamespace, 1, map[string]string{ + testConfig := createMachineSetTestConfig(RandomString(6), RandomString(6), 1, map[string]string{ nodeGroupMinSizeAnnotationKey: "1", nodeGroupMaxSizeAnnotationKey: "10", }) @@ -412,13 +564,17 @@ func TestControllerFindMachineOwner(t *testing.T) { if testResult1 == nil { t.Fatal("expected non-nil result") } - if testConfig.spec.machineSetName != testResult1.Name { - t.Errorf("expected %q, got %q", testConfig.spec.machineSetName, testResult1.Name) + if testConfig.spec.machineSetName != testResult1.GetName() { + t.Errorf("expected %q, got %q", testConfig.spec.machineSetName, testResult1.GetName()) } - // Test #2: Lookup fails as the machine UUID != machineset UUID + // Test #2: Lookup fails as the machine ownerref Name != machineset Name testMachine2 := testConfig.machines[0].DeepCopy() - testMachine2.OwnerReferences[0].UID = "does-not-match-machineset" + ownerRefs := testMachine2.GetOwnerReferences() + ownerRefs[0].Name = "does-not-match-machineset" + + testMachine2.SetOwnerReferences(ownerRefs) + testResult2, err := controller.findMachineOwner(testMachine2) if err != nil { t.Fatalf("unexpected error, got %v", err) @@ -428,7 +584,7 @@ func TestControllerFindMachineOwner(t *testing.T) { } // Test #3: Delete the MachineSet and lookup should fail - if err := controller.machineSetInformer.Informer().GetStore().Delete(testResult1); err != nil { + if err := deleteResource(controller.managementClient, controller.machineSetInformer, controller.machineSetResource, testConfig.machineSet); err != nil { t.Fatalf("unexpected error, got %v", err) } testResult3, err := controller.findMachineOwner(testConfig.machines[0].DeepCopy()) @@ -441,7 +597,7 @@ func TestControllerFindMachineOwner(t *testing.T) { } func TestControllerFindMachineByProviderID(t *testing.T) { - testConfig := createMachineSetTestConfig(testNamespace, 1, map[string]string{ + testConfig := createMachineSetTestConfig(RandomString(6), RandomString(6), 1, map[string]string{ nodeGroupMinSizeAnnotationKey: "1", nodeGroupMaxSizeAnnotationKey: "10", }) @@ -473,7 +629,7 @@ func TestControllerFindMachineByProviderID(t *testing.T) { } // Test #2: Verify machine returned by fake provider ID is correct machine - fakeProviderID := fmt.Sprintf("%s$s/%s", testConfig.machines[0].Namespace, testConfig.machines[0].Name) + fakeProviderID := fmt.Sprintf("%s$s/%s", testConfig.machines[0].GetNamespace(), testConfig.machines[0].GetName()) machine, err = controller.findMachineByProviderID(normalizedProviderID(fakeProviderID)) if err != nil { t.Fatalf("unexpected error: %v", err) @@ -485,7 +641,9 @@ func TestControllerFindMachineByProviderID(t *testing.T) { // Test #3: Verify machine is not found if it has a // non-existent or different provider ID. machine = testConfig.machines[0].DeepCopy() - machine.Spec.ProviderID = pointer.StringPtr("does-not-match") + if err := unstructured.SetNestedField(machine.Object, "does-not-match", "spec", "providerID"); err != nil { + t.Fatalf("unexpected error: %v", err) + } if err := controller.machineInformer.Informer().GetStore().Update(machine); err != nil { t.Fatalf("unexpected error updating machine, got %v", err) } @@ -499,7 +657,7 @@ func TestControllerFindMachineByProviderID(t *testing.T) { } func TestControllerFindNodeByNodeName(t *testing.T) { - testConfig := createMachineSetTestConfig(testNamespace, 1, map[string]string{ + testConfig := createMachineSetTestConfig(RandomString(6), RandomString(6), 1, map[string]string{ nodeGroupMinSizeAnnotationKey: "1", nodeGroupMaxSizeAnnotationKey: "10", }) @@ -526,102 +684,151 @@ func TestControllerFindNodeByNodeName(t *testing.T) { } } -func TestControllerMachinesInMachineSet(t *testing.T) { - testConfig1 := createMachineSetTestConfig("testConfig1", 5, map[string]string{ - nodeGroupMinSizeAnnotationKey: "1", - nodeGroupMaxSizeAnnotationKey: "10", - }) +func TestControllerListMachinesForScalableResource(t *testing.T) { + test := func(t *testing.T, testConfig1 *testConfig, testConfig2 *testConfig) { + controller, stop := mustCreateTestController(t, testConfig1) + defer stop() - controller, stop := mustCreateTestController(t, testConfig1) - defer stop() + if err := addTestConfigs(t, controller, testConfig2); err != nil { + t.Fatalf("unexpected error: %v", err) + } - // Construct a second set of objects and add the machines, - // 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("testConfig2", 5, map[string]string{ - nodeGroupMinSizeAnnotationKey: "1", - nodeGroupMaxSizeAnnotationKey: "10", - }) + scalableResource1 := testConfig1.machineSet + if testConfig1.machineDeployment != nil { + scalableResource1 = testConfig1.machineDeployment + } - if err := addTestConfigs(t, controller, testConfig2); err != nil { - t.Fatalf("unexpected error: %v", err) - } + scalableResource2 := testConfig2.machineSet + if testConfig2.machineDeployment != nil { + scalableResource2 = testConfig2.machineDeployment + } - machinesInTestObjs1, err := controller.listMachines(testConfig1.spec.namespace, labels.Everything()) - if err != nil { - t.Fatalf("error listing machines: %v", err) - } + machinesInScalableResource1, err := controller.listMachinesForScalableResource(scalableResource1) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } - machinesInTestObjs2, err := controller.listMachines(testConfig2.spec.namespace, labels.Everything()) - if err != nil { - t.Fatalf("error listing machines: %v", err) - } + machinesInScalableResource2, err := controller.listMachinesForScalableResource(scalableResource2) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } - actual := len(machinesInTestObjs1) + len(machinesInTestObjs2) - expected := len(testConfig1.machines) + len(testConfig2.machines) - if actual != expected { - t.Fatalf("expected %d machines, got %d", expected, actual) - } + actual := len(machinesInScalableResource1) + len(machinesInScalableResource2) + expected := len(testConfig1.machines) + len(testConfig2.machines) + if actual != expected { + t.Fatalf("expected %d machines, got %d", expected, actual) + } - // Sort results as order is not guaranteed. - sort.Slice(machinesInTestObjs1, func(i, j int) bool { - return machinesInTestObjs1[i].Name < machinesInTestObjs1[j].Name - }) - sort.Slice(machinesInTestObjs2, func(i, j int) bool { - return machinesInTestObjs2[i].Name < machinesInTestObjs2[j].Name - }) + // Sort results as order is not guaranteed. + sort.Slice(machinesInScalableResource1, func(i, j int) bool { + return machinesInScalableResource1[i].GetName() < machinesInScalableResource1[j].GetName() + }) + sort.Slice(machinesInScalableResource2, func(i, j int) bool { + return machinesInScalableResource2[i].GetName() < machinesInScalableResource2[j].GetName() + }) - for i, m := range machinesInTestObjs1 { - if m.Name != testConfig1.machines[i].Name { - t.Errorf("expected %q, got %q", testConfig1.machines[i].Name, m.Name) + for i, m := range machinesInScalableResource1 { + if m.GetName() != testConfig1.machines[i].GetName() { + t.Errorf("expected %q, got %q", testConfig1.machines[i].GetName(), m.GetName()) + } + if m.GetNamespace() != testConfig1.machines[i].GetNamespace() { + t.Errorf("expected %q, got %q", testConfig1.machines[i].GetNamespace(), m.GetNamespace()) + } } - if m.Namespace != testConfig1.machines[i].Namespace { - t.Errorf("expected %q, got %q", testConfig1.machines[i].Namespace, m.Namespace) + + for i, m := range machinesInScalableResource2 { + if m.GetName() != testConfig2.machines[i].GetName() { + t.Errorf("expected %q, got %q", testConfig2.machines[i].GetName(), m.GetName()) + } + if m.GetNamespace() != testConfig2.machines[i].GetNamespace() { + t.Errorf("expected %q, got %q", testConfig2.machines[i].GetNamespace(), m.GetNamespace()) + } } - } - for i, m := range machinesInTestObjs2 { - if m.Name != testConfig2.machines[i].Name { - t.Errorf("expected %q, got %q", testConfig2.machines[i].Name, m.Name) + // Finally everything in the respective objects should be equal. + if !reflect.DeepEqual(testConfig1.machines, machinesInScalableResource1) { + t.Fatalf("expected %+v, got %+v", testConfig1.machines, machinesInScalableResource2) } - if m.Namespace != testConfig2.machines[i].Namespace { - t.Errorf("expected %q, got %q", testConfig2.machines[i].Namespace, m.Namespace) + if !reflect.DeepEqual(testConfig2.machines, machinesInScalableResource2) { + t.Fatalf("expected %+v, got %+v", testConfig2.machines, machinesInScalableResource2) } } - // Finally everything in the respective objects should be equal. - if !reflect.DeepEqual(testConfig1.machines, machinesInTestObjs1) { - t.Fatalf("expected %+v, got %+v", testConfig1.machines, machinesInTestObjs1) - } - if !reflect.DeepEqual(testConfig2.machines, machinesInTestObjs2) { - t.Fatalf("expected %+v, got %+v", testConfig2.machines, machinesInTestObjs2) - } + t.Run("MachineSet", func(t *testing.T) { + namespace := RandomString(6) + testConfig1 := createMachineSetTestConfig(namespace, RandomString(6), 5, map[string]string{ + nodeGroupMinSizeAnnotationKey: "1", + nodeGroupMaxSizeAnnotationKey: "10", + }) + + // Construct a second set of objects and add the machines, + // 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{ + nodeGroupMinSizeAnnotationKey: "1", + nodeGroupMaxSizeAnnotationKey: "10", + }) + + test(t, testConfig1, testConfig2) + }) + + t.Run("MachineDeployment", func(t *testing.T) { + namespace := RandomString(6) + testConfig1 := createMachineDeploymentTestConfig(namespace, RandomString(6), 5, map[string]string{ + nodeGroupMinSizeAnnotationKey: "1", + nodeGroupMaxSizeAnnotationKey: "10", + }) + + // Construct a second set of objects and add the machines, + // 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 := createMachineDeploymentTestConfig(namespace, RandomString(6), 5, map[string]string{ + nodeGroupMinSizeAnnotationKey: "1", + nodeGroupMaxSizeAnnotationKey: "10", + }) + + test(t, testConfig1, testConfig2) + }) } func TestControllerLookupNodeGroupForNonExistentNode(t *testing.T) { - testConfig := createMachineSetTestConfig(testNamespace, 1, map[string]string{ - nodeGroupMinSizeAnnotationKey: "1", - nodeGroupMaxSizeAnnotationKey: "10", - }) + test := func(t *testing.T, testConfig *testConfig) { + controller, stop := mustCreateTestController(t, testConfig) + defer stop() - controller, stop := mustCreateTestController(t, testConfig) - defer stop() + node := testConfig.nodes[0].DeepCopy() + node.Spec.ProviderID = "does-not-exist" - node := testConfig.nodes[0].DeepCopy() - node.Spec.ProviderID = "does-not-exist" + ng, err := controller.nodeGroupForNode(node) - ng, err := controller.nodeGroupForNode(node) + // Looking up a node that doesn't exist doesn't generate an + // error. But, equally, the ng should actually be nil. + if err != nil { + t.Fatalf("unexpected error: %v", err) + } - // Looking up a node that doesn't exist doesn't generate an - // error. But, equally, the ng should actually be nil. - if err != nil { - t.Fatalf("unexpected error: %v", err) + if ng != nil { + t.Fatalf("unexpected nodegroup: %v", ng) + } } - if ng != nil { - t.Fatalf("unexpected nodegroup: %v", ng) - } + t.Run("MachineSet", func(t *testing.T) { + testConfig := createMachineSetTestConfig(RandomString(6), RandomString(6), 1, map[string]string{ + nodeGroupMinSizeAnnotationKey: "1", + nodeGroupMaxSizeAnnotationKey: "10", + }) + test(t, testConfig) + }) + + t.Run("MachineDeployment", func(t *testing.T) { + testConfig := createMachineDeploymentTestConfig(RandomString(6), RandomString(6), 1, map[string]string{ + nodeGroupMinSizeAnnotationKey: "1", + nodeGroupMaxSizeAnnotationKey: "10", + }) + test(t, testConfig) + }) } func TestControllerNodeGroupForNodeWithMissingMachineOwner(t *testing.T) { @@ -630,8 +837,9 @@ func TestControllerNodeGroupForNodeWithMissingMachineOwner(t *testing.T) { defer stop() machine := testConfig.machines[0].DeepCopy() - machine.OwnerReferences = []v1.OwnerReference{} - if err := controller.machineInformer.Informer().GetStore().Update(newUnstructuredFromMachine(machine)); err != nil { + machine.SetOwnerReferences([]metav1.OwnerReference{}) + + if err := updateResource(controller.managementClient, controller.machineInformer, controller.machineResource, machine); err != nil { t.Fatalf("unexpected error updating machine, got %v", err) } @@ -646,7 +854,7 @@ func TestControllerNodeGroupForNodeWithMissingMachineOwner(t *testing.T) { } t.Run("MachineSet", func(t *testing.T) { - testConfig := createMachineSetTestConfig(testNamespace, 1, map[string]string{ + testConfig := createMachineSetTestConfig(RandomString(6), RandomString(6), 1, map[string]string{ nodeGroupMinSizeAnnotationKey: "1", nodeGroupMaxSizeAnnotationKey: "10", }) @@ -654,7 +862,7 @@ func TestControllerNodeGroupForNodeWithMissingMachineOwner(t *testing.T) { }) t.Run("MachineDeployment", func(t *testing.T) { - testConfig := createMachineDeploymentTestConfig(testNamespace, 1, map[string]string{ + testConfig := createMachineDeploymentTestConfig(RandomString(6), RandomString(6), 1, map[string]string{ nodeGroupMinSizeAnnotationKey: "1", nodeGroupMaxSizeAnnotationKey: "10", }) @@ -662,6 +870,32 @@ func TestControllerNodeGroupForNodeWithMissingMachineOwner(t *testing.T) { }) } +func TestControllerNodeGroupForNodeWithMissingSetMachineOwner(t *testing.T) { + testConfig := createMachineDeploymentTestConfig(RandomString(6), RandomString(6), 1, map[string]string{ + nodeGroupMinSizeAnnotationKey: "1", + nodeGroupMaxSizeAnnotationKey: "10", + }) + + controller, stop := mustCreateTestController(t, testConfig) + defer stop() + + machineSet := testConfig.machineSet.DeepCopy() + machineSet.SetOwnerReferences([]metav1.OwnerReference{}) + + if err := updateResource(controller.managementClient, controller.machineSetInformer, controller.machineSetResource, machineSet); err != nil { + t.Fatalf("unexpected error updating machine, got %v", err) + } + + ng, err := controller.nodeGroupForNode(testConfig.nodes[0]) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + if ng != nil { + t.Fatalf("unexpected nodegroup: %v", ng) + } +} + func TestControllerNodeGroupForNodeWithPositiveScalingBounds(t *testing.T) { test := func(t *testing.T, testConfig *testConfig) { controller, stop := mustCreateTestController(t, testConfig) @@ -679,7 +913,7 @@ func TestControllerNodeGroupForNodeWithPositiveScalingBounds(t *testing.T) { } t.Run("MachineSet", func(t *testing.T) { - testConfig := createMachineSetTestConfig(testNamespace, 1, map[string]string{ + testConfig := createMachineSetTestConfig(RandomString(6), RandomString(6), 1, map[string]string{ nodeGroupMinSizeAnnotationKey: "1", nodeGroupMaxSizeAnnotationKey: "1", }) @@ -687,7 +921,7 @@ func TestControllerNodeGroupForNodeWithPositiveScalingBounds(t *testing.T) { }) t.Run("MachineDeployment", func(t *testing.T) { - testConfig := createMachineDeploymentTestConfig(testNamespace, 1, map[string]string{ + testConfig := createMachineDeploymentTestConfig(RandomString(6), RandomString(6), 1, map[string]string{ nodeGroupMinSizeAnnotationKey: "1", nodeGroupMaxSizeAnnotationKey: "1", }) @@ -715,18 +949,20 @@ func TestControllerNodeGroups(t *testing.T) { controller, stop := mustCreateTestController(t) defer stop() + namespace := RandomString(6) + // Test #1: zero nodegroups assertNodegroupLen(t, controller, 0) // Test #2: add 5 machineset-based nodegroups - machineSetConfigs := createMachineSetTestConfigs("MachineSet", 5, 1, annotations) + machineSetConfigs := createMachineSetTestConfigs(namespace, 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("MachineDeployment", 2, 1, annotations) + machineDeploymentConfigs := createMachineDeploymentTestConfigs(namespace, RandomString(6), 2, 1, annotations) if err := addTestConfigs(t, controller, machineDeploymentConfigs...); err != nil { t.Fatalf("unexpected error: %v", err) } @@ -750,14 +986,14 @@ func TestControllerNodeGroups(t *testing.T) { } // Test #5: machineset with no scaling bounds results in no nodegroups - machineSetConfigs = createMachineSetTestConfigs("MachineSet", 5, 1, annotations) + machineSetConfigs = createMachineSetTestConfigs(namespace, 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("MachineDeployment", 2, 1, annotations) + machineDeploymentConfigs = createMachineDeploymentTestConfigs(namespace, RandomString(6), 2, 1, annotations) if err := addTestConfigs(t, controller, machineDeploymentConfigs...); err != nil { t.Fatalf("unexpected error: %v", err) } @@ -769,7 +1005,7 @@ func TestControllerNodeGroups(t *testing.T) { } // Test #7: machineset with bad scaling bounds results in an error and no nodegroups - machineSetConfigs = createMachineSetTestConfigs("MachineSet", 5, 1, annotations) + machineSetConfigs = createMachineSetTestConfigs(namespace, RandomString(6), 5, 1, annotations) if err := addTestConfigs(t, controller, machineSetConfigs...); err != nil { t.Fatalf("unexpected error: %v", err) } @@ -778,7 +1014,7 @@ func TestControllerNodeGroups(t *testing.T) { } // Test #8: machinedeployment with bad scaling bounds results in an error and no nodegroups - machineDeploymentConfigs = createMachineDeploymentTestConfigs("MachineDeployment", 2, 1, annotations) + machineDeploymentConfigs = createMachineDeploymentTestConfigs(namespace, RandomString(6), 2, 1, annotations) if err := addTestConfigs(t, controller, machineDeploymentConfigs...); err != nil { t.Fatalf("unexpected error: %v", err) } @@ -842,19 +1078,19 @@ func TestControllerNodeGroupsNodeCount(t *testing.T) { t.Run("MachineSet", func(t *testing.T) { for _, tc := range testCases { - test(t, tc, createMachineSetTestConfigs(testNamespace, tc.nodeGroups, tc.nodesPerGroup, annotations)) + test(t, tc, createMachineSetTestConfigs(RandomString(6), RandomString(6), tc.nodeGroups, tc.nodesPerGroup, annotations)) } }) t.Run("MachineDeployment", func(t *testing.T) { for _, tc := range testCases { - test(t, tc, createMachineDeploymentTestConfigs(testNamespace, tc.nodeGroups, tc.nodesPerGroup, annotations)) + test(t, tc, createMachineDeploymentTestConfigs(RandomString(6), RandomString(6), tc.nodeGroups, tc.nodesPerGroup, annotations)) } }) } func TestControllerFindMachineFromNodeAnnotation(t *testing.T) { - testConfig := createMachineSetTestConfig(testNamespace, 1, map[string]string{ + testConfig := createMachineSetTestConfig(RandomString(6), RandomString(6), 1, map[string]string{ nodeGroupMinSizeAnnotationKey: "1", nodeGroupMaxSizeAnnotationKey: "10", }) @@ -866,8 +1102,9 @@ func TestControllerFindMachineFromNodeAnnotation(t *testing.T) { // want to force findMachineByProviderID() to fallback to // searching using the annotation on the node object. for _, machine := range testConfig.machines { - machine.Spec.ProviderID = nil - if err := controller.machineInformer.Informer().GetStore().Update(newUnstructuredFromMachine(machine)); err != nil { + unstructured.RemoveNestedField(machine.Object, "spec", "providerID") + + if err := updateResource(controller.managementClient, controller.machineInformer, controller.machineResource, machine); err != nil { t.Fatalf("unexpected error updating machine, got %v", err) } } @@ -901,7 +1138,7 @@ func TestControllerFindMachineFromNodeAnnotation(t *testing.T) { } func TestControllerMachineSetNodeNamesWithoutLinkage(t *testing.T) { - testConfig := createMachineSetTestConfig(testNamespace, 3, map[string]string{ + testConfig := createMachineSetTestConfig(RandomString(6), RandomString(6), 3, map[string]string{ nodeGroupMinSizeAnnotationKey: "1", nodeGroupMaxSizeAnnotationKey: "10", }) @@ -910,15 +1147,13 @@ func TestControllerMachineSetNodeNamesWithoutLinkage(t *testing.T) { defer stop() // Remove all linkage between node and machine. - for _, machine := range testConfig.machines { - machine.Spec.ProviderID = nil - if err := controller.machineInformer.Informer().GetStore().Update(newUnstructuredFromMachine(machine)); err != nil { - t.Fatalf("unexpected error updating machine, got %v", err) - } - } - for _, machine := range testConfig.machines { - machine.Status.NodeRef = nil - if err := controller.machineInformer.Informer().GetStore().Update(newUnstructuredFromMachine(machine)); err != nil { + for i := range testConfig.machines { + machine := testConfig.machines[i].DeepCopy() + + unstructured.RemoveNestedField(machine.Object, "spec", "providerID") + unstructured.RemoveNestedField(machine.Object, "status", "nodeRef") + + if err := updateResource(controller.managementClient, controller.machineInformer, controller.machineResource, machine); err != nil { t.Fatalf("unexpected error updating machine, got %v", err) } } @@ -945,7 +1180,7 @@ func TestControllerMachineSetNodeNamesWithoutLinkage(t *testing.T) { } func TestControllerMachineSetNodeNamesUsingProviderID(t *testing.T) { - testConfig := createMachineSetTestConfig(testNamespace, 3, map[string]string{ + testConfig := createMachineSetTestConfig(RandomString(6), RandomString(6), 3, map[string]string{ nodeGroupMinSizeAnnotationKey: "1", nodeGroupMaxSizeAnnotationKey: "10", }) @@ -956,9 +1191,12 @@ func TestControllerMachineSetNodeNamesUsingProviderID(t *testing.T) { // Remove Status.NodeRef.Name on all the machines. We want to // force machineSetNodeNames() to only consider the provider // ID for lookups. - for _, machine := range testConfig.machines { - machine.Status.NodeRef = nil - if err := controller.machineInformer.Informer().GetStore().Update(newUnstructuredFromMachine(machine)); err != nil { + for i := range testConfig.machines { + machine := testConfig.machines[i].DeepCopy() + + unstructured.RemoveNestedField(machine.Object, "status", "nodeRef") + + if err := updateResource(controller.managementClient, controller.machineInformer, controller.machineResource, machine); err != nil { t.Fatalf("unexpected error updating machine, got %v", err) } } @@ -994,7 +1232,7 @@ func TestControllerMachineSetNodeNamesUsingProviderID(t *testing.T) { } func TestControllerMachineSetNodeNamesUsingStatusNodeRefName(t *testing.T) { - testConfig := createMachineSetTestConfig(testNamespace, 3, map[string]string{ + testConfig := createMachineSetTestConfig(RandomString(6), RandomString(6), 3, map[string]string{ nodeGroupMinSizeAnnotationKey: "1", nodeGroupMaxSizeAnnotationKey: "10", }) @@ -1005,9 +1243,12 @@ func TestControllerMachineSetNodeNamesUsingStatusNodeRefName(t *testing.T) { // Remove all the provider ID values on all the machines. We // want to force machineSetNodeNames() to fallback to // searching using Status.NodeRef.Name. - for _, machine := range testConfig.machines { - machine.Spec.ProviderID = nil - if err := controller.machineInformer.Informer().GetStore().Update(newUnstructuredFromMachine(machine)); err != nil { + for i := range testConfig.machines { + machine := testConfig.machines[i].DeepCopy() + + unstructured.RemoveNestedField(machine.Object, "spec", "providerID") + + if err := updateResource(controller.managementClient, controller.machineInformer, controller.machineResource, machine); err != nil { t.Fatalf("unexpected error updating machine, got %v", err) } } @@ -1062,7 +1303,7 @@ func TestControllerGetAPIVersionGroup(t *testing.T) { } func TestControllerGetAPIVersionGroupWithMachineDeployments(t *testing.T) { - testConfig := createMachineDeploymentTestConfig(testNamespace, 1, map[string]string{ + testConfig := createMachineDeploymentTestConfig(RandomString(6), RandomString(6), 1, map[string]string{ nodeGroupMinSizeAnnotationKey: "1", nodeGroupMaxSizeAnnotationKey: "1", }) @@ -1070,35 +1311,43 @@ func TestControllerGetAPIVersionGroupWithMachineDeployments(t *testing.T) { t.Fatalf("unexpected error: %v", err) } - testConfig.machineDeployment.TypeMeta.APIVersion = fmt.Sprintf("%s/v1beta1", customCAPIGroup) - testConfig.machineSet.TypeMeta.APIVersion = fmt.Sprintf("%s/v1beta1", customCAPIGroup) + testConfig.machineDeployment.SetAPIVersion(fmt.Sprintf("%s/v1beta1", customCAPIGroup)) + testConfig.machineSet.SetAPIVersion(fmt.Sprintf("%s/v1beta1", customCAPIGroup)) + for _, machine := range testConfig.machines { - machine.TypeMeta.APIVersion = fmt.Sprintf("%s/v1beta1", customCAPIGroup) + machine.SetAPIVersion(fmt.Sprintf("%s/v1beta1", customCAPIGroup)) } + controller, stop := mustCreateTestController(t, testConfig) defer stop() - machineDeployments, err := controller.listMachineDeployments(testNamespace, labels.Everything()) + machineDeployments, err := controller.managementClient.Resource(controller.machineDeploymentResource).Namespace(testConfig.spec.namespace). + List(context.TODO(), metav1.ListOptions{}) if err != nil { t.Fatalf("unexpected error: %v", err) } - if l := len(machineDeployments); l != 1 { + + if l := len(machineDeployments.Items); l != 1 { t.Fatalf("Incorrect number of MachineDeployments, expected 1, got %d", l) } - machineSets, err := controller.listMachineSets(testNamespace, labels.Everything()) + machineSets, err := controller.managementClient.Resource(controller.machineSetResource).Namespace(testConfig.spec.namespace). + List(context.TODO(), metav1.ListOptions{}) if err != nil { t.Fatalf("unexpected error: %v", err) } - if l := len(machineSets); l != 1 { - t.Fatalf("Incorrect number of MachineSets, expected 1, got %d", l) + + if l := len(machineSets.Items); l != 1 { + t.Fatalf("Incorrect number of MachineDeployments, expected 1, got %d", l) } - machines, err := controller.listMachines(testNamespace, labels.Everything()) + machines, err := controller.managementClient.Resource(controller.machineResource).Namespace(testConfig.spec.namespace). + List(context.TODO(), metav1.ListOptions{}) if err != nil { t.Fatalf("unexpected error: %v", err) } - if l := len(machines); l != 1 { + + if l := len(machines.Items); l != 1 { t.Fatalf("Incorrect number of Machines, expected 1, got %d", l) } @@ -1136,7 +1385,7 @@ func TestGetAPIGroupPreferredVersion(t *testing.T) { discoveryClient := &fakediscovery.FakeDiscovery{ Fake: &clientgotesting.Fake{ - Resources: []*v1.APIResourceList{ + Resources: []*metav1.APIResourceList{ { GroupVersion: fmt.Sprintf("%s/v1beta1", customCAPIGroup), }, @@ -1192,10 +1441,10 @@ func TestGroupVersionHasResource(t *testing.T) { discoveryClient := &fakediscovery.FakeDiscovery{ Fake: &clientgotesting.Fake{ - Resources: []*v1.APIResourceList{ + Resources: []*metav1.APIResourceList{ { GroupVersion: fmt.Sprintf("%s/v1alpha3", defaultCAPIGroup), - APIResources: []v1.APIResource{ + APIResources: []metav1.APIResource{ { Name: resourceNameMachineDeployment, }, @@ -1276,3 +1525,16 @@ func TestMachineKeyFromFailedProviderID(t *testing.T) { }) } } + +const CharSet = "0123456789abcdefghijklmnopqrstuvwxyz" + +var rnd = rand.New(rand.NewSource(time.Now().UnixNano())) + +// RandomString returns a random alphanumeric string. +func RandomString(n int) string { + result := make([]byte, n) + for i := range result { + result[i] = CharSet[rnd.Intn(len(CharSet))] + } + return string(result) +} diff --git a/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_converters.go b/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_converters.go deleted file mode 100644 index 0b90285280f8..000000000000 --- a/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_converters.go +++ /dev/null @@ -1,196 +0,0 @@ -/* -Copyright 2020 The Kubernetes Authors. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package clusterapi - -import ( - corev1 "k8s.io/api/core/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" - "k8s.io/utils/pointer" -) - -func newMachineDeploymentFromUnstructured(u *unstructured.Unstructured) *MachineDeployment { - machineDeployment := MachineDeployment{ - TypeMeta: metav1.TypeMeta{ - Kind: u.GetKind(), - APIVersion: u.GetAPIVersion(), - }, - ObjectMeta: metav1.ObjectMeta{ - Name: u.GetName(), - Namespace: u.GetNamespace(), - UID: u.GetUID(), - Labels: u.GetLabels(), - Annotations: u.GetAnnotations(), - OwnerReferences: u.GetOwnerReferences(), - DeletionTimestamp: u.GetDeletionTimestamp(), - }, - Spec: MachineDeploymentSpec{}, - Status: MachineDeploymentStatus{}, - } - - replicas, found, err := unstructured.NestedInt64(u.Object, "spec", "replicas") - if err == nil && found { - machineDeployment.Spec.Replicas = pointer.Int32Ptr(int32(replicas)) - } - - return &machineDeployment -} - -func newMachineSetFromUnstructured(u *unstructured.Unstructured) *MachineSet { - machineSet := MachineSet{ - TypeMeta: metav1.TypeMeta{ - Kind: u.GetKind(), - APIVersion: u.GetAPIVersion(), - }, - ObjectMeta: metav1.ObjectMeta{ - Name: u.GetName(), - Namespace: u.GetNamespace(), - UID: u.GetUID(), - Labels: u.GetLabels(), - Annotations: u.GetAnnotations(), - OwnerReferences: u.GetOwnerReferences(), - DeletionTimestamp: u.GetDeletionTimestamp(), - }, - Spec: MachineSetSpec{}, - Status: MachineSetStatus{}, - } - - replicas, found, err := unstructured.NestedInt64(u.Object, "spec", "replicas") - if err == nil && found { - machineSet.Spec.Replicas = pointer.Int32Ptr(int32(replicas)) - } - - return &machineSet -} - -func newMachineFromUnstructured(u *unstructured.Unstructured) *Machine { - machine := Machine{ - TypeMeta: metav1.TypeMeta{ - Kind: u.GetKind(), - APIVersion: u.GetAPIVersion(), - }, - ObjectMeta: metav1.ObjectMeta{ - Name: u.GetName(), - Namespace: u.GetNamespace(), - UID: u.GetUID(), - Labels: u.GetLabels(), - Annotations: u.GetAnnotations(), - OwnerReferences: u.GetOwnerReferences(), - ClusterName: u.GetClusterName(), - DeletionTimestamp: u.GetDeletionTimestamp(), - }, - Spec: MachineSpec{}, - Status: MachineStatus{}, - } - - if providerID, _, _ := unstructured.NestedString(u.Object, "spec", "providerID"); providerID != "" { - machine.Spec.ProviderID = pointer.StringPtr(providerID) - } - - nodeRef := corev1.ObjectReference{} - - if nodeRefKind, _, _ := unstructured.NestedString(u.Object, "status", "nodeRef", "kind"); nodeRefKind != "" { - nodeRef.Kind = nodeRefKind - } - - if nodeRefName, _, _ := unstructured.NestedString(u.Object, "status", "nodeRef", "name"); nodeRefName != "" { - nodeRef.Name = nodeRefName - } - - if nodeRef.Name != "" || nodeRef.Kind != "" { - machine.Status.NodeRef = &nodeRef - } - - if failureMessage, _, _ := unstructured.NestedString(u.Object, "status", "failureMessage"); failureMessage != "" { - machine.Status.FailureMessage = pointer.StringPtr(failureMessage) - } - - return &machine -} - -func newUnstructuredFromMachineSet(m *MachineSet) *unstructured.Unstructured { - u := unstructured.Unstructured{} - - u.SetAPIVersion(m.APIVersion) - u.SetAnnotations(m.Annotations) - u.SetKind(m.Kind) - u.SetLabels(m.Labels) - u.SetName(m.Name) - u.SetNamespace(m.Namespace) - u.SetOwnerReferences(m.OwnerReferences) - u.SetUID(m.UID) - u.SetDeletionTimestamp(m.DeletionTimestamp) - - if m.Spec.Replicas != nil { - unstructured.SetNestedField(u.Object, int64(*m.Spec.Replicas), "spec", "replicas") - } - - return &u -} - -func newUnstructuredFromMachineDeployment(m *MachineDeployment) *unstructured.Unstructured { - u := unstructured.Unstructured{} - - u.SetAPIVersion(m.APIVersion) - u.SetAnnotations(m.Annotations) - u.SetKind(m.Kind) - u.SetLabels(m.Labels) - u.SetName(m.Name) - u.SetNamespace(m.Namespace) - u.SetOwnerReferences(m.OwnerReferences) - u.SetUID(m.UID) - u.SetDeletionTimestamp(m.DeletionTimestamp) - - if m.Spec.Replicas != nil { - unstructured.SetNestedField(u.Object, int64(*m.Spec.Replicas), "spec", "replicas") - } - - return &u -} - -func newUnstructuredFromMachine(m *Machine) *unstructured.Unstructured { - u := unstructured.Unstructured{} - - u.SetAPIVersion(m.APIVersion) - u.SetAnnotations(m.Annotations) - u.SetKind(m.Kind) - u.SetLabels(m.Labels) - u.SetName(m.Name) - u.SetNamespace(m.Namespace) - u.SetOwnerReferences(m.OwnerReferences) - u.SetUID(m.UID) - u.SetDeletionTimestamp(m.DeletionTimestamp) - - if m.Spec.ProviderID != nil && *m.Spec.ProviderID != "" { - unstructured.SetNestedField(u.Object, *m.Spec.ProviderID, "spec", "providerID") - } - - if m.Status.NodeRef != nil { - if m.Status.NodeRef.Kind != "" { - unstructured.SetNestedField(u.Object, m.Status.NodeRef.Kind, "status", "nodeRef", "kind") - } - if m.Status.NodeRef.Name != "" { - unstructured.SetNestedField(u.Object, m.Status.NodeRef.Name, "status", "nodeRef", "name") - } - } - - if m.Status.FailureMessage != nil { - unstructured.SetNestedField(u.Object, *m.Status.FailureMessage, "status", "failureMessage") - } - - return &u -} diff --git a/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_machinedeployment.go b/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_machinedeployment.go deleted file mode 100644 index 7ce675cc801d..000000000000 --- a/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_machinedeployment.go +++ /dev/null @@ -1,152 +0,0 @@ -/* -Copyright 2020 The Kubernetes Authors. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package clusterapi - -import ( - "context" - "fmt" - "path" - "time" - - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" - klog "k8s.io/klog/v2" - "k8s.io/utils/pointer" -) - -type machineDeploymentScalableResource struct { - controller *machineController - machineDeployment *MachineDeployment - maxSize int - minSize int -} - -var _ scalableResource = (*machineDeploymentScalableResource)(nil) - -func (r machineDeploymentScalableResource) ID() string { - return path.Join(r.Namespace(), r.Name()) -} - -func (r machineDeploymentScalableResource) MaxSize() int { - return r.maxSize -} - -func (r machineDeploymentScalableResource) MinSize() int { - return r.minSize -} - -func (r machineDeploymentScalableResource) Name() string { - return r.machineDeployment.Name -} - -func (r machineDeploymentScalableResource) Namespace() string { - return r.machineDeployment.Namespace -} - -func (r machineDeploymentScalableResource) Nodes() ([]string, error) { - var result []string - - if err := r.controller.filterAllMachineSets(func(machineSet *MachineSet) error { - if machineSetIsOwnedByMachineDeployment(machineSet, r.machineDeployment) { - providerIDs, err := r.controller.machineSetProviderIDs(machineSet) - if err != nil { - return err - } - result = append(result, providerIDs...) - } - return nil - }); err != nil { - return nil, err - } - - return result, nil -} - -func (r machineDeploymentScalableResource) Replicas() (int32, error) { - freshMachineDeployment, err := r.controller.getMachineDeployment(r.machineDeployment.Namespace, r.machineDeployment.Name, metav1.GetOptions{}) - if err != nil { - return 0, err - } - - if freshMachineDeployment == nil { - return 0, fmt.Errorf("unknown machineDeployment %s", r.machineDeployment.Name) - } - - if freshMachineDeployment.Spec.Replicas == nil { - klog.Warningf("MachineDeployment %q has nil spec.replicas. This is unsupported behaviour. Falling back to status.replicas.", r.machineDeployment.Name) - } - // If no value for replicas on the MachineSet spec, fallback to the status - // TODO: Remove this fallback once defaulting is implemented for MachineSet Replicas - return pointer.Int32PtrDerefOr(freshMachineDeployment.Spec.Replicas, freshMachineDeployment.Status.Replicas), nil -} - -func (r machineDeploymentScalableResource) SetSize(nreplicas int32) error { - u, err := r.controller.dynamicclient.Resource(*r.controller.machineDeploymentResource).Namespace(r.machineDeployment.Namespace).Get(context.TODO(), r.machineDeployment.Name, metav1.GetOptions{}) - - if err != nil { - return err - } - - if u == nil { - return fmt.Errorf("unknown machineDeployment %s", r.machineDeployment.Name) - } - - u = u.DeepCopy() - if err := unstructured.SetNestedField(u.Object, int64(nreplicas), "spec", "replicas"); err != nil { - return fmt.Errorf("failed to set replica value: %v", err) - } - - _, updateErr := r.controller.dynamicclient.Resource(*r.controller.machineDeploymentResource).Namespace(u.GetNamespace()).Update(context.TODO(), u, metav1.UpdateOptions{}) - return updateErr -} - -func (r machineDeploymentScalableResource) UnmarkMachineForDeletion(machine *Machine) error { - return unmarkMachineForDeletion(r.controller, machine) -} - -func (r machineDeploymentScalableResource) MarkMachineForDeletion(machine *Machine) error { - u, err := r.controller.dynamicclient.Resource(*r.controller.machineResource).Namespace(machine.Namespace).Get(context.TODO(), machine.Name, metav1.GetOptions{}) - if err != nil { - return err - } - - u = u.DeepCopy() - - annotations := u.GetAnnotations() - if annotations == nil { - annotations = map[string]string{} - } - annotations[machineDeleteAnnotationKey] = time.Now().String() - u.SetAnnotations(annotations) - - _, updateErr := r.controller.dynamicclient.Resource(*r.controller.machineResource).Namespace(u.GetNamespace()).Update(context.TODO(), u, metav1.UpdateOptions{}) - return updateErr -} - -func newMachineDeploymentScalableResource(controller *machineController, machineDeployment *MachineDeployment) (*machineDeploymentScalableResource, error) { - minSize, maxSize, err := parseScalingBounds(machineDeployment.Annotations) - if err != nil { - return nil, fmt.Errorf("error validating min/max annotations: %v", err) - } - - return &machineDeploymentScalableResource{ - controller: controller, - machineDeployment: machineDeployment, - maxSize: maxSize, - minSize: minSize, - }, nil -} diff --git a/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_machineset.go b/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_machineset.go deleted file mode 100644 index 5db264e287b8..000000000000 --- a/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_machineset.go +++ /dev/null @@ -1,138 +0,0 @@ -/* -Copyright 2020 The Kubernetes Authors. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package clusterapi - -import ( - "context" - "fmt" - "path" - "time" - - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" - klog "k8s.io/klog/v2" - "k8s.io/utils/pointer" -) - -type machineSetScalableResource struct { - controller *machineController - machineSet *MachineSet - maxSize int - minSize int -} - -var _ scalableResource = (*machineSetScalableResource)(nil) - -func (r machineSetScalableResource) ID() string { - return path.Join(r.Namespace(), r.Name()) -} - -func (r machineSetScalableResource) MaxSize() int { - return r.maxSize -} - -func (r machineSetScalableResource) MinSize() int { - return r.minSize -} - -func (r machineSetScalableResource) Name() string { - return r.machineSet.Name -} - -func (r machineSetScalableResource) Namespace() string { - return r.machineSet.Namespace -} - -func (r machineSetScalableResource) Nodes() ([]string, error) { - return r.controller.machineSetProviderIDs(r.machineSet) -} - -func (r machineSetScalableResource) Replicas() (int32, error) { - freshMachineSet, err := r.controller.getMachineSet(r.machineSet.Namespace, r.machineSet.Name, metav1.GetOptions{}) - if err != nil { - return 0, err - } - - if freshMachineSet == nil { - return 0, fmt.Errorf("unknown machineSet %s", r.machineSet.Name) - } - - if freshMachineSet.Spec.Replicas == nil { - klog.Warningf("MachineSet %q has nil spec.replicas. This is unsupported behaviour. Falling back to status.replicas.", r.machineSet.Name) - } - - // If no value for replicas on the MachineSet spec, fallback to the status - // TODO: Remove this fallback once defaulting is implemented for MachineSet Replicas - return pointer.Int32PtrDerefOr(freshMachineSet.Spec.Replicas, freshMachineSet.Status.Replicas), nil -} - -func (r machineSetScalableResource) SetSize(nreplicas int32) error { - u, err := r.controller.dynamicclient.Resource(*r.controller.machineSetResource).Namespace(r.machineSet.Namespace).Get(context.TODO(), r.machineSet.Name, metav1.GetOptions{}) - - if err != nil { - return err - } - - if u == nil { - return fmt.Errorf("unknown machineSet %s", r.machineSet.Name) - } - - u = u.DeepCopy() - if err := unstructured.SetNestedField(u.Object, int64(nreplicas), "spec", "replicas"); err != nil { - return fmt.Errorf("failed to set replica value: %v", err) - } - - _, updateErr := r.controller.dynamicclient.Resource(*r.controller.machineSetResource).Namespace(u.GetNamespace()).Update(context.TODO(), u, metav1.UpdateOptions{}) - return updateErr -} - -func (r machineSetScalableResource) MarkMachineForDeletion(machine *Machine) error { - u, err := r.controller.dynamicclient.Resource(*r.controller.machineResource).Namespace(machine.Namespace).Get(context.TODO(), machine.Name, metav1.GetOptions{}) - if err != nil { - return err - } - - u = u.DeepCopy() - - annotations := u.GetAnnotations() - if annotations == nil { - annotations = map[string]string{} - } - annotations[machineDeleteAnnotationKey] = time.Now().String() - u.SetAnnotations(annotations) - - _, updateErr := r.controller.dynamicclient.Resource(*r.controller.machineResource).Namespace(u.GetNamespace()).Update(context.TODO(), u, metav1.UpdateOptions{}) - return updateErr -} - -func (r machineSetScalableResource) UnmarkMachineForDeletion(machine *Machine) error { - return unmarkMachineForDeletion(r.controller, machine) -} - -func newMachineSetScalableResource(controller *machineController, machineSet *MachineSet) (*machineSetScalableResource, error) { - minSize, maxSize, err := parseScalingBounds(machineSet.Annotations) - if err != nil { - return nil, fmt.Errorf("error validating min/max annotations: %v", err) - } - - return &machineSetScalableResource{ - controller: controller, - machineSet: machineSet, - maxSize: maxSize, - minSize: minSize, - }, nil -} diff --git a/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_machineset_test.go b/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_machineset_test.go deleted file mode 100644 index 92930ee01c6d..000000000000 --- a/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_machineset_test.go +++ /dev/null @@ -1,150 +0,0 @@ -/* -Copyright 2020 The Kubernetes Authors. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package clusterapi - -import ( - "context" - "testing" - - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" -) - -func TestSetSize(t *testing.T) { - initialReplicas := int32(1) - updatedReplicas := int32(5) - - testConfig := createMachineSetTestConfig(testNamespace, int(initialReplicas), nil) - controller, stop := mustCreateTestController(t, testConfig) - defer stop() - - sr, err := newMachineSetScalableResource(controller, testConfig.machineSet) - if err != nil { - t.Fatal(err) - } - - err = sr.SetSize(updatedReplicas) - if err != nil { - t.Fatal(err) - } - - // fetch machineSet - u, err := sr.controller.dynamicclient.Resource(*sr.controller.machineSetResource).Namespace(testConfig.machineSet.Namespace). - Get(context.TODO(), testConfig.machineSet.Name, metav1.GetOptions{}) - if err != nil { - t.Fatal(err) - } - - replicas, found, err := unstructured.NestedInt64(u.Object, "spec", "replicas") - if err != nil { - t.Fatal(err) - } - if !found { - t.Fatal("spec.replicas not found") - } - - got := int32(replicas) - if got != updatedReplicas { - t.Errorf("expected %v, got: %v", updatedReplicas, got) - } -} - -func TestReplicas(t *testing.T) { - initialReplicas := int32(1) - updatedReplicas := int32(5) - - testConfig := createMachineSetTestConfig(testNamespace, int(initialReplicas), nil) - controller, stop := mustCreateTestController(t, testConfig) - defer stop() - - sr, err := newMachineSetScalableResource(controller, testConfig.machineSet) - if err != nil { - t.Fatal(err) - } - - i, err := sr.Replicas() - if err != nil { - t.Fatal(err) - } - - if i != initialReplicas { - t.Errorf("expected %v, got: %v", initialReplicas, i) - } - - // fetch and update machineSet - u, err := sr.controller.dynamicclient.Resource(*sr.controller.machineSetResource).Namespace(testConfig.machineSet.Namespace). - Get(context.TODO(), testConfig.machineSet.Name, metav1.GetOptions{}) - if err != nil { - t.Fatal(err) - } - - if err := unstructured.SetNestedField(u.Object, int64(updatedReplicas), "spec", "replicas"); err != nil { - t.Fatal(err) - } - - _, err = sr.controller.dynamicclient.Resource(*sr.controller.machineSetResource).Namespace(u.GetNamespace()). - Update(context.TODO(), u, metav1.UpdateOptions{}) - if err != nil { - t.Fatal(err) - } - - i, err = sr.Replicas() - if err != nil { - t.Fatal(err) - } - - if i != updatedReplicas { - t.Errorf("expected %v, got: %v", updatedReplicas, i) - } -} - -func TestSetSizeAndReplicas(t *testing.T) { - initialReplicas := int32(1) - updatedReplicas := int32(5) - - testConfig := createMachineSetTestConfig(testNamespace, int(initialReplicas), nil) - controller, stop := mustCreateTestController(t, testConfig) - defer stop() - - sr, err := newMachineSetScalableResource(controller, testConfig.machineSet) - if err != nil { - t.Fatal(err) - } - - i, err := sr.Replicas() - if err != nil { - t.Fatal(err) - } - - if i != initialReplicas { - t.Errorf("expected %v, got: %v", initialReplicas, i) - } - - err = sr.SetSize(updatedReplicas) - if err != nil { - t.Fatal(err) - } - - i, err = sr.Replicas() - if err != nil { - t.Fatal(err) - } - - if i != updatedReplicas { - t.Errorf("expected %v, got: %v", updatedReplicas, i) - } -} diff --git a/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_nodegroup.go b/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_nodegroup.go index 49cf372040e7..2632aba5212f 100644 --- a/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_nodegroup.go +++ b/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_nodegroup.go @@ -20,8 +20,10 @@ import ( "fmt" corev1 "k8s.io/api/core/v1" - "k8s.io/autoscaler/cluster-autoscaler/cloudprovider" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" schedulerframework "k8s.io/kubernetes/pkg/scheduler/framework/v1alpha1" + + "k8s.io/autoscaler/cluster-autoscaler/cloudprovider" ) const ( @@ -32,19 +34,11 @@ const ( type nodegroup struct { machineController *machineController - scalableResource scalableResource + scalableResource *unstructuredScalableResource } var _ cloudprovider.NodeGroup = (*nodegroup)(nil) -func (ng *nodegroup) Name() string { - return ng.scalableResource.Name() -} - -func (ng *nodegroup) Namespace() string { - return ng.scalableResource.Namespace() -} - func (ng *nodegroup) MinSize() int { return ng.scalableResource.MinSize() } @@ -147,9 +141,6 @@ func (ng *nodegroup) DeleteNodes(nodes []*corev1.Node) error { continue } - if machine.Annotations == nil { - machine.Annotations = map[string]string{} - } nodeGroup, err := ng.machineController.nodeGroupForNode(node) if err != nil { return err @@ -160,7 +151,7 @@ func (ng *nodegroup) DeleteNodes(nodes []*corev1.Node) error { } if err := ng.scalableResource.SetSize(replicas - 1); err != nil { - nodeGroup.scalableResource.UnmarkMachineForDeletion(machine) + _ = nodeGroup.scalableResource.UnmarkMachineForDeletion(machine) return err } @@ -216,7 +207,7 @@ func (ng *nodegroup) Debug() string { // Nodes returns a list of all nodes that belong to this node group. // This includes instances that might have not become a kubernetes node yet. func (ng *nodegroup) Nodes() ([]cloudprovider.Instance, error) { - nodes, err := ng.scalableResource.Nodes() + providerIDs, err := ng.scalableResource.ProviderIDs() if err != nil { return nil, err } @@ -225,10 +216,10 @@ func (ng *nodegroup) Nodes() ([]cloudprovider.Instance, error) { // The IDs returned here are used to check if a node is registered or not and // must match the ID on the Node object itself. // https://github.com/kubernetes/autoscaler/blob/a973259f1852303ba38a3a61eeee8489cf4e1b13/cluster-autoscaler/clusterstate/clusterstate.go#L967-L985 - instances := make([]cloudprovider.Instance, len(nodes)) - for i := range nodes { + instances := make([]cloudprovider.Instance, len(providerIDs)) + for i := range providerIDs { instances[i] = cloudprovider.Instance{ - Id: nodes[i], + Id: providerIDs[i], } } @@ -257,7 +248,10 @@ func (ng *nodegroup) Exist() bool { // Create creates the node group on the cloud nodegroup side. // Implementation optional. func (ng *nodegroup) Create() (cloudprovider.NodeGroup, error) { - return nil, cloudprovider.ErrAlreadyExist + if ng.Exist() { + return nil, cloudprovider.ErrAlreadyExist + } + return nil, cloudprovider.ErrNotImplemented } // Delete deletes the node group on the cloud nodegroup side. This will @@ -274,22 +268,12 @@ func (ng *nodegroup) Autoprovisioned() bool { return false } -func newNodegroupFromMachineSet(controller *machineController, machineSet *MachineSet) (*nodegroup, error) { - scalableResource, err := newMachineSetScalableResource(controller, machineSet) +func newNodegroupFromScalableResource(controller *machineController, unstructuredScalableResource *unstructured.Unstructured) (*nodegroup, error) { + scalableResource, err := newUnstructuredScalableResource(controller, unstructuredScalableResource) if err != nil { return nil, err } - return &nodegroup{ - machineController: controller, - scalableResource: scalableResource, - }, nil -} -func newNodegroupFromMachineDeployment(controller *machineController, machineDeployment *MachineDeployment) (*nodegroup, error) { - scalableResource, err := newMachineDeploymentScalableResource(controller, machineDeployment) - if err != nil { - return nil, err - } 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 29a5212fa235..8deb746b7506 100644 --- a/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_nodegroup_test.go +++ b/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_nodegroup_test.go @@ -27,10 +27,10 @@ import ( corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - v1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/util/wait" + "k8s.io/autoscaler/cluster-autoscaler/cloudprovider" - "k8s.io/utils/pointer" ) const ( @@ -104,18 +104,18 @@ func TestNodeGroupNewNodeGroupConstructor(t *testing.T) { errors: false, }} - newNodeGroup := func(t *testing.T, controller *machineController, testConfig *testConfig) (*nodegroup, error) { + newNodeGroup := func(controller *machineController, testConfig *testConfig) (*nodegroup, error) { if testConfig.machineDeployment != nil { - return newNodegroupFromMachineDeployment(controller, testConfig.machineDeployment) + return newNodegroupFromScalableResource(controller, testConfig.machineDeployment) } - return newNodegroupFromMachineSet(controller, testConfig.machineSet) + return newNodegroupFromScalableResource(controller, testConfig.machineSet) } test := func(t *testing.T, tc testCase, testConfig *testConfig) { controller, stop := mustCreateTestController(t, testConfig) defer stop() - ng, err := newNodeGroup(t, controller, testConfig) + ng, err := newNodeGroup(controller, testConfig) if tc.errors && err == nil { t.Fatal("expected an error") } @@ -134,26 +134,25 @@ func TestNodeGroupNewNodeGroupConstructor(t *testing.T) { t.Fatal("expected nodegroup to be non-nil") } - var expectedName string + var expectedName, expectedKind string - switch v := (ng.scalableResource).(type) { - case *machineSetScalableResource: - expectedName = testConfig.spec.machineSetName - case *machineDeploymentScalableResource: + if testConfig.machineDeployment != nil { + expectedKind = machineDeploymentKind expectedName = testConfig.spec.machineDeploymentName - default: - t.Fatalf("unexpected type: %T", v) + } else { + expectedKind = machineSetKind + expectedName = testConfig.spec.machineSetName } - expectedID := path.Join(testConfig.spec.namespace, expectedName) + expectedID := path.Join(expectedKind, testConfig.spec.namespace, expectedName) expectedDebug := fmt.Sprintf(debugFormat, expectedID, tc.minSize, tc.maxSize, tc.replicas) - if ng.Name() != expectedName { - t.Errorf("expected %q, got %q", expectedName, ng.Name()) + if ng.scalableResource.Name() != expectedName { + t.Errorf("expected %q, got %q", expectedName, ng.scalableResource.Name()) } - if ng.Namespace() != testConfig.spec.namespace { - t.Errorf("expected %q, got %q", testConfig.spec.namespace, ng.Namespace()) + if ng.scalableResource.Namespace() != testConfig.spec.namespace { + t.Errorf("expected %q, got %q", testConfig.spec.namespace, ng.scalableResource.Namespace()) } if ng.MinSize() != tc.minSize { @@ -198,7 +197,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(testNamespace, tc.nodeCount, tc.annotations)) + test(t, tc, createMachineSetTestConfig(RandomString(6), RandomString(6), tc.nodeCount, tc.annotations)) }) } }) @@ -206,7 +205,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(testNamespace, tc.nodeCount, tc.annotations)) + test(t, tc, createMachineDeploymentTestConfig(RandomString(6), RandomString(6), tc.nodeCount, tc.annotations)) }) } }) @@ -270,27 +269,16 @@ func TestNodeGroupIncreaseSizeErrors(t *testing.T) { t.Errorf("expected error message to contain %q, got %q", tc.errorMsg, err.Error()) } - switch v := (ng.scalableResource).(type) { - case *machineSetScalableResource: - // A nodegroup is immutable; get a fresh copy. - ms, err := ng.machineController.getMachineSet(ng.Namespace(), ng.Name(), v1.GetOptions{}) - if err != nil { - t.Fatalf("unexpected error: %v", err) - } - if actual := pointer.Int32PtrDerefOr(ms.Spec.Replicas, 0); actual != tc.initial { - t.Errorf("expected %v, got %v", tc.initial, actual) - } - case *machineDeploymentScalableResource: - // A nodegroup is immutable; get a fresh copy. - md, err := ng.machineController.getMachineDeployment(ng.Namespace(), ng.Name(), v1.GetOptions{}) - if err != nil { - t.Fatalf("unexpected error: %v", err) - } - if actual := pointer.Int32PtrDerefOr(md.Spec.Replicas, 0); actual != tc.initial { - t.Errorf("expected %v, got %v", tc.initial, actual) - } - default: - t.Errorf("unexpected type: %T", v) + gvr, err := ng.scalableResource.GroupVersionResource() + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + scalableResource, err := ng.machineController.managementScaleClient.Scales(testConfig.spec.namespace). + Get(context.TODO(), gvr.GroupResource(), ng.scalableResource.Name(), metav1.GetOptions{}) + + if scalableResource.Spec.Replicas != tc.initial { + t.Errorf("expected %v, got %v", tc.initial, scalableResource.Spec.Replicas) } } @@ -301,7 +289,7 @@ func TestNodeGroupIncreaseSizeErrors(t *testing.T) { nodeGroupMinSizeAnnotationKey: "1", nodeGroupMaxSizeAnnotationKey: "10", } - test(t, &tc, createMachineSetTestConfig(testNamespace, int(tc.initial), annotations)) + test(t, &tc, createMachineSetTestConfig(RandomString(6), RandomString(6), int(tc.initial), annotations)) }) } }) @@ -313,7 +301,7 @@ func TestNodeGroupIncreaseSizeErrors(t *testing.T) { nodeGroupMinSizeAnnotationKey: "1", nodeGroupMaxSizeAnnotationKey: "10", } - test(t, &tc, createMachineDeploymentTestConfig(testNamespace, int(tc.initial), annotations)) + test(t, &tc, createMachineDeploymentTestConfig(RandomString(6), RandomString(6), int(tc.initial), annotations)) }) } }) @@ -354,27 +342,16 @@ func TestNodeGroupIncreaseSize(t *testing.T) { t.Fatalf("unexpected error: %v", err) } - switch v := (ng.scalableResource).(type) { - case *machineSetScalableResource: - // A nodegroup is immutable; get a fresh copy. - ms, err := ng.machineController.getMachineSet(ng.Namespace(), ng.Name(), v1.GetOptions{}) - if err != nil { - t.Fatalf("unexpected error: %v", err) - } - if actual := pointer.Int32PtrDerefOr(ms.Spec.Replicas, 0); actual != tc.expected { - t.Errorf("expected %v, got %v", tc.expected, actual) - } - case *machineDeploymentScalableResource: - // A nodegroup is immutable; get a fresh copy. - md, err := ng.machineController.getMachineDeployment(ng.Namespace(), ng.Name(), v1.GetOptions{}) - if err != nil { - t.Fatalf("unexpected error: %v", err) - } - if actual := pointer.Int32PtrDerefOr(md.Spec.Replicas, 0); actual != tc.expected { - t.Errorf("expected %v, got %v", tc.expected, actual) - } - default: - t.Errorf("unexpected type: %T", v) + gvr, err := ng.scalableResource.GroupVersionResource() + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + scalableResource, err := ng.machineController.managementScaleClient.Scales(ng.scalableResource.Namespace()). + Get(context.TODO(), gvr.GroupResource(), ng.scalableResource.Name(), metav1.GetOptions{}) + + if scalableResource.Spec.Replicas != tc.expected { + t.Errorf("expected %v, got %v", tc.expected, scalableResource.Spec.Replicas) } } @@ -390,7 +367,7 @@ func TestNodeGroupIncreaseSize(t *testing.T) { expected: 4, delta: 1, } - test(t, &tc, createMachineSetTestConfig(testNamespace, int(tc.initial), annotations)) + test(t, &tc, createMachineSetTestConfig(RandomString(6), RandomString(6), int(tc.initial), annotations)) }) t.Run("MachineDeployment", func(t *testing.T) { @@ -400,7 +377,7 @@ func TestNodeGroupIncreaseSize(t *testing.T) { expected: 4, delta: 1, } - test(t, &tc, createMachineDeploymentTestConfig(testNamespace, int(tc.initial), annotations)) + test(t, &tc, createMachineDeploymentTestConfig(RandomString(6), RandomString(6), int(tc.initial), annotations)) }) } @@ -428,31 +405,28 @@ func TestNodeGroupDecreaseTargetSize(t *testing.T) { } ng := nodegroups[0] + + gvr, err := ng.scalableResource.GroupVersionResource() + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + // DecreaseTargetSize should only decrease the size when the current target size of the nodeGroup // is bigger than the number existing instances for that group. We force such a scenario with targetSizeIncrement. - switch v := (ng.scalableResource).(type) { - case *machineSetScalableResource: - testConfig.machineSet.Spec.Replicas = int32ptr(*testConfig.machineSet.Spec.Replicas + tc.targetSizeIncrement) - u := newUnstructuredFromMachineSet(testConfig.machineSet) - if err := controller.machineSetInformer.Informer().GetStore().Add(u); err != nil { - t.Fatalf("failed to add new machine: %v", err) - } - _, err := controller.dynamicclient.Resource(*controller.machineSetResource).Namespace(u.GetNamespace()).Update(context.TODO(), u, metav1.UpdateOptions{}) - if err != nil { - t.Fatalf("failed to updating machine: %v", err) - } - case *machineDeploymentScalableResource: - testConfig.machineDeployment.Spec.Replicas = int32ptr(*testConfig.machineDeployment.Spec.Replicas + tc.targetSizeIncrement) - u := newUnstructuredFromMachineDeployment(testConfig.machineDeployment) - if err := controller.machineDeploymentInformer.Informer().GetStore().Add(u); err != nil { - } - _, err := controller.dynamicclient.Resource(*controller.machineDeploymentResource).Namespace(u.GetNamespace()).Update(context.TODO(), u, metav1.UpdateOptions{}) - if err != nil { - t.Fatalf("failed to updating machine: %v", err) - } - default: - t.Errorf("unexpected type: %T", v) + scalableResource, err := controller.managementScaleClient.Scales(testConfig.spec.namespace). + Get(context.TODO(), gvr.GroupResource(), ng.scalableResource.Name(), metav1.GetOptions{}) + if err != nil { + t.Fatalf("unexpected error: %v", err) } + + scalableResource.Spec.Replicas += tc.targetSizeIncrement + + _, err = ng.machineController.managementScaleClient.Scales(ng.scalableResource.Namespace()). + Update(context.TODO(), gvr.GroupResource(), scalableResource, metav1.UpdateOptions{}) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + // A nodegroup is immutable; get a fresh copy after adding targetSizeIncrement. nodegroups, err = controller.nodeGroups() if err != nil { @@ -473,27 +447,14 @@ func TestNodeGroupDecreaseTargetSize(t *testing.T) { t.Fatalf("expected error: %v, got: %v", tc.expectedError, err) } - switch v := (ng.scalableResource).(type) { - case *machineSetScalableResource: - // A nodegroup is immutable; get a fresh copy. - ms, err := ng.machineController.getMachineSet(ng.Namespace(), ng.Name(), v1.GetOptions{}) - if err != nil { - t.Fatalf("unexpected error: %v", err) - } - if actual := pointer.Int32PtrDerefOr(ms.Spec.Replicas, 0); actual != tc.expected { - t.Errorf("expected %v, got %v", tc.expected, actual) - } - case *machineDeploymentScalableResource: - // A nodegroup is immutable; get a fresh copy. - md, err := ng.machineController.getMachineDeployment(ng.Namespace(), ng.Name(), v1.GetOptions{}) - if err != nil { - t.Fatalf("unexpected error: %v", err) - } - if actual := pointer.Int32PtrDerefOr(md.Spec.Replicas, 0); actual != tc.expected { - t.Errorf("expected %v, got %v", tc.expected, actual) - } - default: - t.Errorf("unexpected type: %T", v) + scalableResource, err = controller.managementScaleClient.Scales(testConfig.spec.namespace). + Get(context.TODO(), gvr.GroupResource(), ng.scalableResource.Name(), metav1.GetOptions{}) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + if scalableResource.Spec.Replicas != tc.expected { + t.Errorf("expected %v, got %v", tc.expected, scalableResource.Spec.Replicas) } } @@ -511,18 +472,18 @@ func TestNodeGroupDecreaseTargetSize(t *testing.T) { delta: -1, expectedError: true, } - test(t, &tc, createMachineSetTestConfig(testNamespace, int(tc.initial), annotations)) + test(t, &tc, createMachineSetTestConfig(RandomString(6), RandomString(6), int(tc.initial), annotations)) }) t.Run("MachineSet", func(t *testing.T) { tc := testCase{ - description: "A node group with targe size 4 but only 3 existing instances should decrease by 1", + description: "A node group with target size 4 but only 3 existing instances should decrease by 1", initial: 3, targetSizeIncrement: 1, expected: 3, delta: -1, } - test(t, &tc, createMachineSetTestConfig(testNamespace, int(tc.initial), annotations)) + test(t, &tc, createMachineSetTestConfig(RandomString(6), RandomString(6), int(tc.initial), annotations)) }) t.Run("MachineDeployment", func(t *testing.T) { @@ -534,7 +495,7 @@ func TestNodeGroupDecreaseTargetSize(t *testing.T) { delta: -1, expectedError: true, } - test(t, &tc, createMachineDeploymentTestConfig(testNamespace, int(tc.initial), annotations)) + test(t, &tc, createMachineDeploymentTestConfig(RandomString(6), RandomString(6), int(tc.initial), annotations)) }) } @@ -596,27 +557,16 @@ func TestNodeGroupDecreaseSizeErrors(t *testing.T) { t.Errorf("expected error message to contain %q, got %q", tc.errorMsg, err.Error()) } - switch v := (ng.scalableResource).(type) { - case *machineSetScalableResource: - // A nodegroup is immutable; get a fresh copy. - ms, err := ng.machineController.getMachineSet(ng.Namespace(), ng.Name(), v1.GetOptions{}) - if err != nil { - t.Fatalf("unexpected error: %v", err) - } - if actual := pointer.Int32PtrDerefOr(ms.Spec.Replicas, 0); actual != tc.initial { - t.Errorf("expected %v, got %v", tc.initial, actual) - } - case *machineDeploymentScalableResource: - // A nodegroup is immutable; get a fresh copy. - md, err := ng.machineController.getMachineDeployment(ng.Namespace(), ng.Name(), v1.GetOptions{}) - if err != nil { - t.Fatalf("unexpected error: %v", err) - } - if actual := pointer.Int32PtrDerefOr(md.Spec.Replicas, 0); actual != tc.initial { - t.Errorf("expected %v, got %v", tc.initial, actual) - } - default: - t.Errorf("unexpected type: %T", v) + gvr, err := ng.scalableResource.GroupVersionResource() + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + scalableResource, err := ng.machineController.managementScaleClient.Scales(testConfig.spec.namespace). + Get(context.TODO(), gvr.GroupResource(), ng.scalableResource.Name(), metav1.GetOptions{}) + + if scalableResource.Spec.Replicas != tc.initial { + t.Errorf("expected %v, got %v", tc.initial, scalableResource.Spec.Replicas) } } @@ -627,7 +577,7 @@ func TestNodeGroupDecreaseSizeErrors(t *testing.T) { nodeGroupMinSizeAnnotationKey: "1", nodeGroupMaxSizeAnnotationKey: "10", } - test(t, &tc, createMachineSetTestConfig(testNamespace, int(tc.initial), annotations)) + test(t, &tc, createMachineSetTestConfig(RandomString(6), RandomString(6), int(tc.initial), annotations)) }) } }) @@ -639,7 +589,7 @@ func TestNodeGroupDecreaseSizeErrors(t *testing.T) { nodeGroupMinSizeAnnotationKey: "1", nodeGroupMaxSizeAnnotationKey: "10", } - test(t, &tc, createMachineDeploymentTestConfig(testNamespace, int(tc.initial), annotations)) + test(t, &tc, createMachineDeploymentTestConfig(RandomString(6), RandomString(6), int(tc.initial), annotations)) }) } }) @@ -684,34 +634,30 @@ func TestNodeGroupDeleteNodes(t *testing.T) { } for i := 5; i < len(testConfig.machines); i++ { - machine, err := controller.getMachine(testConfig.machines[i].Namespace, testConfig.machines[i].Name, v1.GetOptions{}) + machine, err := controller.managementClient.Resource(controller.machineResource). + Namespace(testConfig.spec.namespace). + Get(context.TODO(), testConfig.machines[i].GetName(), metav1.GetOptions{}) if err != nil { t.Fatalf("unexpected error: %v", err) } - if _, found := machine.Annotations[machineDeleteAnnotationKey]; !found { - t.Errorf("expected annotation %q on machine %s", machineDeleteAnnotationKey, machine.Name) + if _, found := machine.GetAnnotations()[machineDeleteAnnotationKey]; !found { + t.Errorf("expected annotation %q on machine %s", machineDeleteAnnotationKey, machine.GetName()) } } - switch v := (ng.scalableResource).(type) { - case *machineSetScalableResource: - updatedMachineSet, err := controller.getMachineSet(testConfig.machineSet.Namespace, testConfig.machineSet.Name, v1.GetOptions{}) - if err != nil { - t.Fatalf("unexpected error: %v", err) - } - if actual := pointer.Int32PtrDerefOr(updatedMachineSet.Spec.Replicas, 0); actual != 5 { - t.Fatalf("expected 5 nodes, got %v", actual) - } - case *machineDeploymentScalableResource: - updatedMachineDeployment, err := controller.getMachineDeployment(testConfig.machineDeployment.Namespace, testConfig.machineDeployment.Name, v1.GetOptions{}) - if err != nil { - t.Fatalf("unexpected error: %v", err) - } - if actual := pointer.Int32PtrDerefOr(updatedMachineDeployment.Spec.Replicas, 0); actual != 5 { - t.Fatalf("expected 5 nodes, got %v", actual) - } - default: - t.Errorf("unexpected type: %T", v) + gvr, err := ng.scalableResource.GroupVersionResource() + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + scalableResource, err := ng.machineController.managementScaleClient.Scales(testConfig.spec.namespace). + Get(context.TODO(), gvr.GroupResource(), ng.scalableResource.Name(), metav1.GetOptions{}) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + if scalableResource.Spec.Replicas != 5 { + t.Errorf("expected 5, got %v", scalableResource.Spec.Replicas) } } @@ -721,14 +667,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(testNamespace, 10, map[string]string{ + test(t, createMachineSetTestConfig(RandomString(6), RandomString(6), 10, map[string]string{ nodeGroupMinSizeAnnotationKey: "1", nodeGroupMaxSizeAnnotationKey: "10", })) }) t.Run("MachineDeployment", func(t *testing.T) { - test(t, createMachineDeploymentTestConfig(testNamespace, 10, map[string]string{ + test(t, createMachineDeploymentTestConfig(RandomString(6), RandomString(6), 10, map[string]string{ nodeGroupMinSizeAnnotationKey: "1", nodeGroupMaxSizeAnnotationKey: "10", })) @@ -765,13 +711,10 @@ func TestNodeGroupMachineSetDeleteNodesWithMismatchedNodes(t *testing.T) { t.Error("expected an error") } - expectedErr0 := `node "test-namespace1-machineset-0-nodeid-0" doesn't belong to node group "test-namespace0/machineset-0"` - if testConfig0.machineDeployment != nil { - expectedErr0 = `node "test-namespace1-machineset-0-nodeid-0" doesn't belong to node group "test-namespace0/machinedeployment-0"` - } + expectedErrSubstring := "doesn't belong to node group" - if !strings.Contains(err0.Error(), string(normalizedProviderString(expectedErr0))) { - t.Errorf("expected: %q, got: %q", expectedErr0, err0.Error()) + if !strings.Contains(err0.Error(), expectedErrSubstring) { + t.Errorf("expected error: %q to contain: %q", err0.Error(), expectedErrSubstring) } // Deleting nodes that are not in ng1 should fail. @@ -780,13 +723,8 @@ func TestNodeGroupMachineSetDeleteNodesWithMismatchedNodes(t *testing.T) { t.Error("expected an error") } - expectedErr1 := `node "test-namespace0-machineset-0-nodeid-0" doesn't belong to node group "test-namespace1/machineset-0"` - if testConfig1.machineDeployment != nil { - expectedErr1 = `node "test-namespace0-machineset-0-nodeid-0" doesn't belong to node group "test-namespace1/machinedeployment-0"` - } - - if !strings.Contains(err1.Error(), string(normalizedProviderString(expectedErr1))) { - t.Errorf("expected: %q, got: %q", expectedErr1, err1.Error()) + if !strings.Contains(err1.Error(), expectedErrSubstring) { + t.Errorf("expected error: %q to contain: %q", err0.Error(), expectedErrSubstring) } // Deleting from correct node group should fail because @@ -808,20 +746,22 @@ func TestNodeGroupMachineSetDeleteNodesWithMismatchedNodes(t *testing.T) { } t.Run("MachineSet", func(t *testing.T) { - testConfig0 := createMachineSetTestConfigs(testNamespace+"0", 1, 2, annotations) - testConfig1 := createMachineSetTestConfigs(testNamespace+"1", 1, 2, annotations) + namespace := RandomString(6) + testConfig0 := createMachineSetTestConfigs(namespace, RandomString(6), 1, 2, annotations) + testConfig1 := createMachineSetTestConfigs(namespace, RandomString(6), 1, 2, annotations) test(t, 2, append(testConfig0, testConfig1...)) }) t.Run("MachineDeployment", func(t *testing.T) { - testConfig0 := createMachineDeploymentTestConfigs(testNamespace+"0", 1, 2, annotations) - testConfig1 := createMachineDeploymentTestConfigs(testNamespace+"1", 1, 2, annotations) + namespace := RandomString(6) + testConfig0 := createMachineDeploymentTestConfigs(namespace, RandomString(6), 1, 2, annotations) + testConfig1 := createMachineDeploymentTestConfigs(namespace, RandomString(6), 1, 2, annotations) test(t, 2, append(testConfig0, testConfig1...)) }) } func TestNodeGroupDeleteNodesTwice(t *testing.T) { - addDeletionTimestampToMachine := func(t *testing.T, controller *machineController, node *corev1.Node) error { + addDeletionTimestampToMachine := func(controller *machineController, node *corev1.Node) error { m, err := controller.findMachineByProviderID(normalizedProviderString(node.Spec.ProviderID)) if err != nil { return err @@ -831,9 +771,12 @@ func TestNodeGroupDeleteNodesTwice(t *testing.T) { // Machine API controllers were running Don't actually // delete since the fake client does not support // finalizers. - now := v1.Now() - m.DeletionTimestamp = &now - if _, err := controller.dynamicclient.Resource(*controller.machineResource).Namespace(m.GetNamespace()).Update(context.Background(), newUnstructuredFromMachine(m), v1.UpdateOptions{}); err != nil { + now := metav1.Now() + + m.SetDeletionTimestamp(&now) + + if _, err := controller.managementClient.Resource(controller.machineResource). + Namespace(m.GetNamespace()).Update(context.TODO(), m, metav1.UpdateOptions{}); err != nil { return err } @@ -893,7 +836,7 @@ func TestNodeGroupDeleteNodesTwice(t *testing.T) { // Assert that we have no DeletionTimestamp for i := expectedSize; i < len(testConfig.machines); i++ { - if !testConfig.machines[i].ObjectMeta.DeletionTimestamp.IsZero() { + if !testConfig.machines[i].GetDeletionTimestamp().IsZero() { t.Fatalf("unexpected DeletionTimestamp") } } @@ -904,7 +847,7 @@ func TestNodeGroupDeleteNodesTwice(t *testing.T) { } for _, node := range nodesToBeDeleted { - if err := addDeletionTimestampToMachine(t, controller, node); err != nil { + if err := addDeletionTimestampToMachine(controller, node); err != nil { t.Fatalf("unexpected err: %v", err) } } @@ -963,25 +906,16 @@ func TestNodeGroupDeleteNodesTwice(t *testing.T) { t.Fatalf("unexpected error: %v", err) } - switch v := (ng.scalableResource).(type) { - case *machineSetScalableResource: - updatedMachineSet, err := controller.getMachineSet(testConfig.machineSet.Namespace, testConfig.machineSet.Name, metav1.GetOptions{}) - if err != nil { - t.Fatalf("unexpected error: %v", err) - } - if actual := pointer.Int32PtrDerefOr(updatedMachineSet.Spec.Replicas, 0); int(actual) != expectedSize { - t.Fatalf("expected %v nodes, got %v", expectedSize, actual) - } - case *machineDeploymentScalableResource: - updatedMachineDeployment, err := controller.getMachineDeployment(testConfig.machineDeployment.Namespace, testConfig.machineDeployment.Name, metav1.GetOptions{}) - if err != nil { - t.Fatalf("unexpected error: %v", err) - } - if actual := pointer.Int32PtrDerefOr(updatedMachineDeployment.Spec.Replicas, 0); int(actual) != expectedSize { - t.Fatalf("expected %v nodes, got %v", expectedSize, actual) - } - default: - t.Errorf("unexpected type: %T", v) + gvr, err := ng.scalableResource.GroupVersionResource() + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + scalableResource, err := ng.machineController.managementScaleClient.Scales(testConfig.spec.namespace). + Get(context.TODO(), gvr.GroupResource(), ng.scalableResource.Name(), metav1.GetOptions{}) + + if scalableResource.Spec.Replicas != int32(expectedSize) { + t.Errorf("expected %v, got %v", expectedSize, scalableResource.Spec.Replicas) } } @@ -991,14 +925,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(testNamespace, 10, map[string]string{ + test(t, createMachineSetTestConfig(RandomString(6), RandomString(6), 10, map[string]string{ nodeGroupMinSizeAnnotationKey: "1", nodeGroupMaxSizeAnnotationKey: "10", })) }) t.Run("MachineDeployment", func(t *testing.T) { - test(t, createMachineDeploymentTestConfig(testNamespace, 10, map[string]string{ + test(t, createMachineDeploymentTestConfig(RandomString(6), RandomString(6), 10, map[string]string{ nodeGroupMinSizeAnnotationKey: "1", nodeGroupMaxSizeAnnotationKey: "10", })) @@ -1012,10 +946,11 @@ func TestNodeGroupWithFailedMachine(t *testing.T) { // Simulate a failed machine machine := testConfig.machines[3].DeepCopy() - machine.Spec.ProviderID = nil - failureMessage := "FailureMessage" - machine.Status.FailureMessage = &failureMessage - if err := controller.machineInformer.Informer().GetStore().Update(newUnstructuredFromMachine(machine)); err != nil { + + unstructured.RemoveNestedField(machine.Object, "spec", "providerID") + unstructured.SetNestedField(machine.Object, "FailureMessage", "status", "failureMessage") + + if err := updateResource(controller.managementClient, controller.machineInformer, controller.machineResource, machine); err != nil { t.Fatalf("unexpected error updating machine, got %v", err) } @@ -1043,7 +978,7 @@ func TestNodeGroupWithFailedMachine(t *testing.T) { }) // The failed machine key is sorted to the first index - failedMachineID := fmt.Sprintf("%s%s_%s", failedMachinePrefix, machine.Namespace, machine.Name) + failedMachineID := fmt.Sprintf("%s%s_%s", failedMachinePrefix, machine.GetNamespace(), machine.GetName()) if nodeNames[0].Id != failedMachineID { t.Fatalf("expected %q, got %q", failedMachineID, nodeNames[0].Id) } @@ -1071,14 +1006,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(testNamespace, 10, map[string]string{ + test(t, createMachineSetTestConfig(RandomString(6), RandomString(6), 10, map[string]string{ nodeGroupMinSizeAnnotationKey: "1", nodeGroupMaxSizeAnnotationKey: "10", })) }) t.Run("MachineDeployment", func(t *testing.T) { - test(t, createMachineDeploymentTestConfig(testNamespace, 10, map[string]string{ + test(t, createMachineDeploymentTestConfig(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 63b08a6e2cbd..a0a017cad67a 100644 --- a/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_provider.go +++ b/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_provider.go @@ -21,14 +21,18 @@ import ( corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/resource" - "k8s.io/autoscaler/cluster-autoscaler/cloudprovider" - "k8s.io/autoscaler/cluster-autoscaler/config" - "k8s.io/autoscaler/cluster-autoscaler/utils/errors" "k8s.io/client-go/discovery" + "k8s.io/client-go/discovery/cached/memory" "k8s.io/client-go/dynamic" "k8s.io/client-go/kubernetes" + "k8s.io/client-go/restmapper" + "k8s.io/client-go/scale" "k8s.io/client-go/tools/clientcmd" klog "k8s.io/klog/v2" + + "k8s.io/autoscaler/cluster-autoscaler/cloudprovider" + "k8s.io/autoscaler/cluster-autoscaler/config" + "k8s.io/autoscaler/cluster-autoscaler/utils/errors" ) const ( @@ -126,12 +130,12 @@ func newProvider( name string, rl *cloudprovider.ResourceLimiter, controller *machineController, -) (cloudprovider.CloudProvider, error) { +) cloudprovider.CloudProvider { return &provider{ providerName: name, resourceLimiter: rl, controller: controller, - }, nil + } } // BuildClusterAPI builds CloudProvider implementation for machine api. @@ -142,22 +146,32 @@ func BuildClusterAPI(opts config.AutoscalingOptions, do cloudprovider.NodeGroupD } // Grab a dynamic interface that we can create informers from - dc, err := dynamic.NewForConfig(externalConfig) + managementClient, err := dynamic.NewForConfig(externalConfig) if err != nil { klog.Fatalf("could not generate dynamic client for config") } - kubeClient, err := kubernetes.NewForConfig(externalConfig) + workloadClient, err := kubernetes.NewForConfig(externalConfig) if err != nil { klog.Fatalf("create kube clientset failed: %v", err) } - discoveryClient, err := discovery.NewDiscoveryClientForConfig(externalConfig) + managementDiscoveryClient, err := discovery.NewDiscoveryClientForConfig(externalConfig) if err != nil { klog.Fatalf("create discovery client failed: %v", err) } - controller, err := newMachineController(dc, kubeClient, discoveryClient) + cachedDiscovery := memory.NewMemCacheClient(managementDiscoveryClient) + managementScaleClient, err := scale.NewForConfig( + externalConfig, + restmapper.NewDeferredDiscoveryRESTMapper(cachedDiscovery), + dynamic.LegacyAPIPathResolverFunc, + scale.NewDiscoveryScaleKindResolver(managementDiscoveryClient)) + if err != nil { + klog.Fatalf("create scale client failed: %v", err) + } + + controller, err := newMachineController(managementClient, workloadClient, managementDiscoveryClient, managementScaleClient) if err != nil { klog.Fatal(err) } @@ -170,10 +184,5 @@ func BuildClusterAPI(opts config.AutoscalingOptions, do cloudprovider.NodeGroupD klog.Fatal(err) } - provider, err := newProvider(ProviderName, rl, controller) - if err != nil { - klog.Fatal(err) - } - - return provider + return newProvider(ProviderName, rl, controller) } diff --git a/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_provider_test.go b/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_provider_test.go index 6ba2774c2cb7..301ee32776a9 100644 --- a/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_provider_test.go +++ b/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_provider_test.go @@ -22,6 +22,7 @@ import ( corev1 "k8s.io/api/core/v1" v1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/autoscaler/cluster-autoscaler/cloudprovider" ) @@ -31,11 +32,7 @@ func TestProviderConstructorProperties(t *testing.T) { controller, stop := mustCreateTestController(t) defer stop() - provider, err := newProvider(ProviderName, &resourceLimits, controller) - if err != nil { - t.Fatalf("unexpected error: %v", err) - } - + provider := newProvider(ProviderName, &resourceLimits, controller) if actual := provider.Name(); actual != ProviderName { t.Errorf("expected %q, got %q", ProviderName, actual) } diff --git a/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_scalableresource.go b/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_scalableresource.go deleted file mode 100644 index aa1228fb4420..000000000000 --- a/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_scalableresource.go +++ /dev/null @@ -1,74 +0,0 @@ -/* -Copyright 2020 The Kubernetes Authors. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package clusterapi - -import ( - "context" - - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" -) - -// scalableResource is a resource that can be scaled up and down by -// adjusting its replica count field. -type scalableResource interface { - // Id returns an unique identifier of the resource - ID() string - - // MaxSize returns maximum size of the resource - MaxSize() int - - // MinSize returns minimum size of the resource - MinSize() int - - // Name returns the name of the resource - Name() string - - // Namespace returns the namespace the resource is in - Namespace() string - - // Nodes returns a list of all machines that already have or should become nodes that belong to this - // resource - Nodes() ([]string, error) - - // SetSize() sets the replica count of the resource - SetSize(nreplicas int32) error - - // Replicas returns the current replica count of the resource - Replicas() (int32, error) - - // MarkMachineForDeletion marks machine for deletion - MarkMachineForDeletion(machine *Machine) error - - // UnmarkMachineForDeletion unmarks machine for deletion - UnmarkMachineForDeletion(machine *Machine) error -} - -func unmarkMachineForDeletion(controller *machineController, machine *Machine) error { - u, err := controller.dynamicclient.Resource(*controller.machineResource).Namespace(machine.Namespace).Get(context.TODO(), machine.Name, metav1.GetOptions{}) - if err != nil { - return err - } - - annotations := u.GetAnnotations() - if _, ok := annotations[machineDeleteAnnotationKey]; ok { - delete(annotations, machineDeleteAnnotationKey) - u.SetAnnotations(annotations) - _, updateErr := controller.dynamicclient.Resource(*controller.machineResource).Namespace(u.GetNamespace()).Update(context.TODO(), u, metav1.UpdateOptions{}) - return updateErr - } - return nil -} diff --git a/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_unstructured.go b/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_unstructured.go new file mode 100644 index 000000000000..3fd5bd9849ce --- /dev/null +++ b/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_unstructured.go @@ -0,0 +1,169 @@ +/* +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 ( + "context" + "fmt" + "path" + "time" + + "github.com/pkg/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/runtime/schema" +) + +type unstructuredScalableResource struct { + controller *machineController + unstructured *unstructured.Unstructured + maxSize int + minSize int +} + +func (r unstructuredScalableResource) ID() string { + return path.Join(r.Kind(), r.Namespace(), r.Name()) +} + +func (r unstructuredScalableResource) MaxSize() int { + return r.maxSize +} + +func (r unstructuredScalableResource) MinSize() int { + return r.minSize +} + +func (r unstructuredScalableResource) Kind() string { + return r.unstructured.GetKind() +} + +func (r unstructuredScalableResource) GroupVersionResource() (schema.GroupVersionResource, error) { + switch r.Kind() { + case machineDeploymentKind: + return r.controller.machineDeploymentResource, nil + case machineSetKind: + return r.controller.machineSetResource, nil + default: + return schema.GroupVersionResource{}, fmt.Errorf("unknown scalable resource kind %s", r.Kind()) + } +} + +func (r unstructuredScalableResource) Name() string { + return r.unstructured.GetName() +} + +func (r unstructuredScalableResource) Namespace() string { + return r.unstructured.GetNamespace() +} + +func (r unstructuredScalableResource) ProviderIDs() ([]string, error) { + providerIds, err := r.controller.scalableResourceProviderIDs(r.unstructured) + if err != nil { + return nil, err + } + + return providerIds, nil +} + +func (r unstructuredScalableResource) Replicas() (int32, error) { + gvr, err := r.GroupVersionResource() + if err != nil { + return 0, err + } + + s, err := r.controller.managementScaleClient.Scales(r.Namespace()).Get(context.TODO(), gvr.GroupResource(), r.Name(), metav1.GetOptions{}) + if err != nil { + return 0, err + } + if s == nil { + return 0, fmt.Errorf("unknown %s %s/%s", r.Kind(), r.Namespace(), r.Name()) + } + return s.Spec.Replicas, nil +} + +func (r unstructuredScalableResource) SetSize(nreplicas int32) error { + gvr, err := r.GroupVersionResource() + if err != nil { + return err + } + + s, err := r.controller.managementScaleClient.Scales(r.Namespace()).Get(context.TODO(), gvr.GroupResource(), r.Name(), metav1.GetOptions{}) + if err != nil { + return err + } + + if s == nil { + return fmt.Errorf("unknown %s %s/%s", r.Kind(), r.Namespace(), r.Name()) + } + + s.Spec.Replicas = nreplicas + _, updateErr := r.controller.managementScaleClient.Scales(r.Namespace()).Update(context.TODO(), gvr.GroupResource(), s, metav1.UpdateOptions{}) + return updateErr +} + +func (r unstructuredScalableResource) UnmarkMachineForDeletion(machine *unstructured.Unstructured) error { + u, err := r.controller.managementClient.Resource(r.controller.machineResource).Namespace(machine.GetNamespace()).Get(context.TODO(), machine.GetName(), metav1.GetOptions{}) + if err != nil { + return err + } + + annotations := u.GetAnnotations() + if _, ok := annotations[machineDeleteAnnotationKey]; ok { + delete(annotations, machineDeleteAnnotationKey) + u.SetAnnotations(annotations) + _, updateErr := r.controller.managementClient.Resource(r.controller.machineResource).Namespace(u.GetNamespace()).Update(context.TODO(), u, metav1.UpdateOptions{}) + + return updateErr + } + + return nil +} + +func (r unstructuredScalableResource) MarkMachineForDeletion(machine *unstructured.Unstructured) error { + u, err := r.controller.managementClient.Resource(r.controller.machineResource).Namespace(machine.GetNamespace()).Get(context.TODO(), machine.GetName(), metav1.GetOptions{}) + if err != nil { + return err + } + + u = u.DeepCopy() + + annotations := u.GetAnnotations() + if annotations == nil { + annotations = map[string]string{} + } + + annotations[machineDeleteAnnotationKey] = time.Now().String() + u.SetAnnotations(annotations) + + _, updateErr := r.controller.managementClient.Resource(r.controller.machineResource).Namespace(u.GetNamespace()).Update(context.TODO(), u, metav1.UpdateOptions{}) + + return updateErr +} + +func newUnstructuredScalableResource(controller *machineController, u *unstructured.Unstructured) (*unstructuredScalableResource, error) { + minSize, maxSize, err := parseScalingBounds(u.GetAnnotations()) + if err != nil { + return nil, errors.Wrap(err, "error validating min/max annotations") + } + + return &unstructuredScalableResource{ + controller: controller, + unstructured: u, + maxSize: maxSize, + minSize: minSize, + }, nil +} diff --git a/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_unstructured_test.go b/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_unstructured_test.go new file mode 100644 index 000000000000..2c51dcfbf79d --- /dev/null +++ b/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_unstructured_test.go @@ -0,0 +1,186 @@ +/* +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 ( + "context" + "testing" + + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +func TestSetSize(t *testing.T) { + initialReplicas := 1 + updatedReplicas := int32(5) + + test := func(t *testing.T, testConfig *testConfig) { + controller, stop := mustCreateTestController(t, testConfig) + defer stop() + + testResource := testConfig.machineSet + if testConfig.machineDeployment != nil { + testResource = testConfig.machineDeployment + } + + sr, err := newUnstructuredScalableResource(controller, testResource) + if err != nil { + t.Fatal(err) + } + + gvr, err := sr.GroupVersionResource() + if err != nil { + t.Fatal(err) + } + + err = sr.SetSize(updatedReplicas) + if err != nil { + t.Fatal(err) + } + + s, err := sr.controller.managementScaleClient.Scales(testResource.GetNamespace()). + Get(context.TODO(), gvr.GroupResource(), testResource.GetName(), metav1.GetOptions{}) + + if s.Spec.Replicas != 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)) + }) + + t.Run("MachineDeployment", func(t *testing.T) { + test(t, createMachineDeploymentTestConfig(RandomString(6), RandomString(6), initialReplicas, nil)) + }) +} + +func TestReplicas(t *testing.T) { + initialReplicas := 1 + updatedReplicas := int32(5) + + test := func(t *testing.T, testConfig *testConfig) { + controller, stop := mustCreateTestController(t, testConfig) + defer stop() + + testResource := testConfig.machineSet + if testConfig.machineDeployment != nil { + testResource = testConfig.machineDeployment + } + + sr, err := newUnstructuredScalableResource(controller, testResource) + if err != nil { + t.Fatal(err) + } + + gvr, err := sr.GroupVersionResource() + if err != nil { + t.Fatal(err) + } + + i, err := sr.Replicas() + if err != nil { + t.Fatal(err) + } + + if i != int32(initialReplicas) { + t.Errorf("expected %v, got: %v", initialReplicas, i) + } + + // fetch and update machineSet + s, err := sr.controller.managementScaleClient.Scales(testResource.GetNamespace()). + Get(context.TODO(), gvr.GroupResource(), testResource.GetName(), metav1.GetOptions{}) + if err != nil { + t.Fatal(err) + } + + s.Spec.Replicas = updatedReplicas + + _, err = sr.controller.managementScaleClient.Scales(testResource.GetNamespace()). + Update(context.TODO(), gvr.GroupResource(), s, metav1.UpdateOptions{}) + if err != nil { + t.Fatal(err) + } + + i, err = sr.Replicas() + if err != nil { + t.Fatal(err) + } + + if i != updatedReplicas { + t.Errorf("expected %v, got: %v", updatedReplicas, i) + } + } + + t.Run("MachineSet", func(t *testing.T) { + test(t, createMachineSetTestConfig(RandomString(6), RandomString(6), initialReplicas, nil)) + }) + + t.Run("MachineDeployment", func(t *testing.T) { + test(t, createMachineDeploymentTestConfig(RandomString(6), RandomString(6), initialReplicas, nil)) + }) +} + +func TestSetSizeAndReplicas(t *testing.T) { + initialReplicas := 1 + updatedReplicas := int32(5) + + test := func(t *testing.T, testConfig *testConfig) { + controller, stop := mustCreateTestController(t, testConfig) + defer stop() + + testResource := testConfig.machineSet + if testConfig.machineDeployment != nil { + testResource = testConfig.machineDeployment + } + + sr, err := newUnstructuredScalableResource(controller, testResource) + if err != nil { + t.Fatal(err) + } + + i, err := sr.Replicas() + if err != nil { + t.Fatal(err) + } + + if i != int32(initialReplicas) { + t.Errorf("expected %v, got: %v", initialReplicas, i) + } + + err = sr.SetSize(updatedReplicas) + if err != nil { + t.Fatal(err) + } + + i, err = sr.Replicas() + if err != nil { + t.Fatal(err) + } + + if i != updatedReplicas { + t.Errorf("expected %v, got: %v", updatedReplicas, i) + } + } + + t.Run("MachineSet", func(t *testing.T) { + test(t, createMachineSetTestConfig(RandomString(6), RandomString(6), initialReplicas, nil)) + }) + + t.Run("MachineDeployment", func(t *testing.T) { + test(t, createMachineDeploymentTestConfig(RandomString(6), RandomString(6), initialReplicas, nil)) + }) +} diff --git a/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_utils.go b/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_utils.go index 0e0f00151b0c..ed7bacf49c72 100644 --- a/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_utils.go +++ b/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_utils.go @@ -22,6 +22,7 @@ import ( "github.com/pkg/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" ) const ( @@ -109,9 +110,9 @@ func parseScalingBounds(annotations map[string]string) (int, int, error) { return minSize, maxSize, nil } -func machineOwnerRef(machine *Machine) *metav1.OwnerReference { - for _, ref := range machine.OwnerReferences { - if ref.Kind == "MachineSet" && ref.Name != "" { +func getOwnerForKind(u *unstructured.Unstructured, kind string) *metav1.OwnerReference { + for _, ref := range u.GetOwnerReferences() { + if ref.Kind == kind && ref.Name != "" { return ref.DeepCopy() } } @@ -119,32 +120,16 @@ func machineOwnerRef(machine *Machine) *metav1.OwnerReference { return nil } -func machineIsOwnedByMachineSet(machine *Machine, machineSet *MachineSet) bool { - if ref := machineOwnerRef(machine); ref != nil { - return ref.UID == machineSet.UID - } - return false +func machineOwnerRef(machine *unstructured.Unstructured) *metav1.OwnerReference { + return getOwnerForKind(machine, machineSetKind) } -func machineSetMachineDeploymentRef(machineSet *MachineSet) *metav1.OwnerReference { - for _, ref := range machineSet.OwnerReferences { - if ref.Kind == "MachineDeployment" { - return ref.DeepCopy() - } - } - - return nil -} - -func machineSetHasMachineDeploymentOwnerRef(machineSet *MachineSet) bool { - return machineSetMachineDeploymentRef(machineSet) != nil +func machineSetOwnerRef(machineSet *unstructured.Unstructured) *metav1.OwnerReference { + return getOwnerForKind(machineSet, machineDeploymentKind) } -func machineSetIsOwnedByMachineDeployment(machineSet *MachineSet, machineDeployment *MachineDeployment) bool { - if ref := machineSetMachineDeploymentRef(machineSet); ref != nil { - return ref.UID == machineDeployment.UID - } - return false +func machineSetHasMachineDeploymentOwnerRef(machineSet *unstructured.Unstructured) bool { + return machineSetOwnerRef(machineSet) != nil } // normalizedProviderString splits s on '/' returning everything after diff --git a/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_utils_test.go b/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_utils_test.go index 41e852ab75fb..421fb830e405 100644 --- a/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_utils_test.go +++ b/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_utils_test.go @@ -17,10 +17,12 @@ limitations under the License. package clusterapi import ( + "reflect" "strings" "testing" - v1 "k8s.io/apimachinery/pkg/apis/meta/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" ) const ( @@ -112,13 +114,21 @@ func TestUtilParseScalingBounds(t *testing.T) { max: 1, }} { t.Run(tc.description, func(t *testing.T) { - machineSet := MachineSet{ - ObjectMeta: v1.ObjectMeta{ - Annotations: tc.annotations, + machineSet := unstructured.Unstructured{ + Object: map[string]interface{}{ + "kind": machineSetKind, + "apiVersion": "cluster.x-k8s.io/v1alpha3", + "metadata": map[string]interface{}{ + "name": "test", + "namespace": "default", + }, + "spec": map[string]interface{}{}, + "status": map[string]interface{}{}, }, } + machineSet.SetAnnotations(tc.annotations) - min, max, err := parseScalingBounds(machineSet.Annotations) + min, max, err := parseScalingBounds(machineSet.GetAnnotations()) if tc.error != nil && err == nil { t.Fatalf("test #%d: expected an error", i) } @@ -141,228 +151,244 @@ func TestUtilParseScalingBounds(t *testing.T) { } } -func TestUtilMachineSetIsOwnedByMachineDeployment(t *testing.T) { +func TestUtilGetOwnerByKindMachineSet(t *testing.T) { for _, tc := range []struct { - description string - machineSet MachineSet - machineDeployment MachineDeployment - owned bool + description string + machineSet *unstructured.Unstructured + machineSetOwnerRefs []metav1.OwnerReference + expectedOwnerRef *metav1.OwnerReference }{{ - description: "not owned as no owner references", - machineSet: MachineSet{}, - machineDeployment: MachineDeployment{}, - owned: false, + description: "not owned as no owner references", + machineSet: &unstructured.Unstructured{}, + machineSetOwnerRefs: []metav1.OwnerReference{}, + expectedOwnerRef: nil, }, { description: "not owned as not the same Kind", - machineSet: MachineSet{ - ObjectMeta: v1.ObjectMeta{ - OwnerReferences: []v1.OwnerReference{{ - Kind: "Other", - }}, - }, - }, - machineDeployment: MachineDeployment{}, - owned: false, - }, { - description: "not owned because no OwnerReference.Name", - machineSet: MachineSet{ - ObjectMeta: v1.ObjectMeta{ - OwnerReferences: []v1.OwnerReference{{ - Kind: "MachineSet", - UID: uuid1, - }}, + machineSet: &unstructured.Unstructured{ + Object: map[string]interface{}{ + "kind": machineSetKind, + "apiVersion": "cluster.x-k8s.io/v1alpha3", + "metadata": map[string]interface{}{ + "name": "test", + "namespace": "default", + }, + "spec": map[string]interface{}{}, + "status": map[string]interface{}{}, }, }, - machineDeployment: MachineDeployment{ - ObjectMeta: v1.ObjectMeta{ - UID: uuid1, + machineSetOwnerRefs: []metav1.OwnerReference{ + { + Kind: "Other", }, }, - owned: false, + expectedOwnerRef: nil, }, { - description: "not owned as UID values don't match", - machineSet: MachineSet{ - ObjectMeta: v1.ObjectMeta{ - OwnerReferences: []v1.OwnerReference{{ - Kind: "MachineSet", - Name: "foo", - UID: uuid2, - }}, + description: "not owned because no OwnerReference.Name", + machineSet: &unstructured.Unstructured{ + Object: map[string]interface{}{ + "kind": machineSetKind, + "apiVersion": "cluster.x-k8s.io/v1alpha3", + "metadata": map[string]interface{}{ + "name": "test", + "namespace": "default", + }, + "spec": map[string]interface{}{}, + "status": map[string]interface{}{}, }, }, - machineDeployment: MachineDeployment{ - TypeMeta: v1.TypeMeta{ - Kind: "MachineDeployment", - }, - ObjectMeta: v1.ObjectMeta{ - UID: uuid1, + machineSetOwnerRefs: []metav1.OwnerReference{ + { + Kind: machineDeploymentKind, + UID: uuid1, }, }, - owned: false, + expectedOwnerRef: nil, }, { description: "owned as UID values match and same Kind and Name not empty", - machineSet: MachineSet{ - ObjectMeta: v1.ObjectMeta{ - OwnerReferences: []v1.OwnerReference{{ - Kind: "MachineDeployment", - Name: "foo", - UID: uuid1, - }}, + machineSet: &unstructured.Unstructured{ + Object: map[string]interface{}{ + "kind": machineSetKind, + "apiVersion": "cluster.x-k8s.io/v1alpha3", + "metadata": map[string]interface{}{ + "name": "test", + "namespace": "default", + }, + "spec": map[string]interface{}{}, + "status": map[string]interface{}{}, }, }, - machineDeployment: MachineDeployment{ - TypeMeta: v1.TypeMeta{ - Kind: "MachineDeployment", - }, - ObjectMeta: v1.ObjectMeta{ + machineSetOwnerRefs: []metav1.OwnerReference{ + { + Kind: machineDeploymentKind, Name: "foo", UID: uuid1, }, }, - owned: true, + expectedOwnerRef: &metav1.OwnerReference{ + Kind: machineDeploymentKind, + Name: "foo", + UID: uuid1, + }, }} { t.Run(tc.description, func(t *testing.T) { - owned := machineSetIsOwnedByMachineDeployment(&tc.machineSet, &tc.machineDeployment) - if tc.owned != owned { - t.Errorf("expected %t, got %t", tc.owned, owned) + tc.machineSet.SetOwnerReferences(tc.machineSetOwnerRefs) + + ownerRef := getOwnerForKind(tc.machineSet, machineDeploymentKind) + if !reflect.DeepEqual(tc.expectedOwnerRef, ownerRef) { + t.Errorf("expected %v, got %v", tc.expectedOwnerRef, ownerRef) } }) } } -func TestUtilMachineIsOwnedByMachineSet(t *testing.T) { +func TestUtilGetOwnerByKindMachine(t *testing.T) { for _, tc := range []struct { - description string - machine Machine - machineSet MachineSet - owned bool + description string + machine *unstructured.Unstructured + machineOwnerRefs []metav1.OwnerReference + expectedOwnerRef *metav1.OwnerReference }{{ - description: "not owned as no owner references", - machine: Machine{}, - machineSet: MachineSet{}, - owned: false, + description: "not owned as no owner references", + machine: &unstructured.Unstructured{}, + machineOwnerRefs: []metav1.OwnerReference{}, + expectedOwnerRef: nil, }, { description: "not owned as not the same Kind", - machine: Machine{ - ObjectMeta: v1.ObjectMeta{ - OwnerReferences: []v1.OwnerReference{{ - Kind: "Other", - }}, - }, - }, - machineSet: MachineSet{}, - owned: false, - }, { - description: "not owned because no OwnerReference.Name", - machine: Machine{ - ObjectMeta: v1.ObjectMeta{ - OwnerReferences: []v1.OwnerReference{{ - Kind: "MachineSet", - UID: uuid1, - }}, + machine: &unstructured.Unstructured{ + Object: map[string]interface{}{ + "kind": machineKind, + "apiVersion": "cluster.x-k8s.io/v1alpha3", + "metadata": map[string]interface{}{ + "name": "test", + "namespace": "default", + }, + "spec": map[string]interface{}{}, + "status": map[string]interface{}{}, }, }, - machineSet: MachineSet{ - ObjectMeta: v1.ObjectMeta{ - UID: uuid1, + machineOwnerRefs: []metav1.OwnerReference{ + { + Kind: "Other", + Name: "foo", + UID: uuid1, }, }, - owned: false, + expectedOwnerRef: nil, }, { - description: "not owned as UID values don't match", - machine: Machine{ - ObjectMeta: v1.ObjectMeta{ - OwnerReferences: []v1.OwnerReference{{ - Kind: "MachineSet", - Name: "foo", - UID: uuid2, - }}, + description: "not owned because no OwnerReference.Name", + machine: &unstructured.Unstructured{ + Object: map[string]interface{}{ + "kind": machineKind, + "apiVersion": "cluster.x-k8s.io/v1alpha3", + "metadata": map[string]interface{}{ + "name": "test", + "namespace": "default", + }, + "spec": map[string]interface{}{}, + "status": map[string]interface{}{}, }, }, - machineSet: MachineSet{ - TypeMeta: v1.TypeMeta{ - Kind: "MachineSet", - }, - ObjectMeta: v1.ObjectMeta{ - UID: uuid1, + machineOwnerRefs: []metav1.OwnerReference{ + { + Kind: machineSetKind, + UID: uuid1, }, }, - owned: false, + expectedOwnerRef: nil, }, { description: "owned as UID values match and same Kind and Name not empty", - machine: Machine{ - ObjectMeta: v1.ObjectMeta{ - OwnerReferences: []v1.OwnerReference{{ - Kind: "MachineSet", - Name: "foo", - UID: uuid1, - }}, + machine: &unstructured.Unstructured{ + Object: map[string]interface{}{ + "kind": machineKind, + "apiVersion": "cluster.x-k8s.io/v1alpha3", + "metadata": map[string]interface{}{ + "name": "test", + "namespace": "default", + }, + "spec": map[string]interface{}{}, + "status": map[string]interface{}{}, }, }, - machineSet: MachineSet{ - TypeMeta: v1.TypeMeta{ - Kind: "MachineSet", - }, - ObjectMeta: v1.ObjectMeta{ + machineOwnerRefs: []metav1.OwnerReference{ + { + Kind: machineSetKind, Name: "foo", - UID: uuid1, + UID: uuid2, }, }, - owned: true, + expectedOwnerRef: &metav1.OwnerReference{ + Kind: machineSetKind, + Name: "foo", + UID: uuid2, + }, }} { t.Run(tc.description, func(t *testing.T) { - owned := machineIsOwnedByMachineSet(&tc.machine, &tc.machineSet) - if tc.owned != owned { - t.Errorf("expected %t, got %t", tc.owned, owned) + tc.machine.SetOwnerReferences(tc.machineOwnerRefs) + + ownerRef := getOwnerForKind(tc.machine, machineSetKind) + if !reflect.DeepEqual(tc.expectedOwnerRef, ownerRef) { + t.Errorf("expected %v, got %v", tc.expectedOwnerRef, ownerRef) } }) } } -func TestUtilMachineSetMachineDeploymentOwnerRef(t *testing.T) { +func TestUtilMachineSetHasMachineDeploymentOwnerRef(t *testing.T) { for _, tc := range []struct { - description string - machineSet MachineSet - machineDeployment MachineDeployment - owned bool + description string + machineSet *unstructured.Unstructured + machineSetOwnerRefs []metav1.OwnerReference + owned bool }{{ - description: "machineset not owned as no owner references", - machineSet: MachineSet{}, - machineDeployment: MachineDeployment{}, - owned: false, + description: "machineset not owned as no owner references", + machineSet: &unstructured.Unstructured{}, + machineSetOwnerRefs: []metav1.OwnerReference{}, + owned: false, }, { description: "machineset not owned as ownerref not a MachineDeployment", - machineSet: MachineSet{ - ObjectMeta: v1.ObjectMeta{ - OwnerReferences: []v1.OwnerReference{{ - Kind: "Other", - }}, + machineSet: &unstructured.Unstructured{ + Object: map[string]interface{}{ + "kind": machineSetKind, + "apiVersion": "cluster.x-k8s.io/v1alpha3", + "metadata": map[string]interface{}{ + "name": "test", + "namespace": "default", + }, + "spec": map[string]interface{}{}, + "status": map[string]interface{}{}, }, }, - machineDeployment: MachineDeployment{}, - owned: false, + machineSetOwnerRefs: []metav1.OwnerReference{ + { + Kind: "Other", + }, + }, + owned: false, }, { description: "machineset owned as Kind matches and Name not empty", - machineSet: MachineSet{ - ObjectMeta: v1.ObjectMeta{ - OwnerReferences: []v1.OwnerReference{{ - Kind: "MachineDeployment", - Name: "foo", - }}, + machineSet: &unstructured.Unstructured{ + Object: map[string]interface{}{ + "kind": machineSetKind, + "apiVersion": "cluster.x-k8s.io/v1alpha3", + "metadata": map[string]interface{}{ + "name": "test", + "namespace": "default", + }, + "spec": map[string]interface{}{}, + "status": map[string]interface{}{}, }, }, - machineDeployment: MachineDeployment{ - TypeMeta: v1.TypeMeta{ - Kind: "MachineDeployment", - }, - ObjectMeta: v1.ObjectMeta{ + machineSetOwnerRefs: []metav1.OwnerReference{ + { + Kind: machineDeploymentKind, Name: "foo", }, }, owned: true, }} { t.Run(tc.description, func(t *testing.T) { - owned := machineSetHasMachineDeploymentOwnerRef(&tc.machineSet) + tc.machineSet.SetOwnerReferences(tc.machineSetOwnerRefs) + owned := machineSetHasMachineDeploymentOwnerRef(tc.machineSet) if tc.owned != owned { t.Errorf("expected %t, got %t", tc.owned, owned) } diff --git a/cluster-autoscaler/cloudprovider/clusterapi/machine_types.go b/cluster-autoscaler/cloudprovider/clusterapi/machine_types.go deleted file mode 100644 index db167b982c9f..000000000000 --- a/cluster-autoscaler/cloudprovider/clusterapi/machine_types.go +++ /dev/null @@ -1,92 +0,0 @@ -/* -Copyright 2020 The Kubernetes Authors. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package clusterapi - -import ( - corev1 "k8s.io/api/core/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" -) - -// Machine is the Schema for the machines API -type Machine struct { - metav1.TypeMeta `json:",inline"` - metav1.ObjectMeta `json:"metadata,omitempty"` - - Spec MachineSpec `json:"spec,omitempty"` - Status MachineStatus `json:"status,omitempty"` -} - -// MachineSpec defines the desired state of Machine -type MachineSpec struct { - // ObjectMeta will autopopulate the Node created. Use this to - // indicate what labels, annotations, name prefix, etc., should be used - // when creating the Node. - // +optional - metav1.ObjectMeta `json:"metadata,omitempty"` - - // Taints is the full, authoritative list of taints to apply to the corresponding - // Node. This list will overwrite any modifications made to the Node on - // an ongoing basis. - // +optional - Taints []corev1.Taint `json:"taints,omitempty"` - - // ProviderID is the identification ID of the machine provided by the provider. - // This field must match the provider ID as seen on the node object corresponding to this machine. - // This field is required by higher level consumers of cluster-api. Example use case is cluster autoscaler - // with cluster-api as provider. Clean-up login in the autoscaler compares machines v/s nodes to find out - // machines at provider which could not get registered as Kubernetes nodes. With cluster-api as a - // generic out-of-tree provider for autoscaler, this field is required by autoscaler to be - // able to have a provider view of the list of machines. Another list of nodes is queries from the k8s apiserver - // and then comparison is done to find out unregistered machines and are marked for delete. - // This field will be set by the actuators and consumed by higher level entities like autoscaler who will - // be interfacing with cluster-api as generic provider. - // +optional - ProviderID *string `json:"providerID,omitempty"` -} - -// MachineStatus defines the observed state of Machine -type MachineStatus struct { - // NodeRef will point to the corresponding Node if it exists. - // +optional - NodeRef *corev1.ObjectReference `json:"nodeRef,omitempty"` - - // FailureMessage will be set in the event that there is a terminal problem - // reconciling the Machine and will contain a more verbose string suitable - // for logging and human consumption. - // - // This field should not be set for transitive errors that a controller - // faces that are expected to be fixed automatically over - // time (like service outages), but instead indicate that something is - // fundamentally wrong with the Machine's spec or the configuration of - // the controller, and that manual intervention is required. Examples - // of terminal errors would be invalid combinations of settings in the - // spec, values that are unsupported by the controller, or the - // responsible controller itself being critically misconfigured. - // - // Any transient errors that occur during the reconciliation of Machines - // can be added as events to the Machine object and/or logged in the - // controller's output. - // +optional - FailureMessage *string `json:"failureMessage,omitempty"` -} - -// MachineList contains a list of Machine -type MachineList struct { - metav1.TypeMeta `json:",inline"` - metav1.ListMeta `json:"metadata,omitempty"` - Items []Machine `json:"items"` -} diff --git a/cluster-autoscaler/cloudprovider/clusterapi/machinedeployment_types.go b/cluster-autoscaler/cloudprovider/clusterapi/machinedeployment_types.go deleted file mode 100644 index d1525bf9d5a1..000000000000 --- a/cluster-autoscaler/cloudprovider/clusterapi/machinedeployment_types.go +++ /dev/null @@ -1,59 +0,0 @@ -/* -Copyright 2020 The Kubernetes Authors. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package clusterapi - -import ( - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" -) - -// MachineDeploymentSpec is the internal autoscaler Schema for MachineDeploymentSpec -type MachineDeploymentSpec struct { - // Number of desired machines. Defaults to 1. - // This is a pointer to distinguish between explicit zero and not specified. - Replicas *int32 `json:"replicas,omitempty"` - - // Label selector for machines. Existing MachineSets whose machines are - // selected by this will be the ones affected by this deployment. - // It must match the machine template's labels. - Selector metav1.LabelSelector `json:"selector"` - - // Template describes the machines that will be created. - Template MachineTemplateSpec `json:"template"` -} - -// MachineDeploymentStatus is the internal autoscaler Schema for MachineDeploymentStatus -type MachineDeploymentStatus struct { - // Number of desired machines. Defaults to 1. - // This is a pointer to distinguish between explicit zero and not specified. - Replicas int32 `json:"replicas,omitempty"` -} - -// MachineDeployment is the internal autoscaler Schema for MachineDeployment -type MachineDeployment struct { - metav1.TypeMeta `json:",inline"` - metav1.ObjectMeta `json:"metadata,omitempty"` - - Spec MachineDeploymentSpec `json:"spec,omitempty"` - Status MachineDeploymentStatus `json:"status,omitempty"` -} - -// MachineDeploymentList is the internal autoscaler Schema for MachineDeploymentList -type MachineDeploymentList struct { - metav1.TypeMeta `json:",inline"` - metav1.ListMeta `json:"metadata,omitempty"` - Items []MachineDeployment `json:"items"` -} diff --git a/cluster-autoscaler/cloudprovider/clusterapi/machineset_types.go b/cluster-autoscaler/cloudprovider/clusterapi/machineset_types.go deleted file mode 100644 index 2f2d7ddb895c..000000000000 --- a/cluster-autoscaler/cloudprovider/clusterapi/machineset_types.go +++ /dev/null @@ -1,81 +0,0 @@ -/* -Copyright 2020 The Kubernetes Authors. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package clusterapi - -import ( - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" -) - -// MachineSet is the internal autoscaler Schema for machineSets -type MachineSet struct { - metav1.TypeMeta `json:",inline"` - metav1.ObjectMeta `json:"metadata,omitempty"` - - Spec MachineSetSpec `json:"spec,omitempty"` - Status MachineSetStatus `json:"status,omitempty"` -} - -// MachineSetSpec is the internal autoscaler Schema for MachineSetSpec -type MachineSetSpec struct { - // Replicas is the number of desired replicas. - // This is a pointer to distinguish between explicit zero and unspecified. - // Defaults to 1. - // +optional - Replicas *int32 `json:"replicas,omitempty"` - - // MinReadySeconds is the minimum number of seconds for which a newly created machine should be ready. - // Defaults to 0 (machine will be considered available as soon as it is ready) - // +optional - MinReadySeconds int32 `json:"minReadySeconds,omitempty"` - - // Selector is a label query over machines that should match the replica count. - // Label keys and values that must match in order to be controlled by this MachineSet. - // It must match the machine template's labels. - // More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/labels/#label-selectors - Selector metav1.LabelSelector `json:"selector"` - - // Template is the object that describes the machine that will be created if - // insufficient replicas are detected. - // +optional - Template MachineTemplateSpec `json:"template,omitempty"` -} - -// MachineTemplateSpec is the internal autoscaler Schema for MachineTemplateSpec -type MachineTemplateSpec struct { - // Standard object's metadata. - // More info: https://git.k8s.io/community/contributors/devel/api-conventions.md#metadata - // +optional - metav1.ObjectMeta `json:"metadata,omitempty"` - - // Specification of the desired behavior of the machine. - // More info: https://git.k8s.io/community/contributors/devel/api-conventions.md#spec-and-status - // +optional - Spec MachineSpec `json:"spec,omitempty"` -} - -// MachineSetStatus is the internal autoscaler Schema for MachineSetStatus -type MachineSetStatus struct { - // Replicas is the most recently observed number of replicas. - Replicas int32 `json:"replicas"` -} - -// MachineSetList is the internal autoscaler Schema for MachineSetList -type MachineSetList struct { - metav1.TypeMeta `json:",inline"` - metav1.ListMeta `json:"metadata,omitempty"` - Items []MachineSet `json:"items"` -} diff --git a/cluster-autoscaler/cloudprovider/clusterapi/zz_generated.deepcopy.go b/cluster-autoscaler/cloudprovider/clusterapi/zz_generated.deepcopy.go deleted file mode 100644 index 9b702a86260a..000000000000 --- a/cluster-autoscaler/cloudprovider/clusterapi/zz_generated.deepcopy.go +++ /dev/null @@ -1,360 +0,0 @@ -// +build !ignore_autogenerated - -/* -Copyright 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. -*/ - -// Code generated by main. DO NOT EDIT. - -package clusterapi - -import ( - v1 "k8s.io/api/core/v1" - "k8s.io/apimachinery/pkg/runtime" -) - -// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. -func (in *Machine) DeepCopyInto(out *Machine) { - *out = *in - out.TypeMeta = in.TypeMeta - in.ObjectMeta.DeepCopyInto(&out.ObjectMeta) - in.Spec.DeepCopyInto(&out.Spec) - in.Status.DeepCopyInto(&out.Status) - return -} - -// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new Machine. -func (in *Machine) DeepCopy() *Machine { - if in == nil { - return nil - } - out := new(Machine) - in.DeepCopyInto(out) - return out -} - -// DeepCopyObject is an autogenerated deepcopy function, copying the receiver, creating a new runtime.Object. -func (in *Machine) DeepCopyObject() runtime.Object { - if c := in.DeepCopy(); c != nil { - return c - } - return nil -} - -// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. -func (in *MachineDeployment) DeepCopyInto(out *MachineDeployment) { - *out = *in - out.TypeMeta = in.TypeMeta - in.ObjectMeta.DeepCopyInto(&out.ObjectMeta) - in.Spec.DeepCopyInto(&out.Spec) - out.Status = in.Status - return -} - -// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new MachineDeployment. -func (in *MachineDeployment) DeepCopy() *MachineDeployment { - if in == nil { - return nil - } - out := new(MachineDeployment) - in.DeepCopyInto(out) - return out -} - -// DeepCopyObject is an autogenerated deepcopy function, copying the receiver, creating a new runtime.Object. -func (in *MachineDeployment) DeepCopyObject() runtime.Object { - if c := in.DeepCopy(); c != nil { - return c - } - return nil -} - -// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. -func (in *MachineDeploymentList) DeepCopyInto(out *MachineDeploymentList) { - *out = *in - out.TypeMeta = in.TypeMeta - out.ListMeta = in.ListMeta - if in.Items != nil { - in, out := &in.Items, &out.Items - *out = make([]MachineDeployment, len(*in)) - for i := range *in { - (*in)[i].DeepCopyInto(&(*out)[i]) - } - } - return -} - -// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new MachineDeploymentList. -func (in *MachineDeploymentList) DeepCopy() *MachineDeploymentList { - if in == nil { - return nil - } - out := new(MachineDeploymentList) - in.DeepCopyInto(out) - return out -} - -// DeepCopyObject is an autogenerated deepcopy function, copying the receiver, creating a new runtime.Object. -func (in *MachineDeploymentList) DeepCopyObject() runtime.Object { - if c := in.DeepCopy(); c != nil { - return c - } - return nil -} - -// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. -func (in *MachineDeploymentSpec) DeepCopyInto(out *MachineDeploymentSpec) { - *out = *in - if in.Replicas != nil { - in, out := &in.Replicas, &out.Replicas - *out = new(int32) - **out = **in - } - in.Selector.DeepCopyInto(&out.Selector) - in.Template.DeepCopyInto(&out.Template) - return -} - -// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new MachineDeploymentSpec. -func (in *MachineDeploymentSpec) DeepCopy() *MachineDeploymentSpec { - if in == nil { - return nil - } - out := new(MachineDeploymentSpec) - in.DeepCopyInto(out) - return out -} - -// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. -func (in *MachineDeploymentStatus) DeepCopyInto(out *MachineDeploymentStatus) { - *out = *in - return -} - -// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new MachineDeploymentStatus. -func (in *MachineDeploymentStatus) DeepCopy() *MachineDeploymentStatus { - if in == nil { - return nil - } - out := new(MachineDeploymentStatus) - in.DeepCopyInto(out) - return out -} - -// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. -func (in *MachineList) DeepCopyInto(out *MachineList) { - *out = *in - out.TypeMeta = in.TypeMeta - out.ListMeta = in.ListMeta - if in.Items != nil { - in, out := &in.Items, &out.Items - *out = make([]Machine, len(*in)) - for i := range *in { - (*in)[i].DeepCopyInto(&(*out)[i]) - } - } - return -} - -// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new MachineList. -func (in *MachineList) DeepCopy() *MachineList { - if in == nil { - return nil - } - out := new(MachineList) - in.DeepCopyInto(out) - return out -} - -// DeepCopyObject is an autogenerated deepcopy function, copying the receiver, creating a new runtime.Object. -func (in *MachineList) DeepCopyObject() runtime.Object { - if c := in.DeepCopy(); c != nil { - return c - } - return nil -} - -// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. -func (in *MachineSet) DeepCopyInto(out *MachineSet) { - *out = *in - out.TypeMeta = in.TypeMeta - in.ObjectMeta.DeepCopyInto(&out.ObjectMeta) - in.Spec.DeepCopyInto(&out.Spec) - in.Status.DeepCopyInto(&out.Status) - return -} - -// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new MachineSet. -func (in *MachineSet) DeepCopy() *MachineSet { - if in == nil { - return nil - } - out := new(MachineSet) - in.DeepCopyInto(out) - return out -} - -// DeepCopyObject is an autogenerated deepcopy function, copying the receiver, creating a new runtime.Object. -func (in *MachineSet) DeepCopyObject() runtime.Object { - if c := in.DeepCopy(); c != nil { - return c - } - return nil -} - -// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. -func (in *MachineSetList) DeepCopyInto(out *MachineSetList) { - *out = *in - out.TypeMeta = in.TypeMeta - out.ListMeta = in.ListMeta - if in.Items != nil { - in, out := &in.Items, &out.Items - *out = make([]MachineSet, len(*in)) - for i := range *in { - (*in)[i].DeepCopyInto(&(*out)[i]) - } - } - return -} - -// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new MachineSetList. -func (in *MachineSetList) DeepCopy() *MachineSetList { - if in == nil { - return nil - } - out := new(MachineSetList) - in.DeepCopyInto(out) - return out -} - -// DeepCopyObject is an autogenerated deepcopy function, copying the receiver, creating a new runtime.Object. -func (in *MachineSetList) DeepCopyObject() runtime.Object { - if c := in.DeepCopy(); c != nil { - return c - } - return nil -} - -// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. -func (in *MachineSetSpec) DeepCopyInto(out *MachineSetSpec) { - *out = *in - if in.Replicas != nil { - in, out := &in.Replicas, &out.Replicas - *out = new(int32) - **out = **in - } - in.Selector.DeepCopyInto(&out.Selector) - in.Template.DeepCopyInto(&out.Template) - return -} - -// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new MachineSetSpec. -func (in *MachineSetSpec) DeepCopy() *MachineSetSpec { - if in == nil { - return nil - } - out := new(MachineSetSpec) - in.DeepCopyInto(out) - return out -} - -// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. -func (in *MachineSetStatus) DeepCopyInto(out *MachineSetStatus) { - *out = *in - return -} - -// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new MachineSetStatus. -func (in *MachineSetStatus) DeepCopy() *MachineSetStatus { - if in == nil { - return nil - } - out := new(MachineSetStatus) - in.DeepCopyInto(out) - return out -} - -// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. -func (in *MachineSpec) DeepCopyInto(out *MachineSpec) { - *out = *in - in.ObjectMeta.DeepCopyInto(&out.ObjectMeta) - if in.Taints != nil { - in, out := &in.Taints, &out.Taints - *out = make([]v1.Taint, len(*in)) - for i := range *in { - (*in)[i].DeepCopyInto(&(*out)[i]) - } - } - if in.ProviderID != nil { - in, out := &in.ProviderID, &out.ProviderID - *out = new(string) - **out = **in - } - return -} - -// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new MachineSpec. -func (in *MachineSpec) DeepCopy() *MachineSpec { - if in == nil { - return nil - } - out := new(MachineSpec) - in.DeepCopyInto(out) - return out -} - -// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. -func (in *MachineStatus) DeepCopyInto(out *MachineStatus) { - *out = *in - if in.FailureMessage != nil { - in, out := &in.FailureMessage, &out.FailureMessage - *out = new(string) - **out = **in - } - if in.NodeRef != nil { - in, out := &in.NodeRef, &out.NodeRef - *out = new(v1.ObjectReference) - **out = **in - } - return -} - -// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new MachineStatus. -func (in *MachineStatus) DeepCopy() *MachineStatus { - if in == nil { - return nil - } - out := new(MachineStatus) - in.DeepCopyInto(out) - return out -} - -// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. -func (in *MachineTemplateSpec) DeepCopyInto(out *MachineTemplateSpec) { - *out = *in - in.ObjectMeta.DeepCopyInto(&out.ObjectMeta) - in.Spec.DeepCopyInto(&out.Spec) - return -} - -// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new MachineTemplateSpec. -func (in *MachineTemplateSpec) DeepCopy() *MachineTemplateSpec { - if in == nil { - return nil - } - out := new(MachineTemplateSpec) - in.DeepCopyInto(out) - return out -}