From f22715639623827fee4baa933ab190eeeb16ff70 Mon Sep 17 00:00:00 2001 From: sadath-12 Date: Wed, 20 Dec 2023 15:23:42 +0530 Subject: [PATCH 1/4] chore: nodeclaim nodepool utils cleanup Signed-off-by: sadath-12 --- pkg/controllers/node/termination/controller.go | 2 +- pkg/controllers/nodeclaim/lifecycle/controller.go | 9 +++++---- pkg/controllers/nodepool/hash/controller.go | 2 +- pkg/utils/nodeclaim/nodeclaim.go | 7 +------ pkg/utils/nodepool/nodepool.go | 4 +--- 5 files changed, 9 insertions(+), 15 deletions(-) diff --git a/pkg/controllers/node/termination/controller.go b/pkg/controllers/node/termination/controller.go index ab1e1ea1e0..5fa248d0e9 100644 --- a/pkg/controllers/node/termination/controller.go +++ b/pkg/controllers/node/termination/controller.go @@ -97,7 +97,7 @@ func (c *Controller) Finalize(ctx context.Context, node *v1.Node) (reconcile.Res } return reconcile.Result{RequeueAfter: 1 * time.Second}, nil } - if err := c.cloudProvider.Delete(ctx, nodeclaimutil.NewFromNode(node)); cloudprovider.IgnoreNodeClaimNotFoundError(err) != nil { + if err := c.cloudProvider.Delete(ctx, maNewFromNode(node)); cloudprovider.IgnoreNodeClaimNotFoundError(err) != nil { return reconcile.Result{}, fmt.Errorf("terminating cloudprovider instance, %w", err) } if err := c.removeFinalizer(ctx, node); err != nil { diff --git a/pkg/controllers/nodeclaim/lifecycle/controller.go b/pkg/controllers/nodeclaim/lifecycle/controller.go index f3261531da..56123334d7 100644 --- a/pkg/controllers/nodeclaim/lifecycle/controller.go +++ b/pkg/controllers/nodeclaim/lifecycle/controller.go @@ -40,9 +40,9 @@ import ( "sigs.k8s.io/karpenter/pkg/apis/v1beta1" "sigs.k8s.io/karpenter/pkg/cloudprovider" + nodeclaimutil "sigs.k8s.io/karpenter/pkg/utils/nodeclaim" "sigs.k8s.io/karpenter/pkg/events" operatorcontroller "sigs.k8s.io/karpenter/pkg/operator/controller" - nodeclaimutil "sigs.k8s.io/karpenter/pkg/utils/nodeclaim" "sigs.k8s.io/karpenter/pkg/utils/result" ) @@ -86,7 +86,7 @@ func (c *Controller) Reconcile(ctx context.Context, nodeClaim *v1beta1.NodeClaim stored := nodeClaim.DeepCopy() controllerutil.AddFinalizer(nodeClaim, v1beta1.TerminationFinalizer) if !equality.Semantic.DeepEqual(nodeClaim, stored) { - if err := nodeclaimutil.Patch(ctx, c.kubeClient, stored, nodeClaim); err != nil { + if err := c.kubeClient.Patch(ctx, nodeClaim, client.MergeFrom(stored)); err != nil { return reconcile.Result{}, client.IgnoreNotFound(err) } } @@ -106,10 +106,11 @@ func (c *Controller) Reconcile(ctx context.Context, nodeClaim *v1beta1.NodeClaim } if !equality.Semantic.DeepEqual(stored, nodeClaim) { statusCopy := nodeClaim.DeepCopy() - if err := nodeclaimutil.Patch(ctx, c.kubeClient, stored, nodeClaim); err != nil { + if err := c.kubeClient.Patch(ctx, nodeClaim, client.MergeFrom(stored)); err != nil { return reconcile.Result{}, client.IgnoreNotFound(multierr.Append(errs, err)) } - if err := nodeclaimutil.PatchStatus(ctx, c.kubeClient, stored, statusCopy); err != nil { + + if err := c.kubeClient.Status().Patch(ctx, statusCopy, client.MergeFrom(stored)); err != nil { return reconcile.Result{}, client.IgnoreNotFound(multierr.Append(errs, err)) } // We sleep here after a patch operation since we want to ensure that we are able to read our own writes diff --git a/pkg/controllers/nodepool/hash/controller.go b/pkg/controllers/nodepool/hash/controller.go index f447f18194..712ab5bd75 100644 --- a/pkg/controllers/nodepool/hash/controller.go +++ b/pkg/controllers/nodepool/hash/controller.go @@ -49,7 +49,7 @@ func NewController(kubeClient client.Client) operatorcontroller.Controller { // Reconcile the resource func (c *Controller) Reconcile(ctx context.Context, np *v1beta1.NodePool) (reconcile.Result, error) { stored := np.DeepCopy() - np.Annotations = lo.Assign(np.Annotations, nodepoolutil.HashAnnotation(np)) + np.Annotations = lo.Assign(np.Annotations, map[string]string{v1beta1.NodePoolHashAnnotationKey: np.Hash()}) if !equality.Semantic.DeepEqual(stored, np) { if err := nodepoolutil.Patch(ctx, c.kubeClient, stored, np); err != nil { diff --git a/pkg/utils/nodeclaim/nodeclaim.go b/pkg/utils/nodeclaim/nodeclaim.go index 1777cef4d1..4abb8e2b09 100644 --- a/pkg/utils/nodeclaim/nodeclaim.go +++ b/pkg/utils/nodeclaim/nodeclaim.go @@ -236,12 +236,7 @@ func Delete(ctx context.Context, c client.Client, nodeClaim *v1beta1.NodeClaim) return c.Delete(ctx, nodeClaim) } -func CreatedCounter(nodeClaim *v1beta1.NodeClaim, reason string) prometheus.Counter { - return metrics.NodeClaimsCreatedCounter.With(prometheus.Labels{ - metrics.ReasonLabel: reason, - metrics.NodePoolLabel: nodeClaim.Labels[v1beta1.NodePoolLabelKey], - }) -} + func LaunchedCounter(nodeClaim *v1beta1.NodeClaim) prometheus.Counter { return metrics.NodeClaimsLaunchedCounter.With(prometheus.Labels{ diff --git a/pkg/utils/nodepool/nodepool.go b/pkg/utils/nodepool/nodepool.go index 3313b7fbcf..cb3a22565c 100644 --- a/pkg/utils/nodepool/nodepool.go +++ b/pkg/utils/nodepool/nodepool.go @@ -49,6 +49,4 @@ func PatchStatus(ctx context.Context, c client.Client, stored, nodePool *v1beta1 return c.Status().Patch(ctx, nodePool, client.MergeFrom(stored)) } -func HashAnnotation(nodePool *v1beta1.NodePool) map[string]string { - return map[string]string{v1beta1.NodePoolHashAnnotationKey: nodePool.Hash()} -} + From 37e72faa4394bb5a73ea260d46991cdea97315ff Mon Sep 17 00:00:00 2001 From: sadath-12 Date: Sat, 23 Dec 2023 12:35:39 +0530 Subject: [PATCH 2/4] stalebot: kind/ prefix handle Signed-off-by: sadath-12 --- .github/workflows/stale.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/stale.yaml b/.github/workflows/stale.yaml index 547fe1f5f8..91afab07c8 100644 --- a/.github/workflows/stale.yaml +++ b/.github/workflows/stale.yaml @@ -19,7 +19,7 @@ jobs: with: repo-token: ${{ secrets.GITHUB_TOKEN }} stale-issue-message: 'This issue has been inactive for 14 days. StaleBot will close this stale issue after 14 more days of inactivity.' - exempt-issue-labels: 'bug,chore,feature,documentation,testing,operational-excellence,automation,roadmap' + exempt-issue-labels: 'bug,chore,feature,documentation,testing,operational-excellence,automation,roadmap,kind/*' stale-issue-label: 'lifecycle/stale' close-issue-label: 'lifecycle/closed' stale-pr-message: 'This PR has been inactive for 14 days. StaleBot will close this stale PR after 14 more days of inactivity.' From 41cdfcf2e919904b96b77eb83ccbcab629f1693b Mon Sep 17 00:00:00 2001 From: sadath-12 Date: Sun, 24 Dec 2023 09:51:49 +0530 Subject: [PATCH 3/4] match main Signed-off-by: sadath-12 --- pkg/controllers/node/termination/controller.go | 2 +- pkg/controllers/nodeclaim/lifecycle/controller.go | 9 ++++----- pkg/controllers/nodepool/hash/controller.go | 2 +- pkg/utils/nodeclaim/nodeclaim.go | 7 ++++++- pkg/utils/nodepool/nodepool.go | 4 +++- 5 files changed, 15 insertions(+), 9 deletions(-) diff --git a/pkg/controllers/node/termination/controller.go b/pkg/controllers/node/termination/controller.go index 5fa248d0e9..ab1e1ea1e0 100644 --- a/pkg/controllers/node/termination/controller.go +++ b/pkg/controllers/node/termination/controller.go @@ -97,7 +97,7 @@ func (c *Controller) Finalize(ctx context.Context, node *v1.Node) (reconcile.Res } return reconcile.Result{RequeueAfter: 1 * time.Second}, nil } - if err := c.cloudProvider.Delete(ctx, maNewFromNode(node)); cloudprovider.IgnoreNodeClaimNotFoundError(err) != nil { + if err := c.cloudProvider.Delete(ctx, nodeclaimutil.NewFromNode(node)); cloudprovider.IgnoreNodeClaimNotFoundError(err) != nil { return reconcile.Result{}, fmt.Errorf("terminating cloudprovider instance, %w", err) } if err := c.removeFinalizer(ctx, node); err != nil { diff --git a/pkg/controllers/nodeclaim/lifecycle/controller.go b/pkg/controllers/nodeclaim/lifecycle/controller.go index 56123334d7..f3261531da 100644 --- a/pkg/controllers/nodeclaim/lifecycle/controller.go +++ b/pkg/controllers/nodeclaim/lifecycle/controller.go @@ -40,9 +40,9 @@ import ( "sigs.k8s.io/karpenter/pkg/apis/v1beta1" "sigs.k8s.io/karpenter/pkg/cloudprovider" - nodeclaimutil "sigs.k8s.io/karpenter/pkg/utils/nodeclaim" "sigs.k8s.io/karpenter/pkg/events" operatorcontroller "sigs.k8s.io/karpenter/pkg/operator/controller" + nodeclaimutil "sigs.k8s.io/karpenter/pkg/utils/nodeclaim" "sigs.k8s.io/karpenter/pkg/utils/result" ) @@ -86,7 +86,7 @@ func (c *Controller) Reconcile(ctx context.Context, nodeClaim *v1beta1.NodeClaim stored := nodeClaim.DeepCopy() controllerutil.AddFinalizer(nodeClaim, v1beta1.TerminationFinalizer) if !equality.Semantic.DeepEqual(nodeClaim, stored) { - if err := c.kubeClient.Patch(ctx, nodeClaim, client.MergeFrom(stored)); err != nil { + if err := nodeclaimutil.Patch(ctx, c.kubeClient, stored, nodeClaim); err != nil { return reconcile.Result{}, client.IgnoreNotFound(err) } } @@ -106,11 +106,10 @@ func (c *Controller) Reconcile(ctx context.Context, nodeClaim *v1beta1.NodeClaim } if !equality.Semantic.DeepEqual(stored, nodeClaim) { statusCopy := nodeClaim.DeepCopy() - if err := c.kubeClient.Patch(ctx, nodeClaim, client.MergeFrom(stored)); err != nil { + if err := nodeclaimutil.Patch(ctx, c.kubeClient, stored, nodeClaim); err != nil { return reconcile.Result{}, client.IgnoreNotFound(multierr.Append(errs, err)) } - - if err := c.kubeClient.Status().Patch(ctx, statusCopy, client.MergeFrom(stored)); err != nil { + if err := nodeclaimutil.PatchStatus(ctx, c.kubeClient, stored, statusCopy); err != nil { return reconcile.Result{}, client.IgnoreNotFound(multierr.Append(errs, err)) } // We sleep here after a patch operation since we want to ensure that we are able to read our own writes diff --git a/pkg/controllers/nodepool/hash/controller.go b/pkg/controllers/nodepool/hash/controller.go index 712ab5bd75..f447f18194 100644 --- a/pkg/controllers/nodepool/hash/controller.go +++ b/pkg/controllers/nodepool/hash/controller.go @@ -49,7 +49,7 @@ func NewController(kubeClient client.Client) operatorcontroller.Controller { // Reconcile the resource func (c *Controller) Reconcile(ctx context.Context, np *v1beta1.NodePool) (reconcile.Result, error) { stored := np.DeepCopy() - np.Annotations = lo.Assign(np.Annotations, map[string]string{v1beta1.NodePoolHashAnnotationKey: np.Hash()}) + np.Annotations = lo.Assign(np.Annotations, nodepoolutil.HashAnnotation(np)) if !equality.Semantic.DeepEqual(stored, np) { if err := nodepoolutil.Patch(ctx, c.kubeClient, stored, np); err != nil { diff --git a/pkg/utils/nodeclaim/nodeclaim.go b/pkg/utils/nodeclaim/nodeclaim.go index 4abb8e2b09..1777cef4d1 100644 --- a/pkg/utils/nodeclaim/nodeclaim.go +++ b/pkg/utils/nodeclaim/nodeclaim.go @@ -236,7 +236,12 @@ func Delete(ctx context.Context, c client.Client, nodeClaim *v1beta1.NodeClaim) return c.Delete(ctx, nodeClaim) } - +func CreatedCounter(nodeClaim *v1beta1.NodeClaim, reason string) prometheus.Counter { + return metrics.NodeClaimsCreatedCounter.With(prometheus.Labels{ + metrics.ReasonLabel: reason, + metrics.NodePoolLabel: nodeClaim.Labels[v1beta1.NodePoolLabelKey], + }) +} func LaunchedCounter(nodeClaim *v1beta1.NodeClaim) prometheus.Counter { return metrics.NodeClaimsLaunchedCounter.With(prometheus.Labels{ diff --git a/pkg/utils/nodepool/nodepool.go b/pkg/utils/nodepool/nodepool.go index cb3a22565c..3313b7fbcf 100644 --- a/pkg/utils/nodepool/nodepool.go +++ b/pkg/utils/nodepool/nodepool.go @@ -49,4 +49,6 @@ func PatchStatus(ctx context.Context, c client.Client, stored, nodePool *v1beta1 return c.Status().Patch(ctx, nodePool, client.MergeFrom(stored)) } - +func HashAnnotation(nodePool *v1beta1.NodePool) map[string]string { + return map[string]string{v1beta1.NodePoolHashAnnotationKey: nodePool.Hash()} +} From 97e781b657b0ed32269085e1b737b8a38c3cfbcb Mon Sep 17 00:00:00 2001 From: sadath-12 Date: Mon, 25 Dec 2023 02:11:48 +0530 Subject: [PATCH 4/4] kind/* split Signed-off-by: sadath-12 --- .github/workflows/stale.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/stale.yaml b/.github/workflows/stale.yaml index 91afab07c8..de1be38ed4 100644 --- a/.github/workflows/stale.yaml +++ b/.github/workflows/stale.yaml @@ -19,7 +19,7 @@ jobs: with: repo-token: ${{ secrets.GITHUB_TOKEN }} stale-issue-message: 'This issue has been inactive for 14 days. StaleBot will close this stale issue after 14 more days of inactivity.' - exempt-issue-labels: 'bug,chore,feature,documentation,testing,operational-excellence,automation,roadmap,kind/*' + exempt-issue-labels: 'bug,chore,feature,documentation,testing,operational-excellence,automation,roadmap,kind/cleanup,kind/documentation,kind/regression,kind/flake,kind/bug,kind/deprecation,kind/support,kind/feature,kind/failing-test,kind/api-change' stale-issue-label: 'lifecycle/stale' close-issue-label: 'lifecycle/closed' stale-pr-message: 'This PR has been inactive for 14 days. StaleBot will close this stale PR after 14 more days of inactivity.'