Skip to content

Commit db87743

Browse files
nicolehanjingnicolehanjing
committed
cloud node controller: handle empty providerID from getProviderID
1 parent 0a14265 commit db87743

File tree

2 files changed

+208
-6
lines changed

2 files changed

+208
-6
lines changed

‎staging/src/k8s.io/cloud-provider/controllers/node/node_controller.go‎

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -474,8 +474,12 @@ func (cnc *CloudNodeController) getNodeModifiersFromCloudProvider(
474474
) ([]nodeModifier, error) {
475475

476476
var nodeModifiers []nodeModifier
477-
if node.Spec.ProviderID == "" && providerID != "" {
478-
nodeModifiers = append(nodeModifiers, func(n *v1.Node) { n.Spec.ProviderID = providerID })
477+
if node.Spec.ProviderID == "" {
478+
if providerID != "" {
479+
nodeModifiers = append(nodeModifiers, func(n *v1.Node) { n.Spec.ProviderID = providerID })
480+
} else if instanceMeta.ProviderID != "" {
481+
nodeModifiers = append(nodeModifiers, func(n *v1.Node) { n.Spec.ProviderID = instanceMeta.ProviderID })
482+
}
479483
}
480484

481485
// If user provided an IP address, ensure that IP address is found
@@ -527,6 +531,11 @@ func (cnc *CloudNodeController) getProviderID(ctx context.Context, node *v1.Node
527531
return node.Spec.ProviderID, nil
528532
}
529533

534+
if _, ok := cnc.cloud.InstancesV2(); ok {
535+
// We don't need providerID when we call InstanceMetadata for InstancesV2
536+
return "", nil
537+
}
538+
530539
providerID, err := cloudprovider.GetInstanceProviderID(ctx, cnc.cloud, types.NodeName(node.Name))
531540
if err == cloudprovider.NotImplemented {
532541
// if the cloud provider being used does not support provider IDs,

‎staging/src/k8s.io/cloud-provider/controllers/node/node_controller_test.go‎

Lines changed: 197 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -764,6 +764,7 @@ func Test_syncNode(t *testing.T) {
764764
Effect: v1.TaintEffectNoSchedule,
765765
},
766766
},
767+
ProviderID: "fake://12345",
767768
},
768769
},
769770
updatedNode: &v1.Node{
@@ -924,9 +925,6 @@ func Test_syncNode(t *testing.T) {
924925
},
925926
},
926927
},
927-
Spec: v1.NodeSpec{
928-
ProviderID: "aws://12345",
929-
},
930928
},
931929
},
932930
{
@@ -991,7 +989,6 @@ func Test_syncNode(t *testing.T) {
991989
CreationTimestamp: metav1.Date(2012, 1, 1, 0, 0, 0, 0, time.UTC),
992990
},
993991
Spec: v1.NodeSpec{
994-
ProviderID: "fake://12345",
995992
Taints: []v1.Taint{
996993
{
997994
Key: "ImproveCoverageTaint",
@@ -1882,3 +1879,199 @@ func TestNodeAddressesNotUpdate(t *testing.T) {
18821879
t.Errorf("Node addresses should not be updated")
18831880
}
18841881
}
1882+
1883+
func TestGetProviderID(t *testing.T) {
1884+
tests := []struct {
1885+
name string
1886+
fakeCloud *fakecloud.Cloud
1887+
existingNode *v1.Node
1888+
expectedProviderID string
1889+
}{
1890+
{
1891+
name: "node initialized with provider ID",
1892+
fakeCloud: &fakecloud.Cloud{
1893+
EnableInstancesV2: false,
1894+
InstanceTypes: map[types.NodeName]string{
1895+
types.NodeName("node0"): "t1.micro",
1896+
},
1897+
ExtID: map[types.NodeName]string{
1898+
types.NodeName("node0"): "12345",
1899+
},
1900+
Addresses: []v1.NodeAddress{
1901+
{
1902+
Type: v1.NodeHostName,
1903+
Address: "node0.cloud.internal",
1904+
},
1905+
{
1906+
Type: v1.NodeInternalIP,
1907+
Address: "10.0.0.1",
1908+
},
1909+
{
1910+
Type: v1.NodeExternalIP,
1911+
Address: "132.143.154.163",
1912+
},
1913+
},
1914+
ErrByProviderID: nil,
1915+
Err: nil,
1916+
},
1917+
existingNode: &v1.Node{
1918+
ObjectMeta: metav1.ObjectMeta{
1919+
Name: "node0",
1920+
CreationTimestamp: metav1.Date(2012, 1, 1, 0, 0, 0, 0, time.UTC),
1921+
},
1922+
Spec: v1.NodeSpec{
1923+
Taints: []v1.Taint{
1924+
{
1925+
Key: cloudproviderapi.TaintExternalCloudProvider,
1926+
Value: "true",
1927+
Effect: v1.TaintEffectNoSchedule,
1928+
},
1929+
},
1930+
ProviderID: "fake://12345",
1931+
},
1932+
},
1933+
expectedProviderID: "fake://12345",
1934+
},
1935+
{
1936+
name: "cloud implemented with Instances (without providerID)",
1937+
fakeCloud: &fakecloud.Cloud{
1938+
EnableInstancesV2: false,
1939+
InstanceTypes: map[types.NodeName]string{
1940+
types.NodeName("node0"): "t1.micro",
1941+
types.NodeName("fake://12345"): "t1.micro",
1942+
},
1943+
ExtID: map[types.NodeName]string{
1944+
types.NodeName("node0"): "12345",
1945+
},
1946+
Addresses: []v1.NodeAddress{
1947+
{
1948+
Type: v1.NodeHostName,
1949+
Address: "node0.cloud.internal",
1950+
},
1951+
{
1952+
Type: v1.NodeInternalIP,
1953+
Address: "10.0.0.1",
1954+
},
1955+
{
1956+
Type: v1.NodeExternalIP,
1957+
Address: "132.143.154.163",
1958+
},
1959+
},
1960+
Err: nil,
1961+
},
1962+
existingNode: &v1.Node{
1963+
ObjectMeta: metav1.ObjectMeta{
1964+
Name: "node0",
1965+
CreationTimestamp: metav1.Date(2012, 1, 1, 0, 0, 0, 0, time.UTC),
1966+
},
1967+
},
1968+
expectedProviderID: "fake://12345",
1969+
},
1970+
{
1971+
name: "cloud implemented with InstancesV2 (with providerID)",
1972+
fakeCloud: &fakecloud.Cloud{
1973+
EnableInstancesV2: true,
1974+
InstanceTypes: map[types.NodeName]string{
1975+
types.NodeName("node0"): "t1.micro",
1976+
types.NodeName("fake://12345"): "t1.micro",
1977+
},
1978+
ExtID: map[types.NodeName]string{
1979+
types.NodeName("node0"): "12345",
1980+
},
1981+
Addresses: []v1.NodeAddress{
1982+
{
1983+
Type: v1.NodeHostName,
1984+
Address: "node0.cloud.internal",
1985+
},
1986+
{
1987+
Type: v1.NodeInternalIP,
1988+
Address: "10.0.0.1",
1989+
},
1990+
{
1991+
Type: v1.NodeExternalIP,
1992+
Address: "132.143.154.163",
1993+
},
1994+
},
1995+
Err: nil,
1996+
},
1997+
existingNode: &v1.Node{
1998+
ObjectMeta: metav1.ObjectMeta{
1999+
Name: "node0",
2000+
CreationTimestamp: metav1.Date(2012, 1, 1, 0, 0, 0, 0, time.UTC),
2001+
},
2002+
Spec: v1.NodeSpec{
2003+
Taints: []v1.Taint{
2004+
{
2005+
Key: cloudproviderapi.TaintExternalCloudProvider,
2006+
Value: "true",
2007+
Effect: v1.TaintEffectNoSchedule,
2008+
},
2009+
},
2010+
ProviderID: "fake://12345",
2011+
},
2012+
},
2013+
expectedProviderID: "fake://12345",
2014+
},
2015+
{
2016+
name: "cloud implemented with InstancesV2 (without providerID)",
2017+
fakeCloud: &fakecloud.Cloud{
2018+
EnableInstancesV2: true,
2019+
InstanceTypes: map[types.NodeName]string{
2020+
types.NodeName("node0"): "t1.micro",
2021+
types.NodeName("fake://12345"): "t1.micro",
2022+
},
2023+
ExtID: map[types.NodeName]string{
2024+
types.NodeName("node0"): "12345",
2025+
},
2026+
Addresses: []v1.NodeAddress{
2027+
{
2028+
Type: v1.NodeHostName,
2029+
Address: "node0.cloud.internal",
2030+
},
2031+
{
2032+
Type: v1.NodeInternalIP,
2033+
Address: "10.0.0.1",
2034+
},
2035+
{
2036+
Type: v1.NodeExternalIP,
2037+
Address: "132.143.154.163",
2038+
},
2039+
},
2040+
Err: nil,
2041+
},
2042+
existingNode: &v1.Node{
2043+
ObjectMeta: metav1.ObjectMeta{
2044+
Name: "node0",
2045+
CreationTimestamp: metav1.Date(2012, 1, 1, 0, 0, 0, 0, time.UTC),
2046+
},
2047+
Spec: v1.NodeSpec{
2048+
Taints: []v1.Taint{
2049+
{
2050+
Key: cloudproviderapi.TaintExternalCloudProvider,
2051+
Value: "true",
2052+
Effect: v1.TaintEffectNoSchedule,
2053+
},
2054+
},
2055+
},
2056+
},
2057+
expectedProviderID: "",
2058+
},
2059+
}
2060+
2061+
for _, test := range tests {
2062+
t.Run(test.name, func(t *testing.T) {
2063+
cloudNodeController := &CloudNodeController{
2064+
cloud: test.fakeCloud,
2065+
}
2066+
2067+
providerID, err := cloudNodeController.getProviderID(context.TODO(), test.existingNode)
2068+
if err != nil {
2069+
t.Fatalf("error getting provider ID: %v", err)
2070+
}
2071+
2072+
if !cmp.Equal(providerID, test.expectedProviderID) {
2073+
t.Errorf("unexpected providerID %s", cmp.Diff(providerID, test.expectedProviderID))
2074+
}
2075+
})
2076+
}
2077+
}

0 commit comments

Comments
 (0)