Add nil checks for vm.Properties.TimeCreated with fallback to preserve GC grace period (#1390)

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: rakechill <30397998+rakechill@users.noreply.github.com>
Co-authored-by: Rachel Gregory <rgregory@microsoft.com>

Author: Copilot

pkg/cloudprovider/cloudprovider.go

@@ -532,7 +532,18 @@ func (c *CloudProvider) vmInstanceToNodeClaim(ctx context.Context, vm *armcomput
 	nodeClaim.Name = GetNodeClaimNameFromVMName(*vm.Name)
 	nodeClaim.Labels = labels
 	nodeClaim.Annotations = annotations
-	nodeClaim.CreationTimestamp = metav1.Time{Time: *vm.Properties.TimeCreated}
+	if vm.Properties != nil && vm.Properties.TimeCreated != nil {
+		nodeClaim.CreationTimestamp = metav1.Time{Time: *vm.Properties.TimeCreated}
+	} else {
+		// Fallback to current time to ensure garbage collection grace period is enforced
+		// when TimeCreated is unavailable. Without this, CreationTimestamp would be epoch (zero value)
+		// and the instance could be immediately garbage collected, bypassing the 5-minute grace period.
+		// TODO: Investigate a more fail-safe approach. If vm.Properties.TimeCreated is NEVER populated,
+		// this fallback means the VM will never be garbage collected since we call this helper every time
+		// we create an in-memory NodeClaim. We currently assume this shouldn't happen because VMs that fail
+		// to come up should eventually stop appearing in Azure API responses.
+		nodeClaim.CreationTimestamp = metav1.Time{Time: time.Now()}
+	}
 	// Set the deletionTimestamp to be the current time if the instance is currently terminating
 	if utils.IsVMDeleting(*vm) {
 		nodeClaim.DeletionTimestamp = &metav1.Time{Time: time.Now()}

pkg/cloudprovider/cloudprovider_test.go

@@ -17,9 +17,14 @@ limitations under the License.
 package cloudprovider
 
 import (
+	"context"
 	"testing"
+	"time"
 
+	"github.com/Azure/azure-sdk-for-go/sdk/azcore/to"
+	"github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/compute/armcompute/v7"
 	. "github.com/onsi/gomega"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
 
 func TestGenerateNodeClaimName(t *testing.T) {
@@ -54,3 +59,68 @@ func TestGenerateNodeClaimName(t *testing.T) {
 		})
 	}
 }
+
+func TestVmInstanceToNodeClaim_NilProperties(t *testing.T) {
+	tests := []struct {
+		name                string
+		vm                  *armcompute.VirtualMachine
+		expectFallbackToNow bool
+		expectExactTime     *time.Time
+	}{
+		{
+			name: "nil Properties - fallback to time.Now()",
+			vm: &armcompute.VirtualMachine{
+				Name: to.Ptr("aks-test-vm"),
+				ID:   to.Ptr("/subscriptions/sub/resourceGroups/rg/providers/Microsoft.Compute/virtualMachines/aks-test-vm"),
+			},
+			expectFallbackToNow: true,
+		},
+		{
+			name: "nil TimeCreated - fallback to time.Now()",
+			vm: &armcompute.VirtualMachine{
+				Name:       to.Ptr("aks-test-vm"),
+				ID:         to.Ptr("/subscriptions/sub/resourceGroups/rg/providers/Microsoft.Compute/virtualMachines/aks-test-vm"),
+				Properties: &armcompute.VirtualMachineProperties{},
+			},
+			expectFallbackToNow: true,
+		},
+		{
+			name: "valid TimeCreated - use exact time",
+			vm: &armcompute.VirtualMachine{
+				Name: to.Ptr("aks-test-vm"),
+				ID:   to.Ptr("/subscriptions/sub/resourceGroups/rg/providers/Microsoft.Compute/virtualMachines/aks-test-vm"),
+				Properties: &armcompute.VirtualMachineProperties{
+					TimeCreated: to.Ptr(time.Date(2024, 1, 1, 0, 0, 0, 0, time.UTC)),
+				},
+			},
+			expectExactTime: to.Ptr(time.Date(2024, 1, 1, 0, 0, 0, 0, time.UTC)),
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			g := NewWithT(t)
+			ctx := context.Background()
+
+			cp := &CloudProvider{}
+			before := time.Now()
+			nodeClaim, err := cp.vmInstanceToNodeClaim(ctx, tt.vm, nil)
+			after := time.Now()
+
+			g.Expect(err).ToNot(HaveOccurred())
+			g.Expect(nodeClaim).ToNot(BeNil())
+			g.Expect(nodeClaim.CreationTimestamp).ToNot(Equal(metav1.Time{}))
+
+			if tt.expectFallbackToNow {
+				// When TimeCreated is unavailable, should fallback to time.Now() for GC safety
+				g.Expect(nodeClaim.CreationTimestamp.Time).To(BeTemporally(">=", before))
+				g.Expect(nodeClaim.CreationTimestamp.Time).To(BeTemporally("<=", after))
+			}
+
+			if tt.expectExactTime != nil {
+				// When TimeCreated is available, should use the exact time from VM
+				g.Expect(nodeClaim.CreationTimestamp.Time).To(Equal(*tt.expectExactTime))
+			}
+		})
+	}
+}