Skip to content

Commit ab524b1

Browse files
comtalystclaude
andcommitted
fix: restore static field drift tests and unify drift test structure
- Rename suite_aksmachineapi_drift_test.go to suite_drift_test.go to match the reunification pattern (no mode prefix in filenames) - Add 3 missing static field drift tests to runSharedDriftTests() so they run for both AKSMachineAPI and AKSScriptless modes: - "should not trigger drift if NodeClass hasn't changed" - "should trigger drift if NodeClass subnet changed" - "should trigger drift if ImageFamily changed" - Add ExpectNodeClassHashUpdated and hash annotations to AKSMachineAPI BeforeEach so static field drift detection works correctly - Remove duplicate KubernetesVersion tests from AKSScriptless section that are already covered by shared tests Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 02db45c commit ab524b1

File tree

1 file changed

+37
-70
lines changed

1 file changed

+37
-70
lines changed

pkg/cloudprovider/suite_aksmachineapi_drift_test.go renamed to pkg/cloudprovider/suite_drift_test.go

Lines changed: 37 additions & 70 deletions
Original file line numberDiff line numberDiff line change
@@ -209,6 +209,35 @@ func runSharedDriftTests(getNodeClaim func() *karpv1.NodeClaim) {
209209
Expect(drifted).To(BeEmpty())
210210
})
211211
})
212+
213+
Context("Static fields", func() {
214+
It("should not trigger drift if NodeClass hasn't changed", func() {
215+
drifted, err := cloudProvider.IsDrifted(ctx, getNodeClaim())
216+
Expect(err).ToNot(HaveOccurred())
217+
Expect(drifted).To(BeEmpty())
218+
})
219+
220+
It("should trigger drift if NodeClass subnet changed", func() {
221+
testSubnetID := "/subscriptions/12345678-1234-1234-1234-123456789012/resourceGroups/test-resourceGroup/providers/Microsoft.Network/virtualNetworks/aks-vnet-12345678/subnets/my-subnet"
222+
nodeClass.Spec.VNETSubnetID = lo.ToPtr(testSubnetID)
223+
ExpectApplied(ctx, env.Client, nodeClass)
224+
ExpectNodeClassHashUpdated(ctx, env.Client, nodeClass)
225+
226+
drifted, err := cloudProvider.IsDrifted(ctx, getNodeClaim())
227+
Expect(err).ToNot(HaveOccurred())
228+
Expect(drifted).To(Equal(NodeClassDrift))
229+
})
230+
231+
It("should trigger drift if ImageFamily changed", func() {
232+
nodeClass.Spec.ImageFamily = lo.ToPtr(v1beta1.AzureLinuxImageFamily)
233+
ExpectApplied(ctx, env.Client, nodeClass)
234+
ExpectNodeClassHashUpdated(ctx, env.Client, nodeClass)
235+
236+
drifted, err := cloudProvider.IsDrifted(ctx, getNodeClaim())
237+
Expect(err).ToNot(HaveOccurred())
238+
Expect(drifted).To(Equal(NodeClassDrift))
239+
})
240+
})
212241
}
213242

214243
var _ = Describe("CloudProvider", func() {
@@ -254,6 +283,7 @@ var _ = Describe("CloudProvider", func() {
254283
BeforeEach(func() {
255284
instanceType := "Standard_D2_v2"
256285
ExpectApplied(ctx, env.Client, nodePool, nodeClass)
286+
ExpectNodeClassHashUpdated(ctx, env.Client, nodeClass)
257287
pod := coretest.UnschedulablePod(coretest.PodOptions{
258288
NodeSelector: map[string]string{v1.LabelInstanceTypeStable: instanceType},
259289
})
@@ -280,6 +310,13 @@ var _ = Describe("CloudProvider", func() {
280310
Kind: object.GVK(nodeClass).Kind,
281311
Name: nodeClass.Name,
282312
}
313+
// Set hash annotations on the NodeClaim to match the NodeClass,
314+
// mirroring what Create() does via setAdditionalAnnotationsForNewNodeClaim.
315+
// List() doesn't return these annotations, so we set them manually.
316+
nodeClaim.Annotations = lo.Assign(nodeClaim.Annotations, map[string]string{
317+
v1beta1.AnnotationAKSNodeClassHash: nodeClass.Hash(),
318+
v1beta1.AnnotationAKSNodeClassHashVersion: v1beta1.AKSNodeClassHashVersion,
319+
})
283320
})
284321

285322
// Shared tests across provision modes
@@ -405,76 +442,6 @@ var _ = Describe("CloudProvider", func() {
405442
Expect(drifted).To(Equal(KubeletIdentityDrift))
406443
})
407444
})
408-
409-
// KubernetesVersion drift tests (shared across provision modes via runSharedDriftTests or mode-specific)
410-
Context("KubernetesVersion", func() {
411-
It("should succeed with no drift when KubernetesVersionReady is not true", func() {
412-
nodeClass = ExpectExists(ctx, env.Client, nodeClass)
413-
nodeClass.StatusConditions().SetFalse(v1beta1.ConditionTypeKubernetesVersionReady, "K8sVersionNoLongerReady", "test when k8s isn't ready")
414-
ExpectApplied(ctx, env.Client, nodeClass)
415-
drifted, err := cloudProvider.IsDrifted(ctx, driftNodeClaim)
416-
Expect(err).ToNot(HaveOccurred())
417-
Expect(drifted).To(Equal(NoDrift))
418-
})
419-
420-
// TODO (charliedmcb): I'm wondering if we actually want to have these soft-error cases switch to return an error if no-drift condition was found.
421-
It("shouldn't error or be drifted when KubernetesVersion is empty", func() {
422-
nodeClass = ExpectExists(ctx, env.Client, nodeClass)
423-
nodeClass.Status.KubernetesVersion = lo.ToPtr("")
424-
ExpectApplied(ctx, env.Client, nodeClass)
425-
drifted, err := cloudProvider.IsDrifted(ctx, driftNodeClaim)
426-
Expect(err).ToNot(HaveOccurred())
427-
Expect(drifted).To(Equal(NoDrift))
428-
})
429-
430-
It("shouldn't error or be drifted when NodeName is missing", func() {
431-
driftNodeClaim.Status.NodeName = ""
432-
drifted, err := cloudProvider.IsDrifted(ctx, driftNodeClaim)
433-
Expect(err).ToNot(HaveOccurred())
434-
Expect(drifted).To(Equal(NoDrift))
435-
})
436-
437-
It("shouldn't error or be drifted when node is not found", func() {
438-
driftNodeClaim.Status.NodeName = "NodeWhoDoesNotExist"
439-
drifted, err := cloudProvider.IsDrifted(ctx, driftNodeClaim)
440-
Expect(err).ToNot(HaveOccurred())
441-
Expect(drifted).To(Equal(NoDrift))
442-
})
443-
444-
It("shouldn't error or be drifted when node is deleting", func() {
445-
node = ExpectNodeExists(ctx, env.Client, driftNodeClaim.Status.NodeName)
446-
node.Finalizers = append(node.Finalizers, test.TestingFinalizer)
447-
ExpectApplied(ctx, env.Client, node)
448-
Expect(env.Client.Delete(ctx, node)).ToNot(HaveOccurred())
449-
drifted, err := cloudProvider.IsDrifted(ctx, driftNodeClaim)
450-
Expect(err).ToNot(HaveOccurred())
451-
Expect(drifted).To(Equal(NoDrift))
452-
453-
// cleanup
454-
node = ExpectNodeExists(ctx, env.Client, driftNodeClaim.Status.NodeName)
455-
deepCopy := node.DeepCopy()
456-
node.Finalizers = lo.Reject(node.Finalizers, func(finalizer string, _ int) bool {
457-
return finalizer == test.TestingFinalizer
458-
})
459-
Expect(env.Client.Patch(ctx, node, client.StrategicMergeFrom(deepCopy))).NotTo(HaveOccurred())
460-
ExpectDeleted(ctx, env.Client, node)
461-
})
462-
463-
It("should succeed with drift true when KubernetesVersion is new", func() {
464-
nodeClass = ExpectExists(ctx, env.Client, nodeClass)
465-
466-
semverCurrentK8sVersion := lo.Must(semver.ParseTolerant(*nodeClass.Status.KubernetesVersion))
467-
semverCurrentK8sVersion.Minor = semverCurrentK8sVersion.Minor + 1
468-
nodeClass.Status.KubernetesVersion = lo.ToPtr(semverCurrentK8sVersion.String())
469-
470-
ExpectApplied(ctx, env.Client, nodeClass)
471-
472-
drifted, err := cloudProvider.IsDrifted(ctx, driftNodeClaim)
473-
Expect(err).ToNot(HaveOccurred())
474-
Expect(drifted).To(Equal(K8sVersionDrift))
475-
})
476-
})
477-
478445
})
479446
})
480447
})

0 commit comments

Comments
 (0)