diff --git a/cmd/webhook/main.go b/cmd/webhook/main.go index 5df2e517d5c7..715caa237d31 100644 --- a/cmd/webhook/main.go +++ b/cmd/webhook/main.go @@ -39,7 +39,7 @@ var ( ) type Options struct { - Port int + Port int } func main() { diff --git a/pkg/cloudprovider/aws/constraints.go b/pkg/cloudprovider/aws/constraints.go index b1fdb931c820..929687369a77 100644 --- a/pkg/cloudprovider/aws/constraints.go +++ b/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), diff --git a/pkg/cloudprovider/aws/node.go b/pkg/cloudprovider/aws/node.go index 63e936c2777a..6310e3faf29b 100644 --- a/pkg/cloudprovider/aws/node.go +++ b/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, }, }, diff --git a/pkg/controllers/controller.go b/pkg/controllers/controller.go index 773262ab46c5..6d007c9d3b1a 100644 --- a/pkg/controllers/controller.go +++ b/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 } diff --git a/pkg/controllers/provisioning/v1alpha1/reallocation/controller.go b/pkg/controllers/provisioning/v1alpha1/reallocation/controller.go index 138054692fe2..434a542046b4 100644 --- a/pkg/controllers/provisioning/v1alpha1/reallocation/controller.go +++ b/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) diff --git a/pkg/controllers/provisioning/v1alpha1/reallocation/suite_test.go b/pkg/controllers/provisioning/v1alpha1/reallocation/suite_test.go index 707d512e5695..a016d04373a1 100644 --- a/pkg/controllers/provisioning/v1alpha1/reallocation/suite_test.go +++ b/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, diff --git a/pkg/test/expectations/expectations.go b/pkg/test/expectations/expectations.go index e8d68b9416e1..3f370ba69f47 100644 --- a/pkg/test/expectations/expectations.go +++ b/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)) }) diff --git a/pkg/utils/conditions/conditions.go b/pkg/utils/conditions/conditions.go new file mode 100644 index 000000000000..3a6347058e22 --- /dev/null +++ b/pkg/utils/conditions/conditions.go @@ -0,0 +1,7 @@ +package conditions + +import "knative.dev/pkg/apis" + +const ( + Active apis.ConditionType = "Active" +)