diff --git a/pkg/controller/machineset.go b/pkg/controller/machineset.go index 0a5a82ef71..b4fe5706c4 100644 --- a/pkg/controller/machineset.go +++ b/pkg/controller/machineset.go @@ -31,6 +31,7 @@ import ( "sync" "time" + corev1 "k8s.io/api/core/v1" apiequality "k8s.io/apimachinery/pkg/api/equality" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -904,29 +905,32 @@ func isMachineStatusEqual(s1, s2 v1alpha1.MachineStatus) bool { // or if it is a candidate for auto-preservation. If none of these conditions are met, it returns true indicating // that the failed machine should be terminated. func (c *controller) shouldFailedMachineBeTerminated(machine *v1alpha1.Machine) bool { - // if preserve expiry time is set and is in the future, machine is already preserved - if machine.Status.CurrentStatus.PreserveExpiryTime != nil { - if machine.Status.CurrentStatus.PreserveExpiryTime.After(time.Now()) { - klog.V(3).Infof("Failed machine %q is preserved until %v", machine.Name, machine.Status.CurrentStatus.PreserveExpiryTime) - return false - } - klog.V(3).Infof("Preservation of failed machine %q has timed out at %v", machine.Name, machine.Status.CurrentStatus.PreserveExpiryTime) + var ( + nodeFound bool + node *corev1.Node + err error + ) + if machineutils.HasMachinePreservationExpired(machine) { + klog.V(3).Infof("Preservation of failed machine %q has timed out at %v.", machine.Name, machine.Status.CurrentStatus.PreserveExpiryTime) return true } - preserveValue, err := c.findEffectivePreserveValue(machine) - if err != nil { - // in case of error fetching node or annotations, we don't want to block deletion of failed machines, so we return true - klog.Errorf("error finding effective preserve value for machine %q: %v. Proceeding with termination of the machine.", machine.Name, err) - return true + + nodeName := machine.Labels[v1alpha1.NodeLabelKey] + if nodeName != "" { + node, err = c.nodeLister.Get(nodeName) + if err != nil { + klog.Warningf("Error fetching node %q of machine %q: %v", nodeName, machine.Name, err) + } else { + nodeFound = true + } } - switch preserveValue { - case machineutils.PreserveMachineAnnotationValueWhenFailed, machineutils.PreserveMachineAnnotationValueNow, machineutils.PreserveMachineAnnotationValueAutoPreserved: // this is in case preservation process is not complete yet + + preserveInfo := machineutils.GetPreserveStateInfo(node, machine) + if machineutils.IsPreservationRequested(machineutils.GetEffectivePreservationAnnotations(&preserveInfo, nodeFound)) { + klog.V(3).Infof("Failed machine %q is either preserved or in the process of being preserved.", machine.Name) return false - case machineutils.PreserveMachineAnnotationValueFalse: - return true - default: - return true } + return true } // manageAutoPreservationOfFailedMachines annotates failed machines with preserve=auto-preserved annotation @@ -1019,23 +1023,3 @@ func removeAutoPreserveAnnotationFromMachine(machineToUpdate *v1alpha1.Machine) delete(machineToUpdate.Annotations, machineutils.PreserveMachineAnnotationKey) return nil } - -func (c *controller) findEffectivePreserveValue(machine *v1alpha1.Machine) (string, error) { - var nodeAnnotationValue, machineAnnotationValue, lANodeAnnotationValue string - machineAnnotationValue = machine.Annotations[machineutils.PreserveMachineAnnotationKey] - lANodeAnnotationValue = machine.Annotations[machineutils.LastAppliedNodePreserveValueAnnotationKey] - nodeName := machine.Labels[v1alpha1.NodeLabelKey] - if nodeName != "" { - node, err := c.nodeLister.Get(nodeName) - if err != nil { - klog.Errorf("error fetching node %q for machine %q: %v", nodeName, machine.Name, err) - return "", err - } - nodeAnnotationValue = node.Annotations[machineutils.PreserveMachineAnnotationKey] - } - if nodeAnnotationValue == "" && lANodeAnnotationValue == "" { - return machineAnnotationValue, nil - } else { - return nodeAnnotationValue, nil - } -} diff --git a/pkg/util/provider/machinecontroller/machine.go b/pkg/util/provider/machinecontroller/machine.go index 844e2f7f53..4fbd625243 100644 --- a/pkg/util/provider/machinecontroller/machine.go +++ b/pkg/util/provider/machinecontroller/machine.go @@ -756,18 +756,6 @@ func (c *controller) isCreationProcessing(machine *v1alpha1.Machine) bool { Machine Preservation operations */ -// preserveStateInfo encapsulates the preservation annotation values found -// on the machine and node objects, along with the effective preservation value for the machine -// and the last applied node preserve value by MCM. -type preserveStateInfo struct { - nodeAnnotated bool - machineAnnotated bool - nodeValue string - machineValue string - lastAppliedNodeValue string - preserveExpiryTimeSet bool -} - // manageMachinePreservation manages machine preservation based on the preserve annotation values on the node and machine objects. func (c *controller) manageMachinePreservation(ctx context.Context, machine *v1alpha1.Machine) (retry machineutils.RetryPeriod, err error) { defer func() { @@ -785,17 +773,29 @@ func (c *controller) manageMachinePreservation(ctx context.Context, machine *v1a retry = machineutils.LongRetry } }() + var ( + nodeFound bool + node *corev1.Node + removeAnnotations bool + ) nodeName := machine.Labels[v1alpha1.NodeLabelKey] - // We buffer the error returned here until we can tell if the machine is preservation-bound. - preserveInfo, getErr := c.getPreserveStateInfo(machine) - if preserveInfo.machineAnnotated && !machineutils.AllowedPreserveAnnotationValues.Has(preserveInfo.machineValue) { + if nodeName != "" { + node, err = c.nodeLister.Get(nodeName) // We don't return on error immediately because we need to determine whether the machine has valid preservation state + if err == nil { + nodeFound = true + } else { + klog.Warningf("Error fetching node %q for machine %q: %v", nodeName, machine.Name, err) + } + } + preserveInfo := machineutils.GetPreserveStateInfo(node, machine) + if preserveInfo.MachineAnnotated && !machineutils.AllowedPreserveAnnotationValues.Has(preserveInfo.MachineValue) { // If machine is annotated incorrectly, log and proceed as though machine is not annotated. - klog.Warningf("Preserve annotation %q=%q on machine %q is invalid", machineutils.PreserveMachineAnnotationKey, preserveInfo.machineValue, machine.Name) - preserveInfo.machineAnnotated = false - preserveInfo.machineValue = "" + klog.Warningf("Preserve annotation %q=%q on machine %q is invalid", machineutils.PreserveMachineAnnotationKey, preserveInfo.MachineValue, machine.Name) + preserveInfo.MachineAnnotated = false + preserveInfo.MachineValue = "" } - if preserveInfo.nodeAnnotated && !machineutils.AllowedPreserveAnnotationValues.Has(preserveInfo.nodeValue) { - klog.Warningf("Preserve annotation %q=%q on node %q backing machine %q is invalid", machineutils.PreserveMachineAnnotationKey, preserveInfo.nodeValue, nodeName, machine.Name) + if preserveInfo.NodeAnnotated && !machineutils.AllowedPreserveAnnotationValues.Has(preserveInfo.NodeValue) { + klog.Warningf("Preserve annotation %q=%q on node %q backing machine %q is invalid", machineutils.PreserveMachineAnnotationKey, preserveInfo.NodeValue, nodeName, machine.Name) return } preservationBound := isMachinePreservationBound(&preserveInfo) @@ -803,28 +803,20 @@ func (c *controller) manageMachinePreservation(ctx context.Context, machine *v1a // We clear the error here to prevent preservation logic from interfering with non-preservation-bound machines. err = nil return - } else if getErr != nil { - if !apierrors.IsNotFound(getErr) { - err = getErr - return - } - klog.Warningf("Couldn't find node %q for machine %q", nodeName, machine.Name) - err = nil } - // Note: when the backing node cannot be found, we assume the machine's annotation value needs to be enforced to enable - // preservation of the machine object. - effectivePreserveValue := getEffectivePreservationAnnotations(&preserveInfo, getErr) - var removeAnnotations bool + // Note: when the backing node cannot be fetched, we assume the machine's annotation value needs to be enforced to enable + // preservation of the machine object. + effectivePreserveValue := machineutils.GetEffectivePreservationAnnotations(&preserveInfo, nodeFound) clone := machine.DeepCopy() switch effectivePreserveValue { - // effectivePreserveValue == "" implies the preservation annotation was deleted to indicate that + // effectivePreserveValue == "" implies the preservation annotation was deleted to express intent that // preservation must be stopped case "", machineutils.PreserveMachineAnnotationValueFalse: clone, err = c.stopPreservationIfActive(ctx, clone, removeAnnotations) case machineutils.PreserveMachineAnnotationValueWhenFailed: // on timing out, remove preserve annotation to prevent incorrect re-preservation - if machineutils.IsMachinePreservationExpired(clone) { + if machineutils.HasMachinePreservationExpired(clone) { removeAnnotations = true clone, err = c.stopPreservationIfActive(ctx, clone, removeAnnotations) } else if !machineutils.IsMachineFailed(clone) { @@ -833,7 +825,7 @@ func (c *controller) manageMachinePreservation(ctx context.Context, machine *v1a clone, err = c.preserveMachine(ctx, clone, effectivePreserveValue) } case machineutils.PreserveMachineAnnotationValueNow: - if machineutils.IsMachinePreservationExpired(clone) { + if machineutils.HasMachinePreservationExpired(clone) { // on timing out, remove preserve annotation to prevent incorrect re-preservation removeAnnotations = true clone, err = c.stopPreservationIfActive(ctx, clone, removeAnnotations) @@ -841,7 +833,7 @@ func (c *controller) manageMachinePreservation(ctx context.Context, machine *v1a clone, err = c.preserveMachine(ctx, clone, effectivePreserveValue) } case machineutils.PreserveMachineAnnotationValueAutoPreserved: - if !machineutils.IsMachineFailed(clone) || machineutils.IsMachinePreservationExpired(clone) { + if !machineutils.IsMachineFailed(clone) || machineutils.HasMachinePreservationExpired(clone) { // To prevent incorrect re-preservation of a recovered, previously auto-preserved machine on future failures // (since the autoPreserveFailedMachineCount maintained by the machineSetController, may have changed), // in addition to stopping preservation, we also remove the preservation annotation on the machine. @@ -872,81 +864,33 @@ func (c *controller) manageMachinePreservation(ctx context.Context, machine *v1a } if shouldAnnotationsBeUpdatedOnMachine(removeAnnotations, &preserveInfo) { - err = c.updatePreserveAnnotationOnMachine(ctx, preserveInfo.nodeValue, clone) + err = c.updatePreserveAnnotationOnMachine(ctx, preserveInfo.NodeValue, clone) } return } -// getEffectivePreservationAnnotations returns the effective preservation value. -// -// If there is no active node annotation AND no previously-applied node annotation, -// enforce machine's preserve annotation. -// Otherwise, the node annotation takes precedence (even if now empty/removed). -// -// lastAppliedNodeValue is required to handle the following scenario: -// -// T1: Node and Machine both have the same annotation with the same value. (MCM is up and running). -// T2 (T2 > T1): MCM went down. -// T3 (T3 > T2): Node annotation was removed. -// T4 (T4 > T3): MCM came back up. -// At T4 it sees a Node with no preserve annotation but a Machine with a preserve annotation. -// It continues to preserve the machine. -func getEffectivePreservationAnnotations(info *preserveStateInfo, getPreserveStateErr error) string { - // If the node cannot be found, nodeValue is "". - // In this case, we want the machine's annotation value to be enforced. - if apierrors.IsNotFound(getPreserveStateErr) { - return info.machineValue - } - // If there is no active node annotation AND no previously-applied node annotation, - // enforce machine's preserve annotation. - // Otherwise, the node annotation takes precedence (even if now empty/removed). - if info.nodeValue == "" && info.lastAppliedNodeValue == "" { - return info.machineValue - } - return info.nodeValue -} - -func isMachinePreservationBound(info *preserveStateInfo) bool { +// isMachinePreservationBound returns whether the machine carries any preservation state. +func isMachinePreservationBound(info *machineutils.PreserveStateInfo) bool { // if machine has no preservation state, the machine is not preservation-bound - if !info.preserveExpiryTimeSet && !info.nodeAnnotated && !info.machineAnnotated && info.lastAppliedNodeValue == "" { + if !info.PreserveExpiryTimeSet && !info.NodeAnnotated && !info.MachineAnnotated && info.LastAppliedNodeValue == "" { return false } return true } -func (c *controller) getPreserveStateInfo(machine *v1alpha1.Machine) (preserveStateInfo, error) { - var info preserveStateInfo - if machine.Annotations != nil { - info.machineValue, info.machineAnnotated = machine.Annotations[machineutils.PreserveMachineAnnotationKey] - info.lastAppliedNodeValue = machine.Annotations[machineutils.LastAppliedNodePreserveValueAnnotationKey] - } - if machine.Status.CurrentStatus.PreserveExpiryTime != nil { - info.preserveExpiryTimeSet = true - } - nodeName := machine.Labels[v1alpha1.NodeLabelKey] - if nodeName != "" { - node, err := c.nodeLister.Get(nodeName) - if err != nil { - return info, err - } - info.nodeValue, info.nodeAnnotated = node.Annotations[machineutils.PreserveMachineAnnotationKey] - } - return info, nil -} - // shouldAnnotationsBeUpdatedOnMachine returns true when the machine's annotation tracking needs // to be synced after a preservation action. -func shouldAnnotationsBeUpdatedOnMachine(removeAnnotations bool, preserveInfo *preserveStateInfo) bool { +func shouldAnnotationsBeUpdatedOnMachine(removeAnnotations bool, preserveInfo *machineutils.PreserveStateInfo) bool { // annotations were already removed by stopPreservationIfActive — nothing left to sync if removeAnnotations { return false } // node annotation is not in control — machine annotation prevails, no sync needed - if !preserveInfo.nodeAnnotated && preserveInfo.lastAppliedNodeValue == "" { + if !preserveInfo.NodeAnnotated && preserveInfo.LastAppliedNodeValue == "" { return false } // node value is unchanged and machine has no annotation to clear — nothing has changed - if preserveInfo.nodeValue == preserveInfo.lastAppliedNodeValue && !preserveInfo.machineAnnotated { + if preserveInfo.NodeValue == preserveInfo.LastAppliedNodeValue && !preserveInfo.MachineAnnotated { return false } return true diff --git a/pkg/util/provider/machinecontroller/machine_test.go b/pkg/util/provider/machinecontroller/machine_test.go index 0704f5e34d..ad5076bbf9 100644 --- a/pkg/util/provider/machinecontroller/machine_test.go +++ b/pkg/util/provider/machinecontroller/machine_test.go @@ -3956,146 +3956,7 @@ var _ = Describe("machine", func() { }), ) }) - Describe("#getEffectivePreservationAnnotations", func() { - type setup struct { - nodeAnnotationValue string - machineAnnotations map[string]string - } - type expect struct { - effectivePreserveValue string - machineAnnotations map[string]string - } - - type testCase struct { - setup setup - expect expect - } - - DescribeTable("getEffectivePreservationAnnotations scenarios", - func(tc testCase) { - info := &preserveStateInfo{ - nodeValue: tc.setup.nodeAnnotationValue, - machineValue: tc.setup.machineAnnotations[machineutils.PreserveMachineAnnotationKey], - lastAppliedNodeValue: tc.setup.machineAnnotations[machineutils.LastAppliedNodePreserveValueAnnotationKey], - } - preserveValue := getEffectivePreservationAnnotations(info, nil) - Expect(preserveValue).To(Equal(tc.expect.effectivePreserveValue)) - }, - Entry("when node is not annotated and laNodeAnnotationValue is empty, should return machine's annotation value and empty string", testCase{ - setup: setup{ - nodeAnnotationValue: "", - machineAnnotations: map[string]string{ - machineutils.PreserveMachineAnnotationKey: "A", - machineutils.LastAppliedNodePreserveValueAnnotationKey: "", - }, - }, - expect: expect{ - effectivePreserveValue: "A", - machineAnnotations: map[string]string{ - machineutils.PreserveMachineAnnotationKey: "A", - machineutils.LastAppliedNodePreserveValueAnnotationKey: "", - }, - }, - }), - Entry("when neither node nor machine is not annotated and laNodeAnnotationValue is empty, should return two empty strings", testCase{ - setup: setup{ - nodeAnnotationValue: "", - machineAnnotations: map[string]string{ - machineutils.PreserveMachineAnnotationKey: "", - machineutils.LastAppliedNodePreserveValueAnnotationKey: "", - }, - }, - expect: expect{ - effectivePreserveValue: "", - machineAnnotations: map[string]string{ - machineutils.PreserveMachineAnnotationKey: "", - machineutils.LastAppliedNodePreserveValueAnnotationKey: "", - }, - }, - }), - Entry("when neither node nor machine is annotated and laNodeAnnotationValue is \"A\", should return two empty strings", testCase{ - setup: setup{ - nodeAnnotationValue: "", - machineAnnotations: map[string]string{ - machineutils.PreserveMachineAnnotationKey: "", - machineutils.LastAppliedNodePreserveValueAnnotationKey: "A", - }, - }, - expect: expect{ - effectivePreserveValue: "", - machineAnnotations: map[string]string{ - machineutils.PreserveMachineAnnotationKey: "", - machineutils.LastAppliedNodePreserveValueAnnotationKey: "", - }, - }, - }), - Entry("when node is annotated, laNodeAnnotationValue is empty, and machine is not annotated, should return node's annotation value as effective value and last applied value", testCase{ - setup: setup{ - nodeAnnotationValue: "A", - machineAnnotations: map[string]string{ - machineutils.PreserveMachineAnnotationKey: "", - machineutils.LastAppliedNodePreserveValueAnnotationKey: "", - }, - }, - expect: expect{ - effectivePreserveValue: "A", - machineAnnotations: map[string]string{ - machineutils.PreserveMachineAnnotationKey: "", - machineutils.LastAppliedNodePreserveValueAnnotationKey: "A", - }, - }, - }), - Entry("when node is annotated, laNodeAnnotationValue is empty, and machine is annotated differently, should return node's annotation value as effective value and last applied value", testCase{ - setup: setup{ - nodeAnnotationValue: "A", - machineAnnotations: map[string]string{ - machineutils.PreserveMachineAnnotationKey: "B", - machineutils.LastAppliedNodePreserveValueAnnotationKey: "", - }, - }, - expect: expect{ - effectivePreserveValue: "A", - machineAnnotations: map[string]string{ - machineutils.PreserveMachineAnnotationKey: "", - machineutils.LastAppliedNodePreserveValueAnnotationKey: "A", - }, - }, - }), - Entry("when node, machine annotation values and laNodeAnnotationValue are the same, should return node's annotation value as effective value and last applied value", testCase{ - setup: setup{ - nodeAnnotationValue: "A", - machineAnnotations: map[string]string{ - machineutils.PreserveMachineAnnotationKey: "A", - machineutils.LastAppliedNodePreserveValueAnnotationKey: "A", - }, - }, - expect: expect{ - effectivePreserveValue: "A", - machineAnnotations: map[string]string{ - machineutils.PreserveMachineAnnotationKey: "", - machineutils.LastAppliedNodePreserveValueAnnotationKey: "A", - }, - }, - }), - Entry("when node, machine annotation values are the same and laNodeAnnotationValue differs, should return node's annotation value as effective value and last applied value", testCase{ - setup: setup{ - nodeAnnotationValue: "A", - machineAnnotations: map[string]string{ - machineutils.PreserveMachineAnnotationKey: "A", - machineutils.LastAppliedNodePreserveValueAnnotationKey: "B", - }, - }, - expect: expect{ - effectivePreserveValue: "A", - machineAnnotations: map[string]string{ - machineutils.PreserveMachineAnnotationKey: "", - machineutils.LastAppliedNodePreserveValueAnnotationKey: "A", - }, - }, - }), - ) - }) Describe("#manageMachinePreservation", func() { type setup struct { machineAnnotationValue string @@ -4606,13 +4467,11 @@ var _ = Describe("machine", func() { type setup struct { machineAnnotations map[string]string preserveExpiryTime *metav1.Time - nodeName string nodeAnnotations map[string]string includeNode bool } type expect struct { bound bool - err error } type testCase struct { setup setup @@ -4621,14 +4480,10 @@ var _ = Describe("machine", func() { DescribeTable("isMachinePreservationBound scenarios", func(tc testCase) { - stop := make(chan struct{}) - defer close(stop) - machine := &v1alpha1.Machine{ ObjectMeta: metav1.ObjectMeta{ Namespace: testNamespace, Name: "m1", - Labels: map[string]string{v1alpha1.NodeLabelKey: tc.setup.nodeName}, Annotations: tc.setup.machineAnnotations, }, Status: v1alpha1.MachineStatus{ @@ -4638,46 +4493,31 @@ var _ = Describe("machine", func() { }, } - var targetCoreObjects []runtime.Object + var node *corev1.Node if tc.setup.includeNode { - targetCoreObjects = append(targetCoreObjects, &corev1.Node{ + node = &corev1.Node{ ObjectMeta: metav1.ObjectMeta{ - Name: tc.setup.nodeName, Annotations: tc.setup.nodeAnnotations, }, - }) + } } - c, trackers := createController(stop, testNamespace, []runtime.Object{machine}, nil, targetCoreObjects, nil, false) - defer trackers.Stop() - waitForCacheSync(stop, c) - - preserveInfo, err := c.getPreserveStateInfo(machine) - if tc.expect.err != nil { - Expect(err).To(HaveOccurred()) - Expect(err.Error()).To(Equal(tc.expect.err.Error())) - } else { - Expect(err).ToNot(HaveOccurred()) - } + preserveInfo := machineutils.GetPreserveStateInfo(node, machine) bound := isMachinePreservationBound(&preserveInfo) Expect(bound).To(Equal(tc.expect.bound)) }, Entry("machine has no annotations, no preserveExpiryTime, and node has no preservation annotation", testCase{ setup: setup{ - nodeName: "node-1", includeNode: true, }, expect: expect{bound: false}, }), - Entry("machine has no node label and no other preservation markers", testCase{ - setup: setup{ - nodeName: "", - }, + Entry("machine has no node and no other preservation markers", testCase{ + setup: setup{}, expect: expect{bound: false}, }), Entry("machine has preserve annotation set", testCase{ setup: setup{ - nodeName: "node-1", machineAnnotations: map[string]string{ machineutils.PreserveMachineAnnotationKey: machineutils.PreserveMachineAnnotationValueNow, }, @@ -4687,7 +4527,6 @@ var _ = Describe("machine", func() { }), Entry("machine has a non-nil preserveExpiryTime", testCase{ setup: setup{ - nodeName: "node-1", preserveExpiryTime: &metav1.Time{Time: metav1.Now().Add(1 * time.Hour)}, includeNode: true, }, @@ -4695,7 +4534,6 @@ var _ = Describe("machine", func() { }), Entry("machine's node has a preservation annotation", testCase{ setup: setup{ - nodeName: "node-1", nodeAnnotations: map[string]string{ machineutils.PreserveMachineAnnotationKey: machineutils.PreserveMachineAnnotationValueNow, }, @@ -4705,7 +4543,6 @@ var _ = Describe("machine", func() { }), Entry("machine has last-applied node preserve value annotation", testCase{ setup: setup{ - nodeName: "node-1", machineAnnotations: map[string]string{ machineutils.LastAppliedNodePreserveValueAnnotationKey: machineutils.PreserveMachineAnnotationValueNow, }, @@ -4713,173 +4550,9 @@ var _ = Describe("machine", func() { }, expect: expect{bound: true}, }), - Entry("node is not found and machine has no preservation markers", testCase{ - setup: setup{ - nodeName: "node-1", - includeNode: false, - }, - expect: expect{ - bound: false, - err: fmt.Errorf("node %q not found", "node-1"), - }, - }), - ) - }) - - Describe("#getPreserveStateInfo", func() { - type setup struct { - machineAnnotations map[string]string - preserveExpiryTime *metav1.Time - nodeName string - nodeAnnotations map[string]string - includeNode bool - } - type expect struct { - machineAnnotated bool - machineValue string - nodeAnnotated bool - nodeValue string - lastAppliedNodeValue string - preserveExpiryTimeSet bool - err error - } - type testCase struct { - setup setup - expect expect - } - - DescribeTable("getPreserveStateInfo scenarios", - func(tc testCase) { - stop := make(chan struct{}) - defer close(stop) - - machine := &v1alpha1.Machine{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: testNamespace, - Name: "m1", - Labels: map[string]string{v1alpha1.NodeLabelKey: tc.setup.nodeName}, - Annotations: tc.setup.machineAnnotations, - }, - Status: v1alpha1.MachineStatus{ - CurrentStatus: v1alpha1.CurrentStatus{ - PreserveExpiryTime: tc.setup.preserveExpiryTime, - }, - }, - } - - var targetCoreObjects []runtime.Object - if tc.setup.includeNode { - targetCoreObjects = append(targetCoreObjects, &corev1.Node{ - ObjectMeta: metav1.ObjectMeta{ - Name: tc.setup.nodeName, - Annotations: tc.setup.nodeAnnotations, - }, - }) - } - - c, trackers := createController(stop, testNamespace, []runtime.Object{machine}, nil, targetCoreObjects, nil, false) - defer trackers.Stop() - waitForCacheSync(stop, c) - - info, err := c.getPreserveStateInfo(machine) - if tc.expect.err != nil { - Expect(err).To(HaveOccurred()) - Expect(err.Error()).To(ContainSubstring(tc.expect.err.Error())) - } else { - Expect(err).ToNot(HaveOccurred()) - } - Expect(info.machineAnnotated).To(Equal(tc.expect.machineAnnotated)) - Expect(info.machineValue).To(Equal(tc.expect.machineValue)) - Expect(info.nodeAnnotated).To(Equal(tc.expect.nodeAnnotated)) - Expect(info.nodeValue).To(Equal(tc.expect.nodeValue)) - Expect(info.lastAppliedNodeValue).To(Equal(tc.expect.lastAppliedNodeValue)) - Expect(info.preserveExpiryTimeSet).To(Equal(tc.expect.preserveExpiryTimeSet)) - }, - Entry("machine has no annotations and no node label", testCase{ + Entry("node is nil and machine has no preservation markers", testCase{ setup: setup{}, - expect: expect{}, - }), - Entry("machine has preserve annotation and node is not annotated", testCase{ - setup: setup{ - nodeName: "node-1", - includeNode: true, - machineAnnotations: map[string]string{ - machineutils.PreserveMachineAnnotationKey: machineutils.PreserveMachineAnnotationValueNow, - }, - }, - expect: expect{ - machineAnnotated: true, - machineValue: machineutils.PreserveMachineAnnotationValueNow, - }, - }), - Entry("machine has last-applied node preserve value annotation but no preserve annotations on node or machine", testCase{ - setup: setup{ - nodeName: "node-1", - includeNode: true, - machineAnnotations: map[string]string{ - machineutils.LastAppliedNodePreserveValueAnnotationKey: machineutils.PreserveMachineAnnotationValueNow, - }, - }, - expect: expect{ - lastAppliedNodeValue: machineutils.PreserveMachineAnnotationValueNow, - }, - }), - Entry("machine has a non-nil preserveExpiryTime", testCase{ - setup: setup{ - nodeName: "node-1", - includeNode: true, - preserveExpiryTime: &metav1.Time{Time: metav1.Now().Add(1 * time.Hour)}, - }, - expect: expect{ - preserveExpiryTimeSet: true, - }, - }), - Entry("node has preserve annotation", testCase{ - setup: setup{ - nodeName: "node-1", - includeNode: true, - nodeAnnotations: map[string]string{ - machineutils.PreserveMachineAnnotationKey: machineutils.PreserveMachineAnnotationValueWhenFailed, - }, - }, - expect: expect{ - nodeAnnotated: true, - nodeValue: machineutils.PreserveMachineAnnotationValueWhenFailed, - }, - }), - Entry("node not found returns error and partial info", testCase{ - setup: setup{ - nodeName: "node-1", - includeNode: false, - machineAnnotations: map[string]string{ - machineutils.PreserveMachineAnnotationKey: machineutils.PreserveMachineAnnotationValueNow, - }, - }, - expect: expect{ - machineAnnotated: true, - machineValue: machineutils.PreserveMachineAnnotationValueNow, - err: fmt.Errorf("node-1"), - }, - }), - Entry("both machine and node have preserve annotations", testCase{ - setup: setup{ - nodeName: "node-1", - includeNode: true, - machineAnnotations: map[string]string{ - machineutils.PreserveMachineAnnotationKey: machineutils.PreserveMachineAnnotationValueNow, - machineutils.LastAppliedNodePreserveValueAnnotationKey: machineutils.PreserveMachineAnnotationValueWhenFailed, - }, - nodeAnnotations: map[string]string{ - machineutils.PreserveMachineAnnotationKey: machineutils.PreserveMachineAnnotationValueAutoPreserved, - }, - }, - expect: expect{ - machineAnnotated: true, - machineValue: machineutils.PreserveMachineAnnotationValueNow, - lastAppliedNodeValue: machineutils.PreserveMachineAnnotationValueWhenFailed, - nodeAnnotated: true, - nodeValue: machineutils.PreserveMachineAnnotationValueAutoPreserved, - }, + expect: expect{bound: false}, }), ) }) @@ -4887,7 +4560,7 @@ var _ = Describe("machine", func() { Describe("#shouldAnnotationsBeUpdatedOnMachine", func() { type testCase struct { removeAnnotations bool - preserveInfo *preserveStateInfo + preserveInfo *machineutils.PreserveStateInfo expect bool } @@ -4897,61 +4570,61 @@ var _ = Describe("machine", func() { }, Entry("removeAnnotations=true: always returns false", testCase{ removeAnnotations: true, - preserveInfo: &preserveStateInfo{nodeAnnotated: true, nodeValue: machineutils.PreserveMachineAnnotationValueNow}, + preserveInfo: &machineutils.PreserveStateInfo{NodeAnnotated: true, NodeValue: machineutils.PreserveMachineAnnotationValueNow}, expect: false, }), Entry("node not annotated and no lastAppliedNodeValue: returns false", testCase{ removeAnnotations: false, - preserveInfo: &preserveStateInfo{nodeAnnotated: false, lastAppliedNodeValue: ""}, + preserveInfo: &machineutils.PreserveStateInfo{NodeAnnotated: false, LastAppliedNodeValue: ""}, expect: false, }), Entry("node value unchanged and machine not annotated: returns false", testCase{ removeAnnotations: false, - preserveInfo: &preserveStateInfo{ - nodeAnnotated: true, - nodeValue: machineutils.PreserveMachineAnnotationValueNow, - lastAppliedNodeValue: machineutils.PreserveMachineAnnotationValueNow, - machineAnnotated: false, + preserveInfo: &machineutils.PreserveStateInfo{ + NodeAnnotated: true, + NodeValue: machineutils.PreserveMachineAnnotationValueNow, + LastAppliedNodeValue: machineutils.PreserveMachineAnnotationValueNow, + MachineAnnotated: false, }, expect: false, }), Entry("node value changed: returns true", testCase{ removeAnnotations: false, - preserveInfo: &preserveStateInfo{ - nodeAnnotated: true, - nodeValue: machineutils.PreserveMachineAnnotationValueNow, - lastAppliedNodeValue: machineutils.PreserveMachineAnnotationValueWhenFailed, - machineAnnotated: false, + preserveInfo: &machineutils.PreserveStateInfo{ + NodeAnnotated: true, + NodeValue: machineutils.PreserveMachineAnnotationValueNow, + LastAppliedNodeValue: machineutils.PreserveMachineAnnotationValueWhenFailed, + MachineAnnotated: false, }, expect: true, }), Entry("node value unchanged but machine is annotated: returns true", testCase{ removeAnnotations: false, - preserveInfo: &preserveStateInfo{ - nodeAnnotated: true, - nodeValue: machineutils.PreserveMachineAnnotationValueNow, - lastAppliedNodeValue: machineutils.PreserveMachineAnnotationValueNow, - machineAnnotated: true, + preserveInfo: &machineutils.PreserveStateInfo{ + NodeAnnotated: true, + NodeValue: machineutils.PreserveMachineAnnotationValueNow, + LastAppliedNodeValue: machineutils.PreserveMachineAnnotationValueNow, + MachineAnnotated: true, }, expect: true, }), Entry("lastAppliedNodeValue set without node annotation: returns true (machine annotation present)", testCase{ removeAnnotations: false, - preserveInfo: &preserveStateInfo{ - nodeAnnotated: false, - nodeValue: "", - lastAppliedNodeValue: machineutils.PreserveMachineAnnotationValueNow, - machineAnnotated: true, + preserveInfo: &machineutils.PreserveStateInfo{ + NodeAnnotated: false, + NodeValue: "", + LastAppliedNodeValue: machineutils.PreserveMachineAnnotationValueNow, + MachineAnnotated: true, }, expect: true, }), Entry("lastAppliedNodeValue set without node annotation and machine not annotated: returns true (node value drifted from last-applied)", testCase{ removeAnnotations: false, - preserveInfo: &preserveStateInfo{ - nodeAnnotated: false, - nodeValue: "", - lastAppliedNodeValue: machineutils.PreserveMachineAnnotationValueNow, - machineAnnotated: false, + preserveInfo: &machineutils.PreserveStateInfo{ + NodeAnnotated: false, + NodeValue: "", + LastAppliedNodeValue: machineutils.PreserveMachineAnnotationValueNow, + MachineAnnotated: false, }, expect: true, }), diff --git a/pkg/util/provider/machineutils/suite_test.go b/pkg/util/provider/machineutils/suite_test.go new file mode 100644 index 0000000000..b4d487b68f --- /dev/null +++ b/pkg/util/provider/machineutils/suite_test.go @@ -0,0 +1,17 @@ +// SPDX-FileCopyrightText: 2024 SAP SE or an SAP affiliate company and Gardener contributors +// +// SPDX-License-Identifier: Apache-2.0 + +package machineutils + +import ( + "testing" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +func TestMachineUtils(t *testing.T) { + RegisterFailHandler(Fail) + RunSpecs(t, "MachineUtils Suite") +} diff --git a/pkg/util/provider/machineutils/utils.go b/pkg/util/provider/machineutils/utils.go index 0ce057eaa9..6dfcd0d75c 100644 --- a/pkg/util/provider/machineutils/utils.go +++ b/pkg/util/provider/machineutils/utils.go @@ -98,6 +98,17 @@ const ( PreserveMachineAnnotationKey = "node.machine.sapcloud.io/preserve" // LastAppliedNodePreserveValueAnnotationKey is the annotation used to store the last preserve value applied by MCM + // + // This annotation is required to distinguish between a node's annotation being removed vs the node never having had a preserve annotation. + // If the former, preservation needs to be stopped, if the latter, the machine's preserve annotation needs to be enforced. + // Eg: + // T1: Node and machine objects annotated with `when-failed` preserve annotation. Machine is in Failed phase and preserved. + // T2 (T2 > T1): MCM went down. + // T3 (T3 > T2): Node annotation was removed, to indicate that preservation should be stopped. MCM is still down. + // T4 (T4 > T3): MCM came back up. + // At T4 MCM sees a Node with no preserve annotation but a Machine with a preserve annotation. + // MCM continues to preserve the machine, whereas it should have stopped preservation because the node annotation was removed at T3. + // To avoid this, MCM stores the last applied preserve value on the machine object, so that it can distinguish between a node annotation being removed vs having never been applied. LastAppliedNodePreserveValueAnnotationKey = "node.machine.sapcloud.io/last-applied-node-preserve-value" // PreserveMachineAnnotationValueNow is the annotation value used to explicitly request that @@ -142,6 +153,18 @@ const ( LongRetry RetryPeriod = RetryPeriod(10 * time.Minute) ) +// PreserveStateInfo encapsulates the preservation annotation values found +// on the machine and node objects, along with the effective preservation value for the machine +// and the last applied node preserve value by MCM. +type PreserveStateInfo struct { + NodeAnnotated bool + MachineAnnotated bool + NodeValue string + MachineValue string + LastAppliedNodeValue string + PreserveExpiryTimeSet bool +} + // EssentialTaints are taints on node object which if added/removed, require an immediate reconcile by machine controller // TODO: update this when taints for ALT updation and PostCreate operations is introduced. var EssentialTaints = []string{TaintNodeCriticalComponentsNotReady} @@ -169,8 +192,8 @@ func IsMachineTriggeredForDeletion(m *v1alpha1.Machine) bool { return m.Annotations[MachinePriority] == "1" } -// IsMachinePreservationExpired checks if the preserve expiry time has passed for a machine -func IsMachinePreservationExpired(m *v1alpha1.Machine) bool { +// HasMachinePreservationExpired checks if the preserve expiry time has passed for a machine +func HasMachinePreservationExpired(m *v1alpha1.Machine) bool { t := m.Status.CurrentStatus.PreserveExpiryTime return t != nil && !t.After(time.Now()) } @@ -211,3 +234,46 @@ func UpdateMachineWithRetries(ctx context.Context, machineClient v1alpha1client. return machine, retryErr } + +// GetPreserveStateInfo collects the preservation state of a machine from its annotations, expiry time, and backing node. +func GetPreserveStateInfo(node *v1.Node, machine *v1alpha1.Machine) PreserveStateInfo { + var info PreserveStateInfo + if machine.Annotations != nil { + info.MachineValue, info.MachineAnnotated = machine.Annotations[PreserveMachineAnnotationKey] + info.LastAppliedNodeValue = machine.Annotations[LastAppliedNodePreserveValueAnnotationKey] + } + if node != nil && node.Annotations != nil { + info.NodeValue, info.NodeAnnotated = node.Annotations[PreserveMachineAnnotationKey] + } + if !machine.Status.CurrentStatus.PreserveExpiryTime.IsZero() { + info.PreserveExpiryTimeSet = true + } + return info +} + +// IsPreservationRequested returns true when value is a preserve annotation value that requests +// preservation (now/when-failed/auto-preserved). +func IsPreservationRequested(value string) bool { + switch value { + case PreserveMachineAnnotationValueNow, PreserveMachineAnnotationValueWhenFailed, PreserveMachineAnnotationValueAutoPreserved: + return true + default: + return false + } +} + +// GetEffectivePreservationAnnotations returns the effective preservation value. +func GetEffectivePreservationAnnotations(info *PreserveStateInfo, nodeFound bool) string { + // If the node cannot be found, nodeValue is "". + // In this case, we want the machine's annotation value to be enforced. + if !nodeFound { + return info.MachineValue + } + // If there is no active node annotation AND no previously-applied node annotation, + // enforce machine's preserve annotation. + // Otherwise, the node annotation takes precedence (even if now empty/removed). + if info.NodeValue == "" && info.LastAppliedNodeValue == "" { + return info.MachineValue + } + return info.NodeValue +} diff --git a/pkg/util/provider/machineutils/utils_test.go b/pkg/util/provider/machineutils/utils_test.go new file mode 100644 index 0000000000..536063b151 --- /dev/null +++ b/pkg/util/provider/machineutils/utils_test.go @@ -0,0 +1,319 @@ +// SPDX-FileCopyrightText: 2024 SAP SE or an SAP affiliate company and Gardener contributors +// +// SPDX-License-Identifier: Apache-2.0 + +package machineutils + +import ( + "time" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + "github.com/gardener/machine-controller-manager/pkg/apis/machine/v1alpha1" +) + +var _ = Describe("machineutils", func() { + Describe("#GetPreserveStateInfo", func() { + type setup struct { + machineAnnotations map[string]string + preserveExpiryTime *metav1.Time + node *corev1.Node + } + type expect struct { + machineAnnotated bool + machineValue string + nodeAnnotated bool + nodeValue string + lastAppliedNodeValue string + preserveExpiryTimeSet bool + } + type testCase struct { + setup setup + expect expect + } + + DescribeTable("GetPreserveStateInfo scenarios", + func(tc testCase) { + machine := &v1alpha1.Machine{ + ObjectMeta: metav1.ObjectMeta{ + Name: "m1", + Annotations: tc.setup.machineAnnotations, + }, + Status: v1alpha1.MachineStatus{ + CurrentStatus: v1alpha1.CurrentStatus{ + PreserveExpiryTime: tc.setup.preserveExpiryTime, + }, + }, + } + + info := GetPreserveStateInfo(tc.setup.node, machine) + Expect(info.MachineAnnotated).To(Equal(tc.expect.machineAnnotated)) + Expect(info.MachineValue).To(Equal(tc.expect.machineValue)) + Expect(info.NodeAnnotated).To(Equal(tc.expect.nodeAnnotated)) + Expect(info.NodeValue).To(Equal(tc.expect.nodeValue)) + Expect(info.LastAppliedNodeValue).To(Equal(tc.expect.lastAppliedNodeValue)) + Expect(info.PreserveExpiryTimeSet).To(Equal(tc.expect.preserveExpiryTimeSet)) + }, + Entry("machine has no annotations and node is nil", testCase{ + setup: setup{}, + expect: expect{}, + }), + Entry("machine has preserve annotation and node is not annotated", testCase{ + setup: setup{ + machineAnnotations: map[string]string{ + PreserveMachineAnnotationKey: PreserveMachineAnnotationValueNow, + }, + node: &corev1.Node{}, + }, + expect: expect{ + machineAnnotated: true, + machineValue: PreserveMachineAnnotationValueNow, + }, + }), + Entry("machine has last-applied node preserve value annotation but no preserve annotations on node or machine", testCase{ + setup: setup{ + machineAnnotations: map[string]string{ + LastAppliedNodePreserveValueAnnotationKey: PreserveMachineAnnotationValueNow, + }, + node: &corev1.Node{}, + }, + expect: expect{ + lastAppliedNodeValue: PreserveMachineAnnotationValueNow, + }, + }), + Entry("machine has a non-nil preserveExpiryTime", testCase{ + setup: setup{ + preserveExpiryTime: &metav1.Time{Time: metav1.Now().Add(1 * time.Hour)}, + node: &corev1.Node{}, + }, + expect: expect{ + preserveExpiryTimeSet: true, + }, + }), + Entry("node has preserve annotation", testCase{ + setup: setup{ + node: &corev1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + PreserveMachineAnnotationKey: PreserveMachineAnnotationValueWhenFailed, + }, + }, + }, + }, + expect: expect{ + nodeAnnotated: true, + nodeValue: PreserveMachineAnnotationValueWhenFailed, + }, + }), + Entry("nil node results in no node annotation state", testCase{ + setup: setup{ + machineAnnotations: map[string]string{ + PreserveMachineAnnotationKey: PreserveMachineAnnotationValueNow, + }, + node: nil, + }, + expect: expect{ + machineAnnotated: true, + machineValue: PreserveMachineAnnotationValueNow, + }, + }), + Entry("both machine and node have preserve annotations", testCase{ + setup: setup{ + machineAnnotations: map[string]string{ + PreserveMachineAnnotationKey: PreserveMachineAnnotationValueNow, + LastAppliedNodePreserveValueAnnotationKey: PreserveMachineAnnotationValueWhenFailed, + }, + node: &corev1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + PreserveMachineAnnotationKey: PreserveMachineAnnotationValueWhenFailed, + }, + }, + }, + }, + expect: expect{ + machineAnnotated: true, + machineValue: PreserveMachineAnnotationValueNow, + lastAppliedNodeValue: PreserveMachineAnnotationValueWhenFailed, + nodeAnnotated: true, + nodeValue: PreserveMachineAnnotationValueWhenFailed, + }, + }), + ) + }) + + Describe("#GetEffectivePreservationAnnotations", func() { + type setup struct { + nodeAnnotationValue string + machineAnnotations map[string]string + } + type expect struct { + effectivePreserveValue string + machineAnnotations map[string]string + } + + type testCase struct { + setup setup + expect expect + } + + DescribeTable("GetEffectivePreservationAnnotations scenarios", + func(tc testCase) { + info := &PreserveStateInfo{ + NodeValue: tc.setup.nodeAnnotationValue, + MachineValue: tc.setup.machineAnnotations[PreserveMachineAnnotationKey], + LastAppliedNodeValue: tc.setup.machineAnnotations[LastAppliedNodePreserveValueAnnotationKey], + } + preserveValue := GetEffectivePreservationAnnotations(info, true) + Expect(preserveValue).To(Equal(tc.expect.effectivePreserveValue)) + }, + Entry("when node is not annotated and laNodeAnnotationValue is empty, should return machine's annotation value and empty string", testCase{ + setup: setup{ + nodeAnnotationValue: "", + machineAnnotations: map[string]string{ + PreserveMachineAnnotationKey: "A", + LastAppliedNodePreserveValueAnnotationKey: "", + }, + }, + expect: expect{ + effectivePreserveValue: "A", + machineAnnotations: map[string]string{ + PreserveMachineAnnotationKey: "A", + LastAppliedNodePreserveValueAnnotationKey: "", + }, + }, + }), + Entry("when neither node nor machine is not annotated and laNodeAnnotationValue is empty, should return two empty strings", testCase{ + setup: setup{ + nodeAnnotationValue: "", + machineAnnotations: map[string]string{ + PreserveMachineAnnotationKey: "", + LastAppliedNodePreserveValueAnnotationKey: "", + }, + }, + expect: expect{ + effectivePreserveValue: "", + machineAnnotations: map[string]string{ + PreserveMachineAnnotationKey: "", + LastAppliedNodePreserveValueAnnotationKey: "", + }, + }, + }), + Entry("when neither node nor machine is annotated and laNodeAnnotationValue is \"A\", should return two empty strings", testCase{ + setup: setup{ + nodeAnnotationValue: "", + machineAnnotations: map[string]string{ + PreserveMachineAnnotationKey: "", + LastAppliedNodePreserveValueAnnotationKey: "A", + }, + }, + expect: expect{ + effectivePreserveValue: "", + machineAnnotations: map[string]string{ + PreserveMachineAnnotationKey: "", + LastAppliedNodePreserveValueAnnotationKey: "", + }, + }, + }), + Entry("when node is annotated, laNodeAnnotationValue is empty, and machine is not annotated, should return node's annotation value as effective value and last applied value", testCase{ + setup: setup{ + nodeAnnotationValue: "A", + machineAnnotations: map[string]string{ + PreserveMachineAnnotationKey: "", + LastAppliedNodePreserveValueAnnotationKey: "", + }, + }, + expect: expect{ + effectivePreserveValue: "A", + machineAnnotations: map[string]string{ + PreserveMachineAnnotationKey: "", + LastAppliedNodePreserveValueAnnotationKey: "A", + }, + }, + }), + Entry("when node is annotated, laNodeAnnotationValue is empty, and machine is annotated differently, should return node's annotation value as effective value and last applied value", testCase{ + setup: setup{ + nodeAnnotationValue: "A", + machineAnnotations: map[string]string{ + PreserveMachineAnnotationKey: "B", + LastAppliedNodePreserveValueAnnotationKey: "", + }, + }, + expect: expect{ + effectivePreserveValue: "A", + machineAnnotations: map[string]string{ + PreserveMachineAnnotationKey: "", + LastAppliedNodePreserveValueAnnotationKey: "A", + }, + }, + }), + Entry("when node, machine annotation values and laNodeAnnotationValue are the same, should return node's annotation value as effective value and last applied value", testCase{ + setup: setup{ + nodeAnnotationValue: "A", + machineAnnotations: map[string]string{ + PreserveMachineAnnotationKey: "A", + LastAppliedNodePreserveValueAnnotationKey: "A", + }, + }, + expect: expect{ + effectivePreserveValue: "A", + machineAnnotations: map[string]string{ + PreserveMachineAnnotationKey: "", + LastAppliedNodePreserveValueAnnotationKey: "A", + }, + }, + }), + Entry("when node, machine annotation values are the same and laNodeAnnotationValue differs, should return node's annotation value as effective value and last applied value", testCase{ + setup: setup{ + nodeAnnotationValue: "A", + machineAnnotations: map[string]string{ + PreserveMachineAnnotationKey: "A", + LastAppliedNodePreserveValueAnnotationKey: "B", + }, + }, + expect: expect{ + effectivePreserveValue: "A", + machineAnnotations: map[string]string{ + PreserveMachineAnnotationKey: "", + LastAppliedNodePreserveValueAnnotationKey: "A", + }, + }, + }), + ) + }) + + Describe("#IsPreservationRequested", func() { + DescribeTable("IsPreservationRequested scenarios", + func(value string, expected bool) { + Expect(IsPreservationRequested(value)).To(Equal(expected)) + }, + Entry("preserve=now is a positive preserve value", PreserveMachineAnnotationValueNow, true), + Entry("preserve=when-failed is a positive preserve value", PreserveMachineAnnotationValueWhenFailed, true), + Entry("preserve=auto-preserved is a positive preserve value", PreserveMachineAnnotationValueAutoPreserved, true), + Entry("preserve=false is not a positive preserve value", PreserveMachineAnnotationValueFalse, false), + Entry("empty value is not a positive preserve value", "", false), + Entry("unrecognized value is not a positive preserve value", "some-invalid-value", false), + ) + }) + + Describe("#HasMachinePreservationExpired", func() { + DescribeTable("HasMachinePreservationExpired scenarios", + func(preserveExpiryTime *metav1.Time, expected bool) { + machine := &v1alpha1.Machine{ + Status: v1alpha1.MachineStatus{ + CurrentStatus: v1alpha1.CurrentStatus{ + PreserveExpiryTime: preserveExpiryTime, + }, + }, + } + Expect(HasMachinePreservationExpired(machine)).To(Equal(expected)) + }, + Entry("nil preserveExpiryTime has not expired", nil, false), + Entry("preserveExpiryTime in the past has expired", &metav1.Time{Time: metav1.Now().Add(-1 * time.Hour)}, true), + Entry("preserveExpiryTime in the future has not expired", &metav1.Time{Time: metav1.Now().Add(1 * time.Hour)}, false), + ) + }) +})