Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 12 additions & 1 deletion pkg/cloudprovider/cloudprovider.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()}
Expand Down
70 changes: 70 additions & 0 deletions pkg/cloudprovider/cloudprovider_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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))
}
})
}
}