From 9738adbb51c9ddff1925776ec058d6de0a30b578 Mon Sep 17 00:00:00 2001 From: Joel Speed Date: Wed, 15 Apr 2020 13:51:55 +0100 Subject: [PATCH] Do not normalize Node IDs outside of CAPI provider --- .../cloudprovider/clusterapi/clusterapi_controller_test.go | 4 ++-- .../cloudprovider/clusterapi/clusterapi_nodegroup.go | 6 +++++- .../cloudprovider/clusterapi/clusterapi_nodegroup_test.go | 6 +++--- 3 files changed, 10 insertions(+), 6 deletions(-) diff --git a/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_controller_test.go b/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_controller_test.go index 9fddba979f50..5cb405593999 100644 --- a/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_controller_test.go +++ b/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_controller_test.go @@ -965,7 +965,7 @@ func TestControllerMachineSetNodeNamesUsingProviderID(t *testing.T) { }) for i := range testConfig.nodes { - if nodeNames[i].Id != string(normalizedProviderString(testConfig.nodes[i].Spec.ProviderID)) { + if nodeNames[i].Id != testConfig.nodes[i].Spec.ProviderID { t.Fatalf("expected %q, got %q", testConfig.nodes[i].Spec.ProviderID, nodeNames[i].Id) } } @@ -1013,7 +1013,7 @@ func TestControllerMachineSetNodeNamesUsingStatusNodeRefName(t *testing.T) { }) for i := range testConfig.nodes { - if nodeNames[i].Id != string(normalizedProviderString(testConfig.nodes[i].Spec.ProviderID)) { + if nodeNames[i].Id != testConfig.nodes[i].Spec.ProviderID { t.Fatalf("expected %q, got %q", testConfig.nodes[i].Spec.ProviderID, nodeNames[i].Id) } } diff --git a/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_nodegroup.go b/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_nodegroup.go index 89731eee11ed..8c13da3cb296 100644 --- a/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_nodegroup.go +++ b/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_nodegroup.go @@ -195,10 +195,14 @@ func (ng *nodegroup) Nodes() ([]cloudprovider.Instance, error) { return nil, err } + // Nodes do not have normalized IDs, so do not normalize the ID here. + // 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[i] = cloudprovider.Instance{ - Id: string(normalizedProviderString(nodes[i])), + Id: nodes[i], } } diff --git a/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_nodegroup_test.go b/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_nodegroup_test.go index 74da6b97f182..1d3d4ed7468e 100644 --- a/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_nodegroup_test.go +++ b/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_nodegroup_test.go @@ -659,7 +659,7 @@ func TestNodeGroupDeleteNodes(t *testing.T) { }) for i := 0; i < len(nodeNames); i++ { - if nodeNames[i].Id != string(normalizedProviderString(testConfig.nodes[i].Spec.ProviderID)) { + if nodeNames[i].Id != testConfig.nodes[i].Spec.ProviderID { t.Fatalf("expected %q, got %q", testConfig.nodes[i].Spec.ProviderID, nodeNames[i].Id) } } @@ -844,7 +844,7 @@ func TestNodeGroupDeleteNodesTwice(t *testing.T) { }) for i := 0; i < len(nodeNames); i++ { - if nodeNames[i].Id != string(normalizedProviderString(testConfig.nodes[i].Spec.ProviderID)) { + if nodeNames[i].Id != testConfig.nodes[i].Spec.ProviderID { t.Fatalf("expected %q, got %q", testConfig.nodes[i].Spec.ProviderID, nodeNames[i].Id) } } @@ -998,7 +998,7 @@ func TestNodeGroupWithFailedMachine(t *testing.T) { nodeIndex = i } - if nodeNames[i].Id != string(normalizedProviderString(testConfig.nodes[nodeIndex].Spec.ProviderID)) { + if nodeNames[i].Id != testConfig.nodes[nodeIndex].Spec.ProviderID { t.Fatalf("expected %q, got %q", testConfig.nodes[nodeIndex].Spec.ProviderID, nodeNames[i].Id) } }