PR comments

pkg/apis/provisioning/v1alpha3/provisioner.go

@@ -113,27 +113,20 @@ var (
 )
 
 var (
-	// Well known, supported labels
+	// Well known labels
 	ArchitectureLabelKey    = "kubernetes.io/arch"
 	OperatingSystemLabelKey = "kubernetes.io/os"
-
-	// Reserved taints
-	NotReadyTaintKey = SchemeGroupVersion.Group + "/not-ready"
-
+	ZoneLabelKey            = "topology.kubernetes.io/zone"
+	InstanceTypeLabelKey    = "node.kubernetes.io/instance-type"
 	// Reserved labels
 	ProvisionerNameLabelKey = SchemeGroupVersion.Group + "/provisioner-name"
-
+	// Reserved taints
+	NotReadyTaintKey = SchemeGroupVersion.Group + "/not-ready"
 	// Reserved annotations
-	KarpenterDoNotEvictPodAnnotation = SchemeGroupVersion.Group + "/do-not-evict"
-	ProvisionerTTLAfterEmptyKey      = SchemeGroupVersion.Group + "/ttl-after-empty"
-
-	// Use ProvisionerSpec instead
-	ZoneLabelKey         = "topology.kubernetes.io/zone"
-	InstanceTypeLabelKey = "node.kubernetes.io/instance-type"
-
+	DoNotEvictPodAnnotationKey      = SchemeGroupVersion.Group + "/do-not-evict"
+	EmptinessTimestampAnnotationKey = SchemeGroupVersion.Group + "/emptiness-timestamp"
 	// Finalizers
 	TerminationFinalizer = SchemeGroupVersion.Group + "/termination"
-
 	// Default provisioner
 	DefaultProvisioner = types.NamespacedName{Name: "default"}
 )

pkg/apis/provisioning/v1alpha3/provisioner_validation.go

@@ -32,7 +32,7 @@ var (
 		ArchitectureLabelKey,
 		OperatingSystemLabelKey,
 		ProvisionerNameLabelKey,
-		ProvisionerTTLAfterEmptyKey,
+		EmptinessTimestampAnnotationKey,
 		ZoneLabelKey,
 		InstanceTypeLabelKey,
 	}

pkg/controllers/node/controller.go

@@ -22,8 +22,8 @@ import (
 	"github.com/awslabs/karpenter/pkg/apis/provisioning/v1alpha3"
 	"github.com/awslabs/karpenter/pkg/utils/functional"
 	"github.com/awslabs/karpenter/pkg/utils/result"
-	"go.uber.org/multierr"
 
+	"go.uber.org/multierr"
 	v1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/equality"
 	"k8s.io/apimachinery/pkg/api/errors"

pkg/controllers/node/emptiness.go

@@ -50,23 +50,28 @@ func (r *Emptiness) Reconcile(ctx context.Context, provisioner *v1alpha3.Provisi
 		return reconcile.Result{}, err
 	}
 	if !empty {
-		if _, ok := n.Annotations[v1alpha3.ProvisionerTTLAfterEmptyKey]; ok {
-			delete(n.Annotations, v1alpha3.ProvisionerTTLAfterEmptyKey)
+		if _, ok := n.Annotations[v1alpha3.EmptinessTimestampAnnotationKey]; ok {
+			delete(n.Annotations, v1alpha3.EmptinessTimestampAnnotationKey)
 			logging.FromContext(ctx).Infof("Removed emptiness TTL from node %s", n.Name)
 		}
 		return reconcile.Result{}, nil
 	}
 	// 3. Set TTL if not set
 	n.Annotations = functional.UnionStringMaps(n.Annotations)
-	if _, ok := n.Annotations[v1alpha3.ProvisionerTTLAfterEmptyKey]; !ok {
-		ttl := time.Duration(ptr.Int64Value(provisioner.Spec.TTLSecondsAfterEmpty)) * time.Second
-		n.Annotations[v1alpha3.ProvisionerTTLAfterEmptyKey] = time.Now().Add(ttl).Format(time.RFC3339)
-		logging.FromContext(ctx).Infof("Added TTL (+%s) to empty node %s", ttl, n.Name)
+	ttl := time.Duration(ptr.Int64Value(provisioner.Spec.TTLSecondsAfterEmpty)) * time.Second
+	emptinessTimestamp, ok := n.Annotations[v1alpha3.EmptinessTimestampAnnotationKey]
+	if !ok {
+		n.Annotations[v1alpha3.EmptinessTimestampAnnotationKey] = time.Now().Format(time.RFC3339)
+		logging.FromContext(ctx).Infof("Added TTL to empty node %s", n.Name)
 		return reconcile.Result{RequeueAfter: ttl}, nil
 	}
 	// 4. Delete node if beyond TTL
-	if node.IsPastEmptyTTL(n) {
-		logging.FromContext(ctx).Infof("Triggering termination for empty node %s", n.Name)
+	emptinessTime, err := time.Parse(time.RFC3339, emptinessTimestamp)
+	if err != nil {
+		return reconcile.Result{}, fmt.Errorf("parsing emptiness timestamp, %s", emptinessTimestamp)
+	}
+	if time.Now().After(emptinessTime.Add(ttl)) {
+		logging.FromContext(ctx).Infof("Triggering termination after %s for empty node %s", ttl, n.Name)
 		if err := r.kubeClient.Delete(ctx, n); err != nil {
 			return reconcile.Result{}, fmt.Errorf("deleting node %s, %w", n.Name, err)
 		}

pkg/controllers/node/expiration.go

@@ -34,7 +34,7 @@ type Expiration struct {
 
 // Reconcile reconciles the node
 func (r *Expiration) Reconcile(ctx context.Context, provisioner *v1alpha3.Provisioner, node *v1.Node) (reconcile.Result, error) {
-	// 1. Ignore if TTLSecondsUntilExpired isn't defined
+	// 1. Ignore node if not applicable
 	if provisioner.Spec.TTLSecondsUntilExpired == nil {
 		return reconcile.Result{}, nil
 	}

pkg/controllers/node/suite_test.go

@@ -231,7 +231,7 @@ var _ = Describe("Controller", func() {
 			ExpectReconcileSucceeded(ctx, controller, client.ObjectKeyFromObject(node))
 
 			node = ExpectNodeExists(env.Client, node.Name)
-			Expect(node.Annotations).ToNot(HaveKey(v1alpha3.ProvisionerTTLAfterEmptyKey))
+			Expect(node.Annotations).ToNot(HaveKey(v1alpha3.EmptinessTimestampAnnotationKey))
 		})
 		It("should not TTL nodes that have ready status false", func() {
 			provisioner.Spec.TTLSecondsAfterEmpty = ptr.Int64(30)
@@ -245,7 +245,7 @@ var _ = Describe("Controller", func() {
 			ExpectReconcileSucceeded(ctx, controller, client.ObjectKeyFromObject(node))
 
 			node = ExpectNodeExists(env.Client, node.Name)
-			Expect(node.Annotations).ToNot(HaveKey(v1alpha3.ProvisionerTTLAfterEmptyKey))
+			Expect(node.Annotations).ToNot(HaveKey(v1alpha3.EmptinessTimestampAnnotationKey))
 		})
 		It("should label nodes as underutilized and add TTL", func() {
 			provisioner.Spec.TTLSecondsAfterEmpty = ptr.Int64(30)
@@ -257,14 +257,14 @@ var _ = Describe("Controller", func() {
 			ExpectReconcileSucceeded(ctx, controller, client.ObjectKeyFromObject(node))
 
 			node = ExpectNodeExists(env.Client, node.Name)
-			Expect(node.Annotations).To(HaveKey(v1alpha3.ProvisionerTTLAfterEmptyKey))
+			Expect(node.Annotations).To(HaveKey(v1alpha3.EmptinessTimestampAnnotationKey))
 		})
 		It("should remove labels from non-empty nodes", func() {
 			provisioner.Spec.TTLSecondsAfterEmpty = ptr.Int64(30)
 			node := test.Node(test.NodeOptions{
 				Labels: map[string]string{v1alpha3.ProvisionerNameLabelKey: provisioner.Name},
 				Annotations: map[string]string{
-					v1alpha3.ProvisionerTTLAfterEmptyKey: time.Now().Add(100 * time.Second).Format(time.RFC3339),
+					v1alpha3.EmptinessTimestampAnnotationKey: time.Now().Add(100 * time.Second).Format(time.RFC3339),
 				},
 			})
 			ExpectCreated(env.Client, provisioner)
@@ -278,15 +278,15 @@ var _ = Describe("Controller", func() {
 			ExpectReconcileSucceeded(ctx, controller, client.ObjectKeyFromObject(node))
 
 			node = ExpectNodeExists(env.Client, node.Name)
-			Expect(node.Annotations).ToNot(HaveKey(v1alpha3.ProvisionerTTLAfterEmptyKey))
+			Expect(node.Annotations).ToNot(HaveKey(v1alpha3.EmptinessTimestampAnnotationKey))
 		})
 		It("should terminate empty nodes past their TTL", func() {
 			provisioner.Spec.TTLSecondsAfterEmpty = ptr.Int64(30)
 			node := test.Node(test.NodeOptions{
 				Finalizers: []string{v1alpha3.TerminationFinalizer},
 				Labels:     map[string]string{v1alpha3.ProvisionerNameLabelKey: provisioner.Name},
 				Annotations: map[string]string{
-					v1alpha3.ProvisionerTTLAfterEmptyKey: time.Now().Add(-100 * time.Second).Format(time.RFC3339),
+					v1alpha3.EmptinessTimestampAnnotationKey: time.Now().Add(-100 * time.Second).Format(time.RFC3339),
 				},
 			})
 			ExpectCreated(env.Client, provisioner, node)

pkg/controllers/termination/suite_test.go

@@ -122,7 +122,7 @@ var _ = Describe("Termination", func() {
 			podEvict := test.Pod(test.PodOptions{NodeName: node.Name})
 			podNoEvict := test.Pod(test.PodOptions{
 				NodeName:    node.Name,
-				Annotations: map[string]string{v1alpha3.KarpenterDoNotEvictPodAnnotation: "true"},
+				Annotations: map[string]string{v1alpha3.DoNotEvictPodAnnotationKey: "true"},
 			})
 
 			ExpectCreated(env.Client, node, podEvict, podNoEvict)

pkg/controllers/termination/terminate.go

@@ -68,7 +68,7 @@ func (t *Terminator) drain(ctx context.Context, node *v1.Node) (bool, error) {
 	critical := []*v1.Pod{}
 
 	for _, p := range pods {
-		if val := p.Annotations[provisioning.KarpenterDoNotEvictPodAnnotation]; val == "true" {
+		if val := p.Annotations[provisioning.DoNotEvictPodAnnotationKey]; val == "true" {
 			logging.FromContext(ctx).Debugf("Unable to drain node %s, pod %s has do-not-evict annotation", node.Name, p.Name)
 			return false, nil
 		}

pkg/utils/functional/functional.go

@@ -15,8 +15,6 @@ limitations under the License.
 package functional
 
 import (
-	"encoding/json"
-	"reflect"
 	"strings"
 	"time"
 
@@ -123,15 +121,3 @@ func MaxDuration(durations ...time.Duration) time.Duration {
 	}
 	return max
 }
-
-func JsonEquals(a, b interface{}) bool {
-	aJson, err := json.Marshal(a)
-	if err != nil {
-		panic(err)
-	}
-	bJson, err := json.Marshal(b)
-	if err != nil {
-		panic(err)
-	}
-	return reflect.DeepEqual(string(aJson), string(bJson))
-}

pkg/utils/node/predicates.go

@@ -17,7 +17,6 @@ package node
 import (
 	"time"
 
-	"github.com/awslabs/karpenter/pkg/apis/provisioning/v1alpha3"
 	v1 "k8s.io/api/core/v1"
 )
 
@@ -33,18 +32,6 @@ func FailedToJoin(node *v1.Node, gracePeriod time.Duration) bool {
 	return condition.LastHeartbeatTime.IsZero()
 }
 
-func IsPastEmptyTTL(node *v1.Node) bool {
-	ttl, ok := node.Annotations[v1alpha3.ProvisionerTTLAfterEmptyKey]
-	if !ok {
-		return false
-	}
-	ttlTime, err := time.Parse(time.RFC3339, ttl)
-	if err != nil {
-		return false
-	}
-	return time.Now().After(ttlTime)
-}
-
 func getNodeCondition(conditions []v1.NodeCondition, match v1.NodeConditionType) v1.NodeCondition {
 	for _, condition := range conditions {
 		if condition.Type == match {