Added SetDefault() for each Reconcile function (#464)

cmd/webhook/main.go

@@ -39,7 +39,7 @@ var (
 )
 
 type Options struct {
-	Port            int
+	Port int
 }
 
 func main() {

pkg/cloudprovider/aws/constraints.go

@@ -101,7 +101,7 @@ func (c *Constraints) GetSubnetTagKey() *string {
 	return aws.String(subnetTag)
 }
 
-func (c *Constraints) Validate(ctx context.Context)  (errs *apis.FieldError) {
+func (c *Constraints) Validate(ctx context.Context) (errs *apis.FieldError) {
 	return errs.Also(
 		c.validateAllowedLabels(ctx),
 		c.validateCapacityType(ctx),

pkg/cloudprovider/aws/node.go

@@ -74,7 +74,7 @@ func (n *NodeFactory) nodeFrom(instance *ec2.Instance) *v1.Node {
 				v1.ResourceMemory: resource.MustParse("384Gi"),
 			},
 			NodeInfo: v1.NodeSystemInfo{
-				Architecture: aws.StringValue(instance.Architecture),
+				Architecture:    aws.StringValue(instance.Architecture),
 				OperatingSystem: v1alpha1.OperatingSystemLinux,
 			},
 		},

pkg/controllers/controller.go

@@ -17,12 +17,12 @@ package controllers
 import (
 	"context"
 	"fmt"
-	"reflect"
 
-	"github.com/awslabs/karpenter/pkg/apis/provisioning/v1alpha1"
+	"github.com/awslabs/karpenter/pkg/utils/conditions"
 
 	"go.uber.org/zap"
 	"k8s.io/apimachinery/pkg/api/errors"
+	"knative.dev/pkg/apis"
 	"sigs.k8s.io/controller-runtime/pkg/builder"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/handler"
@@ -49,22 +49,24 @@ func (c *GenericController) Reconcile(ctx context.Context, req reconcile.Request
 	}
 	// 2. Copy object for merge patch base
 	persisted := resource.DeepCopyObject()
-	// 3. Set to true to remove race condition
+	// 3. Set defaults to enforce invariants on object being reconciled
+	if _, ok := resource.(apis.Defaultable); ok {
+		resource.(apis.Defaultable).SetDefaults(ctx)
+	}
+	// 4. Set to true to remove race condition where multiple controllers set the status of the same object
 	// TODO: remove status conditions on provisioners
-	if provisioner, ok := resource.(*v1alpha1.Provisioner); ok {
-		provisioner.StatusConditions().MarkTrue(v1alpha1.Active)
+	if conditionsAccessor, ok := resource.(apis.ConditionsAccessor); ok {
+		apis.NewLivingConditionSet(conditions.Active).Manage(conditionsAccessor).MarkTrue(conditions.Active)
 	}
-	// 4. Reconcile
+	// 5. Reconcile
 	if _, err := c.Controller.Reconcile(ctx, resource); err != nil {
 		zap.S().Errorf("Controller failed to reconcile kind %s, %s",
 			resource.GetObjectKind().GroupVersionKind().Kind, err.Error())
 		return reconcile.Result{Requeue: true}, nil
 	}
-	// 5. Update Status using a merge patch
-	if !reflect.DeepEqual(resource, persisted) {
-		if err := c.Status().Patch(ctx, resource, client.MergeFrom(persisted)); err != nil {
-			return reconcile.Result{}, fmt.Errorf("Failed to persist changes to %s, %w", req.NamespacedName, err)
-		}
+	// 6. Update Status using a merge patch
+	if err := c.Status().Patch(ctx, resource, client.MergeFrom(persisted)); err != nil {
+		return reconcile.Result{}, fmt.Errorf("Failed to persist changes to %s, %w", req.NamespacedName, err)
 	}
 	return reconcile.Result{RequeueAfter: c.Interval()}, nil
 }

pkg/controllers/provisioning/v1alpha1/reallocation/controller.go

@@ -67,7 +67,6 @@ func NewController(kubeClient client.Client, coreV1Client corev1.CoreV1Interface
 // Reconcile executes a reallocation control loop for the resource
 func (c *Controller) Reconcile(ctx context.Context, object client.Object) (reconcile.Result, error) {
 	provisioner := object.(*v1alpha1.Provisioner)
-
 	// 1. Set TTL on TTLable Nodes
 	if err := c.utilization.markUnderutilized(ctx, provisioner); err != nil {
 		return reconcile.Result{}, fmt.Errorf("adding ttl and underutilized label, %w", err)

pkg/controllers/provisioning/v1alpha1/reallocation/suite_test.go

@@ -28,7 +28,6 @@ import (
 	v1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	corev1 "k8s.io/client-go/kubernetes/typed/core/v1"
-	"knative.dev/pkg/ptr"
 
 	. "github.com/awslabs/karpenter/pkg/test/expectations"
 	. "github.com/onsi/ginkgo"
@@ -71,8 +70,7 @@ var _ = Describe("Reallocation", func() {
 				Namespace: "default",
 			},
 			Spec: v1alpha1.ProvisionerSpec{
-				Cluster:    &v1alpha1.ClusterSpec{Name: "test-cluster", Endpoint: "http://test-cluster", CABundle: "dGVzdC1jbHVzdGVyCg=="},
-				TTLSeconds: ptr.Int32(300),
+				Cluster: &v1alpha1.ClusterSpec{Name: "test-cluster", Endpoint: "http://test-cluster", CABundle: "dGVzdC1jbHVzdGVyCg=="},
 			},
 		}
 		ctx = context.Background()
@@ -83,8 +81,7 @@ var _ = Describe("Reallocation", func() {
 	})
 
 	Context("Reconciliation", func() {
-		It("should label nodes as underutilized", func() {
-
+		It("should label nodes as underutilized and add TTL", func() {
 			node := test.NodeWith(test.NodeOptions{
 				Labels: map[string]string{
 					v1alpha1.ProvisionerNameLabelKey:      provisioner.Name,

pkg/test/expectations/expectations.go

@@ -20,6 +20,7 @@ import (
 	"time"
 
 	"github.com/awslabs/karpenter/pkg/apis/provisioning/v1alpha1"
+	"github.com/awslabs/karpenter/pkg/utils/conditions"
 	"github.com/awslabs/karpenter/pkg/utils/log"
 	. "github.com/onsi/gomega"
 	v1 "k8s.io/api/core/v1"
@@ -93,7 +94,7 @@ func ExpectEventuallyReconciled(c client.Client, provisioner *v1alpha1.Provision
 	nn := types.NamespacedName{Name: provisioner.GetName(), Namespace: provisioner.GetNamespace()}
 	Eventually(func() bool {
 		Expect(c.Get(context.Background(), nn, provisioner)).To(Succeed())
-		return !provisioner.StatusConditions().GetCondition(v1alpha1.Active).IsUnknown()
+		return !provisioner.StatusConditions().GetCondition(conditions.Active).IsUnknown()
 	}, ReconcilerPropagationTime, RequestInterval).Should(BeTrue(), func() string {
 		return fmt.Sprintf("resources active condition was never updated\n%s", log.Pretty(provisioner))
 	})

pkg/utils/conditions/conditions.go

@@ -0,0 +1,7 @@
+package conditions
+
+import "knative.dev/pkg/apis"
+
+const (
+	Active apis.ConditionType = "Active"
+)