diff --git a/pkg/controllers/machine/disruption/drift.go b/pkg/controllers/machine/disruption/drift.go index 1bf5bab68a49..0b4b36ef272d 100644 --- a/pkg/controllers/machine/disruption/drift.go +++ b/pkg/controllers/machine/disruption/drift.go @@ -102,16 +102,18 @@ func (d *Drift) Reconcile(ctx context.Context, provisioner *v1alpha5.Provisioner // isDrifted will check if a machine is drifted from the fields in the provisioner.Spec and // the cloudprovider func (d *Drift) isDrifted(ctx context.Context, provisioner *v1alpha5.Provisioner, machine *v1alpha5.Machine) (cloudprovider.DriftReason, error) { + // First check for static drift or node requirements have drifted to save on API calls. + if reason := lo.FindOrElse([]cloudprovider.DriftReason{areStaticFieldsDrifted(provisioner, machine), areNodeRequirementsDrifted(provisioner, machine)}, "", func(i cloudprovider.DriftReason) bool { + return i != "" + }); reason != "" { + return reason, nil + } + driftedReason, err := d.cloudProvider.IsMachineDrifted(ctx, machine) if err != nil { return "", err } - - reason := lo.FindOrElse([]cloudprovider.DriftReason{driftedReason, areStaticFieldsDrifted(provisioner, machine), areNodeRequirementsDrifted(provisioner, machine)}, "", func(i cloudprovider.DriftReason) bool { - return i != "" - }) - - return reason, nil + return driftedReason, nil } // Eligible fields for static drift are described in the docs diff --git a/pkg/controllers/machine/disruption/drift_test.go b/pkg/controllers/machine/disruption/drift_test.go index dbaad0516d7c..c027f49739fa 100644 --- a/pkg/controllers/machine/disruption/drift_test.go +++ b/pkg/controllers/machine/disruption/drift_test.go @@ -28,6 +28,7 @@ import ( "github.com/aws/karpenter-core/pkg/apis/settings" "github.com/aws/karpenter-core/pkg/apis/v1alpha5" + "github.com/aws/karpenter-core/pkg/controllers/machine/disruption" controllerprov "github.com/aws/karpenter-core/pkg/controllers/provisioner" "github.com/aws/karpenter-core/pkg/test" @@ -63,6 +64,33 @@ var _ = Describe("Drift", func() { machine = ExpectExists(ctx, env.Client, machine) Expect(machine.StatusConditions().GetCondition(v1alpha5.MachineDrifted).IsTrue()).To(BeTrue()) }) + It("should detect static drift before cloud provider drift", func() { + cp.Drifted = "drifted" + provisioner.Annotations = lo.Assign(provisioner.Annotations, map[string]string{ + v1alpha5.ProvisionerHashAnnotationKey: "123456789", + }) + ExpectApplied(ctx, env.Client, provisioner, machine) + ExpectReconcileSucceeded(ctx, disruptionController, client.ObjectKeyFromObject(machine)) + + machine = ExpectExists(ctx, env.Client, machine) + Expect(machine.StatusConditions().GetCondition(v1alpha5.MachineDrifted).IsTrue()).To(BeTrue()) + Expect(machine.StatusConditions().GetCondition(v1alpha5.MachineDrifted).Reason).To(Equal(string(disruption.ProvisionerStaticallyDrifted))) + }) + It("should detect node requirement drift before cloud provider drift", func() { + cp.Drifted = "drifted" + provisioner.Spec.Requirements = []v1.NodeSelectorRequirement{ + v1.NodeSelectorRequirement{ + Key: v1.LabelInstanceTypeStable, + Operator: v1.NodeSelectorOpDoesNotExist, + }, + } + ExpectApplied(ctx, env.Client, provisioner, machine) + ExpectReconcileSucceeded(ctx, disruptionController, client.ObjectKeyFromObject(machine)) + + machine = ExpectExists(ctx, env.Client, machine) + Expect(machine.StatusConditions().GetCondition(v1alpha5.MachineDrifted).IsTrue()).To(BeTrue()) + Expect(machine.StatusConditions().GetCondition(v1alpha5.MachineDrifted).Reason).To(Equal(string(disruption.NodeRequirementDrifted))) + }) It("should not detect drift if the feature flag is disabled", func() { cp.Drifted = "drifted" ctx = settings.ToContext(ctx, test.Settings(settings.Settings{DriftEnabled: false}))