From 715c44694c4e8dfcffedf4cd607e9c4dccecfb60 Mon Sep 17 00:00:00 2001 From: Amanuel Engeda <74629455+engedaam@users.noreply.github.com> Date: Tue, 2 Jul 2024 23:21:51 -0700 Subject: [PATCH] feat: Adding print columns for v1 NodePool (#1365) --- kwok/charts/crds/karpenter.sh_nodepools.yaml | 17 +++++ pkg/apis/crds/karpenter.sh_nodepools.yaml | 17 +++++ pkg/apis/v1/nodepool.go | 7 +- .../nodepool/counter/controller.go | 19 +++++- .../nodepool/counter/suite_test.go | 64 ++++++++++++++----- 5 files changed, 104 insertions(+), 20 deletions(-) diff --git a/kwok/charts/crds/karpenter.sh_nodepools.yaml b/kwok/charts/crds/karpenter.sh_nodepools.yaml index 4c225ffc40..a1fc00dddd 100644 --- a/kwok/charts/crds/karpenter.sh_nodepools.yaml +++ b/kwok/charts/crds/karpenter.sh_nodepools.yaml @@ -20,9 +20,26 @@ spec: - jsonPath: .spec.template.spec.nodeClassRef.name name: NodeClass type: string + - jsonPath: .status.resources.nodes + name: Nodes + type: string + - jsonPath: .status.conditions[?(@.type=="Ready")].status + name: Ready + type: string + - jsonPath: .metadata.creationTimestamp + name: Age + type: date - jsonPath: .spec.weight name: Weight priority: 1 + type: integer + - jsonPath: .status.resources.cpu + name: CPU + priority: 1 + type: string + - jsonPath: .status.resources.memory + name: Memory + priority: 1 type: string name: v1 schema: diff --git a/pkg/apis/crds/karpenter.sh_nodepools.yaml b/pkg/apis/crds/karpenter.sh_nodepools.yaml index 9196652280..b0b026cd5b 100644 --- a/pkg/apis/crds/karpenter.sh_nodepools.yaml +++ b/pkg/apis/crds/karpenter.sh_nodepools.yaml @@ -20,9 +20,26 @@ spec: - jsonPath: .spec.template.spec.nodeClassRef.name name: NodeClass type: string + - jsonPath: .status.resources.nodes + name: Nodes + type: string + - jsonPath: .status.conditions[?(@.type=="Ready")].status + name: Ready + type: string + - jsonPath: .metadata.creationTimestamp + name: Age + type: date - jsonPath: .spec.weight name: Weight priority: 1 + type: integer + - jsonPath: .status.resources.cpu + name: CPU + priority: 1 + type: string + - jsonPath: .status.resources.memory + name: Memory + priority: 1 type: string name: v1 schema: diff --git a/pkg/apis/v1/nodepool.go b/pkg/apis/v1/nodepool.go index 8b5a2c20ec..739c0d3d1f 100644 --- a/pkg/apis/v1/nodepool.go +++ b/pkg/apis/v1/nodepool.go @@ -198,7 +198,12 @@ type ObjectMeta struct { // +kubebuilder:object:root=true // +kubebuilder:resource:path=nodepools,scope=Cluster,categories=karpenter // +kubebuilder:printcolumn:name="NodeClass",type="string",JSONPath=".spec.template.spec.nodeClassRef.name",description="" -// +kubebuilder:printcolumn:name="Weight",type="string",JSONPath=".spec.weight",priority=1,description="" +// +kubebuilder:printcolumn:name="Nodes",type="string",JSONPath=".status.resources.nodes",description="" +// +kubebuilder:printcolumn:name="Ready",type="string",JSONPath=".status.conditions[?(@.type==\"Ready\")].status",description="" +// +kubebuilder:printcolumn:name="Age",type="date",JSONPath=".metadata.creationTimestamp",description="" +// +kubebuilder:printcolumn:name="Weight",type="integer",JSONPath=".spec.weight",priority=1,description="" +// +kubebuilder:printcolumn:name="CPU",type="string",JSONPath=".status.resources.cpu",priority=1,description="" +// +kubebuilder:printcolumn:name="Memory",type="string",JSONPath=".status.resources.memory",priority=1,description="" // +kubebuilder:subresource:status type NodePool struct { metav1.TypeMeta `json:",inline"` diff --git a/pkg/controllers/nodepool/counter/controller.go b/pkg/controllers/nodepool/counter/controller.go index e61501223c..a808f5d4dd 100644 --- a/pkg/controllers/nodepool/counter/controller.go +++ b/pkg/controllers/nodepool/counter/controller.go @@ -18,6 +18,7 @@ package counter import ( "context" + "fmt" "time" "k8s.io/apimachinery/pkg/api/equality" @@ -30,7 +31,6 @@ import ( "sigs.k8s.io/karpenter/pkg/apis/v1beta1" "sigs.k8s.io/karpenter/pkg/controllers/state" - "sigs.k8s.io/karpenter/pkg/utils/functional" v1 "k8s.io/api/core/v1" "sigs.k8s.io/controller-runtime/pkg/client" @@ -47,6 +47,16 @@ type Controller struct { cluster *state.Cluster } +var ResourceNode = v1.ResourceName("nodes") + +var BaseResources = v1.ResourceList{ + v1.ResourceCPU: resource.MustParse("0"), + v1.ResourceMemory: resource.MustParse("0"), + v1.ResourcePods: resource.MustParse("0"), + v1.ResourceEphemeralStorage: resource.MustParse("0"), + ResourceNode: resource.MustParse("0"), +} + // NewController is a constructor func NewController(kubeClient client.Client, cluster *state.Cluster) *Controller { return &Controller{ @@ -77,7 +87,8 @@ func (c *Controller) Reconcile(ctx context.Context, nodePool *v1beta1.NodePool) } func (c *Controller) resourceCountsFor(ownerLabel string, ownerName string) v1.ResourceList { - var res v1.ResourceList + res := BaseResources.DeepCopy() + nodeCount := 0 // Record all resources provisioned by the nodepools, we look at the cluster state nodes as their capacity // is accurately reported even for nodes that haven't fully started yet. This allows us to update our nodepool // status immediately upon node creation instead of waiting for the node to become ready. @@ -89,10 +100,12 @@ func (c *Controller) resourceCountsFor(ownerLabel string, ownerName string) v1.R } if n.Labels()[ownerLabel] == ownerName { res = resources.MergeInto(res, n.Capacity()) + nodeCount += 1 } return true }) - return functional.FilterMap(res, func(_ v1.ResourceName, v resource.Quantity) bool { return !v.IsZero() }) + res[ResourceNode] = resource.MustParse(fmt.Sprintf("%d", nodeCount)) + return res } func (c *Controller) Register(_ context.Context, m manager.Manager) error { diff --git a/pkg/controllers/nodepool/counter/suite_test.go b/pkg/controllers/nodepool/counter/suite_test.go index 029a64a968..67726807b2 100644 --- a/pkg/controllers/nodepool/counter/suite_test.go +++ b/pkg/controllers/nodepool/counter/suite_test.go @@ -29,6 +29,7 @@ import ( clock "k8s.io/utils/clock/testing" "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/karpenter/pkg/utils/resources" . "sigs.k8s.io/karpenter/pkg/utils/testing" . "sigs.k8s.io/karpenter/pkg/test/expectations" @@ -76,6 +77,7 @@ var _ = AfterSuite(func() { var nodePool *v1beta1.NodePool var nodeClaim, nodeClaim2 *v1beta1.NodeClaim +var expected v1.ResourceList var _ = Describe("Counter", func() { BeforeEach(func() { @@ -110,11 +112,18 @@ var _ = Describe("Counter", func() { }, }, }) + expected = counter.BaseResources.DeepCopy() ExpectApplied(ctx, env.Client, nodePool) ExpectObjectReconciled(ctx, env.Client, nodePoolInformerController, nodePool) ExpectObjectReconciled(ctx, env.Client, nodePoolController, nodePool) nodePool = ExpectExists(ctx, env.Client, nodePool) }) + It("should set well-known resource to zero when no nodes exist in the cluster", func() { + ExpectObjectReconciled(ctx, env.Client, nodePoolController, nodePool) + nodePool = ExpectExists(ctx, env.Client, nodePool) + + Expect(nodePool.Status.Resources).To(BeComparableTo(expected)) + }) It("should set the counter from the nodeClaim and then to the node when it initializes", func() { ExpectApplied(ctx, env.Client, node, nodeClaim) // Don't initialize the node yet @@ -126,7 +135,9 @@ var _ = Describe("Counter", func() { ExpectObjectReconciled(ctx, env.Client, nodePoolController, nodePool) nodePool = ExpectExists(ctx, env.Client, nodePool) - Expect(nodePool.Status.Resources).To(BeEquivalentTo(nodeClaim.Status.Capacity)) + expected = resources.MergeInto(expected, nodeClaim.Status.Capacity) + expected[v1.ResourceName("nodes")] = resource.MustParse("1") + Expect(nodePool.Status.Resources).To(BeComparableTo(expected)) // Change the node capacity to be different than the nodeClaim capacity node.Status.Capacity = v1.ResourceList{ @@ -144,7 +155,10 @@ var _ = Describe("Counter", func() { ExpectObjectReconciled(ctx, env.Client, nodePoolController, nodePool) nodePool = ExpectExists(ctx, env.Client, nodePool) - Expect(nodePool.Status.Resources).To(BeEquivalentTo(node.Status.Capacity)) + expected = counter.BaseResources.DeepCopy() + expected = resources.MergeInto(expected, node.Status.Capacity) + expected[v1.ResourceName("nodes")] = resource.MustParse("1") + Expect(nodePool.Status.Resources).To(BeComparableTo(expected)) }) It("should increase the counter when new nodes are created", func() { ExpectApplied(ctx, env.Client, node, nodeClaim) @@ -154,8 +168,13 @@ var _ = Describe("Counter", func() { nodePool = ExpectExists(ctx, env.Client, nodePool) // Should equal both the nodeClaim and node capacity - Expect(nodePool.Status.Resources).To(BeEquivalentTo(nodeClaim.Status.Capacity)) - Expect(nodePool.Status.Resources).To(BeEquivalentTo(node.Status.Capacity)) + expected = resources.MergeInto(expected, nodeClaim.Status.Capacity) + expected[v1.ResourceName("nodes")] = resource.MustParse("1") + Expect(nodePool.Status.Resources).To(BeComparableTo(expected)) + expected = counter.BaseResources.DeepCopy() + expected = resources.MergeInto(expected, node.Status.Capacity) + expected[v1.ResourceName("nodes")] = resource.MustParse("1") + Expect(nodePool.Status.Resources).To(BeComparableTo(expected)) }) It("should decrease the counter when an existing node is deleted", func() { ExpectApplied(ctx, env.Client, node, nodeClaim, node2, nodeClaim2) @@ -165,13 +184,14 @@ var _ = Describe("Counter", func() { nodePool = ExpectExists(ctx, env.Client, nodePool) // Should equal the sums of the nodeClaims and nodes - resources := v1.ResourceList{ - v1.ResourceCPU: resource.MustParse("600m"), - v1.ResourcePods: resource.MustParse("1256"), - v1.ResourceMemory: resource.MustParse("6Gi"), + res := v1.ResourceList{ + v1.ResourceCPU: resource.MustParse("600m"), + v1.ResourcePods: resource.MustParse("1256"), + v1.ResourceMemory: resource.MustParse("6Gi"), + v1.ResourceName("nodes"): resource.MustParse("2"), } - Expect(nodePool.Status.Resources).To(BeEquivalentTo(resources)) - Expect(nodePool.Status.Resources).To(BeEquivalentTo(resources)) + expected = resources.MergeInto(expected, res) + Expect(nodePool.Status.Resources).To(BeComparableTo(expected)) ExpectDeleted(ctx, env.Client, node, nodeClaim) ExpectReconcileSucceeded(ctx, nodeController, client.ObjectKeyFromObject(node)) @@ -180,10 +200,16 @@ var _ = Describe("Counter", func() { nodePool = ExpectExists(ctx, env.Client, nodePool) // Should equal both the nodeClaim and node capacity - Expect(nodePool.Status.Resources).To(BeEquivalentTo(nodeClaim2.Status.Capacity)) - Expect(nodePool.Status.Resources).To(BeEquivalentTo(node2.Status.Capacity)) + expected = counter.BaseResources.DeepCopy() + expected = resources.MergeInto(expected, nodeClaim2.Status.Capacity) + expected[v1.ResourceName("nodes")] = resource.MustParse("1") + Expect(nodePool.Status.Resources).To(BeComparableTo(expected)) + expected = counter.BaseResources.DeepCopy() + expected = resources.MergeInto(expected, node2.Status.Capacity) + expected[v1.ResourceName("nodes")] = resource.MustParse("1") + Expect(nodePool.Status.Resources).To(BeComparableTo(expected)) }) - It("should nil out the counter when all nodes are deleted", func() { + It("should zero out the counter when all nodes are deleted", func() { ExpectApplied(ctx, env.Client, node, nodeClaim) ExpectMakeNodesAndNodeClaimsInitializedAndStateUpdated(ctx, env.Client, nodeController, nodeClaimController, []*v1.Node{node}, []*v1beta1.NodeClaim{nodeClaim}) @@ -191,8 +217,13 @@ var _ = Describe("Counter", func() { nodePool = ExpectExists(ctx, env.Client, nodePool) // Should equal both the nodeClaim and node capacity - Expect(nodePool.Status.Resources).To(BeEquivalentTo(nodeClaim.Status.Capacity)) - Expect(nodePool.Status.Resources).To(BeEquivalentTo(node.Status.Capacity)) + expected = resources.MergeInto(expected, nodeClaim.Status.Capacity) + expected[v1.ResourceName("nodes")] = resource.MustParse("1") + Expect(nodePool.Status.Resources).To(BeComparableTo(expected)) + expected = counter.BaseResources.DeepCopy() + expected = resources.MergeInto(expected, node.Status.Capacity) + expected[v1.ResourceName("nodes")] = resource.MustParse("1") + Expect(nodePool.Status.Resources).To(BeComparableTo(expected)) ExpectDeleted(ctx, env.Client, node, nodeClaim) @@ -200,6 +231,7 @@ var _ = Describe("Counter", func() { ExpectReconcileSucceeded(ctx, nodeClaimController, client.ObjectKeyFromObject(nodeClaim)) ExpectObjectReconciled(ctx, env.Client, nodePoolController, nodePool) nodePool = ExpectExists(ctx, env.Client, nodePool) - Expect(nodePool.Status.Resources).To(BeNil()) + expected = counter.BaseResources.DeepCopy() + Expect(nodePool.Status.Resources).To(BeComparableTo(expected)) }) })