Repository: kubernetes-sigs/cloud-provider-azure
Pull request #4804 (2023-10-16 07:07:06)

[release-1.28] report vmPowerStateUnknown when vmssVM intanceView is nil
This is an automated cherry-pick of #4798

/assign jwtty

```release-note
Fix the potential error that a deleting vmss instance is considered as shutdown.
```


Patch:
diff --git a/pkg/provider/azure.go b/pkg/provider/azure.go
index 22dc4b427..38d46c1a4 100644
--- a/pkg/provider/azure.go
+++ b/pkg/provider/azure.go
@@ -1357,7 +1357,7 @@ func (az *Cloud) updateNodeTaint(node *v1.Node) {
 				klog.Errorf("failed to add taint %s to the node %s", v1.TaintNodeOutOfService, node.Name)
 			}
 		} else {
-			klog.V(2).Infof("node %s is not ready but node shutdown taint is not added, skip adding node out-of-service taint", node.Name)
+			klog.V(2).Infof("node %s is not ready but either shutdown taint is missing or out-of-service taint is already added, skip adding node out-of-service taint", node.Name)
 		}
 	}
 }
diff --git a/pkg/provider/azure_instances.go b/pkg/provider/azure_instances.go
index f14fe57d6..656a2b873 100644
--- a/pkg/provider/azure_instances.go
+++ b/pkg/provider/azure_instances.go
@@ -37,6 +37,7 @@ const (
 	vmPowerStateStopped      = "stopped"
 	vmPowerStateDeallocated  = "deallocated"
 	vmPowerStateDeallocating = "deallocating"
+	vmPowerStateUnknown      = "unknown"
 
 	// nodeNameEnvironmentName is the environment variable name for getting node name.
 	// It is only used for out-of-tree cloud provider.
diff --git a/pkg/provider/azure_standard.go b/pkg/provider/azure_standard.go
index 9f5dd0594..17ea57d4e 100644
--- a/pkg/provider/azure_standard.go
+++ b/pkg/provider/azure_standard.go
@@ -515,8 +515,8 @@ func (as *availabilitySet) GetPowerStatusByNodeName(name string) (powerState str
 	}
 
 	// vm.InstanceView or vm.InstanceView.Statuses are nil when the VM is under deleting.
-	klog.V(3).Infof("InstanceView for node %q is nil, assuming it's stopped", name)
-	return vmPowerStateStopped, nil
+	klog.V(3).Infof("InstanceView for node %q is nil, assuming it's deleting", name)
+	return vmPowerStateUnknown, nil
 }
 
 // GetProvisioningStateByNodeName returns the provisioningState for the specified node.
diff --git a/pkg/provider/azure_vmss.go b/pkg/provider/azure_vmss.go
index 24b29ac81..5ab2b996b 100644
--- a/pkg/provider/azure_vmss.go
+++ b/pkg/provider/azure_vmss.go
@@ -293,8 +293,8 @@ func (ss *ScaleSet) GetPowerStatusByNodeName(name string) (powerState string, er
 	}
 
 	// vm.InstanceView or vm.InstanceView.Statuses are nil when the VM is under deleting.
-	klog.V(3).Infof("InstanceView for node %q is nil, assuming it's stopped", name)
-	return vmPowerStateStopped, nil
+	klog.V(3).Infof("InstanceView for node %q is nil, assuming it's deleting", name)
+	return vmPowerStateUnknown, nil
 }
 
 // GetProvisioningStateByNodeName returns the provisioningState for the specified node.
diff --git a/pkg/provider/azure_vmssflex.go b/pkg/provider/azure_vmssflex.go
index 22889373b..c4a585594 100644
--- a/pkg/provider/azure_vmssflex.go
+++ b/pkg/provider/azure_vmssflex.go
@@ -279,8 +279,8 @@ func (fs *FlexScaleSet) GetPowerStatusByNodeName(name string) (powerState string
 	}
 
 	// vm.InstanceView or vm.InstanceView.Statuses are nil when the VM is under deleting.
-	klog.V(3).Infof("InstanceView for node %q is nil, assuming it's stopped", name)
-	return vmPowerStateStopped, nil
+	klog.V(3).Infof("InstanceView for node %q is nil, assuming it's deleting", name)
+	return vmPowerStateUnknown, nil
 }
 
 // GetPrimaryInterface gets machine primary network interface by node name.


Test patch:
diff --git a/pkg/provider/azure_standard_test.go b/pkg/provider/azure_standard_test.go
index 335365b0f..1f298d470 100644
--- a/pkg/provider/azure_standard_test.go
+++ b/pkg/provider/azure_standard_test.go
@@ -1042,7 +1042,7 @@ func TestGetStandardVMPowerStatusByNodeName(t *testing.T) {
 			expectedStatus: "Running",
 		},
 		{
-			name:     "GetPowerStatusByNodeName should get vmPowerStateStopped if vm.InstanceView is nil",
+			name:     "GetPowerStatusByNodeName should get vmPowerStateUnknown if vm.InstanceView is nil",
 			nodeName: "vm3",
 			vm: compute.VirtualMachine{
 				Name: pointer.String("vm3"),
@@ -1050,10 +1050,10 @@ func TestGetStandardVMPowerStatusByNodeName(t *testing.T) {
 					ProvisioningState: pointer.String("Succeeded"),
 				},
 			},
-			expectedStatus: vmPowerStateStopped,
+			expectedStatus: vmPowerStateUnknown,
 		},
 		{
-			name:     "GetPowerStatusByNodeName should get vmPowerStateStopped if vm.InstanceView.statuses is nil",
+			name:     "GetPowerStatusByNodeName should get vmPowerStateUnknown if vm.InstanceView.statuses is nil",
 			nodeName: "vm4",
 			vm: compute.VirtualMachine{
 				Name: pointer.String("vm4"),
@@ -1062,7 +1062,7 @@ func TestGetStandardVMPowerStatusByNodeName(t *testing.T) {
 					InstanceView:      &compute.VirtualMachineInstanceView{},
 				},
 			},
-			expectedStatus: vmPowerStateStopped,
+			expectedStatus: vmPowerStateUnknown,
 		},
 	}
 	for _, test := range testcases {
diff --git a/pkg/provider/azure_vmss_test.go b/pkg/provider/azure_vmss_test.go
index 2982e2b97..ad6d91dc4 100644
--- a/pkg/provider/azure_vmss_test.go
+++ b/pkg/provider/azure_vmss_test.go
@@ -787,10 +787,10 @@ func TestGetPowerStatusByNodeName(t *testing.T) {
 			expectedPowerState: "Running",
 		},
 		{
-			description:        "GetPowerStatusByNodeName should return vmPowerStateStopped when the vm.InstanceView.Statuses is nil",
+			description:        "GetPowerStatusByNodeName should return vmPowerStateUnknown when the vm.InstanceView.Statuses is nil",
 			vmList:             []string{"vmss-vm-000001"},
 			nilStatus:          true,
-			expectedPowerState: vmPowerStateStopped,
+			expectedPowerState: vmPowerStateUnknown,
 		},
 	}
 
diff --git a/pkg/provider/azure_vmssflex_test.go b/pkg/provider/azure_vmssflex_test.go
index a02d1267d..af22c654f 100644
--- a/pkg/provider/azure_vmssflex_test.go
+++ b/pkg/provider/azure_vmssflex_test.go
@@ -551,12 +551,12 @@ func TestGetPowerStatusByNodeNameVmssFlex(t *testing.T) {
 			expectedErr:                    cloudprovider.InstanceNotFound,
 		},
 		{
-			description:                    "GetPowerStatusByNodeName should return stopped if the node powerstate is nil",
+			description:                    "GetPowerStatusByNodeName should return unknown if the node powerstate is nil",
 			nodeName:                       "vmssflex1000003",
 			testVMListWithoutInstanceView:  testVMListWithoutInstanceView,
 			testVMListWithOnlyInstanceView: testVMListWithOnlyInstanceView,
 			vmListErr:                      nil,
-			expectedPowerStatus:            "stopped",
+			expectedPowerStatus:            "unknown",
 			expectedErr:                    nil,
 		},
 	}