diff --git a/cluster-autoscaler/cloudprovider/openshiftmachineapi/machineapi_controller.go b/cluster-autoscaler/cloudprovider/openshiftmachineapi/machineapi_controller.go index 6f3712af1642..50392ddcbd47 100644 --- a/cluster-autoscaler/cloudprovider/openshiftmachineapi/machineapi_controller.go +++ b/cluster-autoscaler/cloudprovider/openshiftmachineapi/machineapi_controller.go @@ -33,7 +33,7 @@ import ( ) const ( - nodeProviderIDIndex = "openshiftmachineapi-nodeProviderIDIndex" + machineProviderIDIndex = "openshiftmachineapi-machineProviderIDIndex" ) // machineController watches for Nodes, Machines, MachineSets and @@ -53,9 +53,12 @@ type machineController struct { type machineSetFilterFunc func(machineSet *v1beta1.MachineSet) error -func indexNodeByNodeProviderID(obj interface{}) ([]string, error) { - if node, ok := obj.(*apiv1.Node); ok { - return []string{node.Spec.ProviderID}, nil +func indexMachineByProviderID(obj interface{}) ([]string, error) { + if machine, ok := obj.(*v1beta1.Machine); ok { + if machine.Spec.ProviderID != nil && *machine.Spec.ProviderID != "" { + return []string{*machine.Spec.ProviderID}, nil + } + return []string{}, nil } return []string{}, nil } @@ -155,7 +158,7 @@ func (c *machineController) run(stopCh <-chan struct{}) error { // node.Spec.ProviderID cannot be found or if the node has no machine // annotation. A DeepCopy() of the object is returned on success. func (c *machineController) findMachineByNodeProviderID(node *apiv1.Node) (*v1beta1.Machine, error) { - objs, err := c.nodeInformer.GetIndexer().ByIndex(nodeProviderIDIndex, node.Spec.ProviderID) + objs, err := c.machineInformer.Informer().GetIndexer().ByIndex(machineProviderIDIndex, node.Spec.ProviderID) if err != nil { return nil, err } @@ -167,16 +170,12 @@ func (c *machineController) findMachineByNodeProviderID(node *apiv1.Node) (*v1be return nil, fmt.Errorf("internal error; expected len==1, got %v", n) } - node, ok := objs[0].(*apiv1.Node) + machine, ok := objs[0].(*v1beta1.Machine) if !ok { - return nil, fmt.Errorf("internal error; unexpected type %T", node) - } - - if machineName, found := node.Annotations[machineAnnotationKey]; found { - return c.findMachine(machineName) + return nil, fmt.Errorf("internal error; unexpected type %T", machine) } - return nil, nil + return machine.DeepCopy(), nil } // findNodeByNodeName find the Node object keyed by node.Name. Returns @@ -247,12 +246,10 @@ func newMachineController( nodeInformer := kubeInformerFactory.Core().V1().Nodes().Informer() nodeInformer.AddEventHandler(cache.ResourceEventHandlerFuncs{}) - indexerFuncs := cache.Indexers{ - nodeProviderIDIndex: indexNodeByNodeProviderID, - } - - if err := nodeInformer.GetIndexer().AddIndexers(indexerFuncs); err != nil { - return nil, fmt.Errorf("cannot add indexers: %v", err) + if err := machineInformer.Informer().GetIndexer().AddIndexers(cache.Indexers{ + machineProviderIDIndex: indexMachineByProviderID, + }); err != nil { + return nil, fmt.Errorf("cannot add machine indexer: %v", err) } return &machineController{ diff --git a/cluster-autoscaler/cloudprovider/openshiftmachineapi/machineapi_controller_test.go b/cluster-autoscaler/cloudprovider/openshiftmachineapi/machineapi_controller_test.go index f821926ee974..b43833b4209d 100644 --- a/cluster-autoscaler/cloudprovider/openshiftmachineapi/machineapi_controller_test.go +++ b/cluster-autoscaler/cloudprovider/openshiftmachineapi/machineapi_controller_test.go @@ -32,6 +32,7 @@ import ( "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" fakekube "k8s.io/client-go/kubernetes/fake" + "k8s.io/utils/pointer" ) type testControllerShutdownFunc func() @@ -200,9 +201,6 @@ func makeLinkedNodeAndMachine(i int, namespace string, owner v1.OwnerReference) }, ObjectMeta: v1.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), - }, }, Spec: apiv1.NodeSpec{ ProviderID: fmt.Sprintf("%s-%s-nodeid-%d", namespace, owner.Name, i), @@ -222,6 +220,9 @@ func makeLinkedNodeAndMachine(i int, namespace string, owner v1.OwnerReference) UID: owner.UID, }}, }, + Spec: v1beta1.MachineSpec{ + ProviderID: pointer.StringPtr(fmt.Sprintf("%s-%s-nodeid-%d", namespace, owner.Name, i)), + }, Status: v1beta1.MachineStatus{ NodeRef: &apiv1.ObjectReference{ Kind: node.Kind, @@ -406,7 +407,8 @@ func TestControllerFindMachineByNodeProviderID(t *testing.T) { defer stop() // Test #1: Verify node can be found because it has a - // ProviderID value and a machine annotation. + // ProviderID value and the machine has a matching ProviderID + // value. machine, err := controller.findMachineByNodeProviderID(testConfig.nodes[0]) if err != nil { t.Fatalf("unexpected error: %v", err) @@ -418,7 +420,7 @@ func TestControllerFindMachineByNodeProviderID(t *testing.T) { t.Fatalf("expected machines to be equal - expected %+v, got %+v", testConfig.machines[0], machine) } - // Test #2: Verify node is not found if it has a non-existent ProviderID + // Test #2: Verify machine is not found if node has no ProviderID node := testConfig.nodes[0].DeepCopy() node.Spec.ProviderID = "" nonExistentMachine, err := controller.findMachineByNodeProviderID(node) @@ -429,11 +431,11 @@ func TestControllerFindMachineByNodeProviderID(t *testing.T) { t.Fatal("expected find to fail") } - // Test #3: Verify node is not found if the stored object has - // no "machine" annotation - node = testConfig.nodes[0].DeepCopy() - delete(node.Annotations, machineAnnotationKey) - if err := controller.nodeInformer.GetStore().Update(node); err != nil { + // Test #3: Verify machine is not found if underlying machine + // has no ProviderID value. + machine = testConfig.machines[0].DeepCopy() + machine.Spec.ProviderID = nil + if err := controller.machineInformer.Informer().GetStore().Update(machine); err != nil { t.Fatalf("unexpected error updating node, got %v", err) } nonExistentMachine, err = controller.findMachineByNodeProviderID(testConfig.nodes[0]) diff --git a/cluster-autoscaler/cloudprovider/openshiftmachineapi/machineapi_nodegroup.go b/cluster-autoscaler/cloudprovider/openshiftmachineapi/machineapi_nodegroup.go index dca51e4f554c..c948c24400a3 100644 --- a/cluster-autoscaler/cloudprovider/openshiftmachineapi/machineapi_nodegroup.go +++ b/cluster-autoscaler/cloudprovider/openshiftmachineapi/machineapi_nodegroup.go @@ -29,7 +29,6 @@ import ( const ( machineDeleteAnnotationKey = "machine.openshift.io/cluster-api-delete-machine" - machineAnnotationKey = "machine.openshift.io/machine" debugFormat = "%s (min: %d, max: %d, replicas: %d)" )