Replace gRPC health probe utility with k8s built-in health probe #1046

fmuyassarov · 2023-01-30T16:10:06Z

Kubernetes 1.23 has introduced native health probes for gRPC which can replace
grpc_health_probe utility. This commit removes baking in grpc_health_probe
binary into the image and updates related health checks to use k8s native gRPC.

netlify · 2023-01-30T16:10:12Z

✅ Deploy Preview for kubernetes-sigs-nfd ready!

Name	Link
🔨 Latest commit	`06036a6`
🔍 Latest deploy log	https://app.netlify.com/sites/kubernetes-sigs-nfd/deploys/650abab7fcd5450008beec5f
😎 Deploy Preview	https://deploy-preview-1046--kubernetes-sigs-nfd.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

ArangoGutierrez

We do register the native gRPC protocol for health checks see:

https://github.com/kubernetes-sigs/node-feature-discovery/blob/master/pkg/nfd-master/nfd-master.go#L236

/lgtm

k8s-ci-robot · 2023-01-31T11:32:35Z

LGTM label has been added.

Git tree hash: a713eb00f5b94fdeabaaef133c0f8c2c2b787501

ArangoGutierrez · 2023-01-31T11:49:59Z

/assign @marquiz

marquiz · 2023-01-31T12:37:58Z

Thanks @fmuyassarov for the patch. I think we're in no hurry with this one. There are probably many pre v1.23 clusters out there and I think e.g. AWS EKS still officially supports v1.22 for some time

/hold

fmuyassarov · 2023-01-31T13:13:28Z

Yep good point. Let's keep it here for some time.

ArangoGutierrez · 2023-01-31T13:52:07Z

Thanks @fmuyassarov for the patch. I think we're in no hurry with this one. There are probably many pre v1.23 clusters out there and I think e.g. AWS EKS still officially supports v1.22 for some time

/hold

What? isn't everyone running latest:nightly in prod? what a world we live in.....

fmuyassarov · 2023-03-13T10:03:17Z

/retest

fmuyassarov · 2023-03-13T10:03:34Z

/test

k8s-ci-robot · 2023-03-13T10:03:36Z

@fmuyassarov: The /test command needs one or more targets.
The following commands are available to trigger required jobs:

/test pull-node-feature-discovery-build-gh-pages-generic
/test pull-node-feature-discovery-build-image-cross-generic
/test pull-node-feature-discovery-build-image-generic
/test pull-node-feature-discovery-build-master
/test pull-node-feature-discovery-verify-docs-master
/test pull-node-feature-discovery-verify-master

Use /test all to run the following jobs that were automatically triggered:

pull-node-feature-discovery-build-image-cross-generic
pull-node-feature-discovery-build-image-generic
pull-node-feature-discovery-build-master
pull-node-feature-discovery-verify-master

In response to this:

/test

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

fmuyassarov · 2023-03-13T10:03:56Z

/test all

ArangoGutierrez · 2023-03-16T09:14:46Z

fmuyassarov let's fix the merge conflict and I'll review again

fmuyassarov · 2023-05-01T03:27:37Z

@ArangoGutierrez sorry for late response on this, rebase is done, but perhaps we can wait a bit more to not breaking possible users of k8s version below v1.23?

ArangoGutierrez · 2023-06-06T18:27:33Z

@marquiz @fmuyassarov should we mark this as milestone for v0.15?

marquiz · 2023-06-07T09:16:51Z

@marquiz @fmuyassarov should we mark this as milestone for v0.15?

v0.15 might be, K8s v1.22 was EOL'd last autumn

fmuyassarov · 2023-06-07T09:46:12Z

@marquiz @fmuyassarov should we mark this as milestone for v0.15?

yes, 0.15 sounds good to me.
/milestone v0.15

k8s-ci-robot · 2023-06-07T09:46:13Z

@fmuyassarov: You must be a member of the kubernetes-sigs/node-feature-discovery-maintainers GitHub team to set the milestone. If you believe you should be able to issue the /milestone command, please contact your Node Feature Discovery Maintainers and have them propose you as an additional delegate for this responsibility.

In response to this:

@marquiz @fmuyassarov should we mark this as milestone for v0.15?

yes, 0.15 sounds good to me.
/milestone v0.15

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

ArangoGutierrez · 2023-06-07T12:34:55Z

/milestone v0.15

ArangoGutierrez · 2023-08-28T10:52:23Z

Let's make this part of #1187

marquiz · 2023-08-28T11:02:34Z

Let's make this part of #1187

What do you mean? They're not related

ArangoGutierrez · 2023-08-28T11:07:28Z

Let's make this part of #1187

What do you mean? They're not related

If we are moving away from GRPC into the API (IOW Kubernetes native objects), moving forward with this makes also more sense.
But 1187 doesn't depend on this.

marquiz · 2023-08-28T11:25:11Z

If we are moving away from GRPC into the API (IOW Kubernetes native objects), moving forward with this makes also more sense.

But this still leverages gRPC. Just uses the K8s internal support for gRPC probes instead of an external tool

ArangoGutierrez · 2023-08-28T12:55:47Z

If we are moving away from GRPC into the API (IOW Kubernetes native objects), moving forward with this makes also more sense.

But this still leverages gRPC. Just uses the K8s internal support for gRPC probes instead of an external tool

Agree, I was thinking just on the Kubernetes native part, and forgot the implementation

k8s-ci-robot · 2023-09-05T18:36:33Z

PR needs rebase.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

marquiz · 2023-09-20T06:58:31Z

@fmuyassarov @ArangoGutierrez I think we could merge this one. K8s v1.22 was EOL'd nearly a year ago and at least all the bigger CSPs (like GKE, EKS, AKS) have officially stopped supporting it, also OpenShift 4.

ArangoGutierrez · 2023-09-20T09:19:58Z

@fmuyassarov @ArangoGutierrez I think we could merge this one. K8s v1.22 was EOL'd nearly a year ago and at least all the bigger CSPs (like GKE, EKS, AKS) have officially stopped supporting it, also OpenShift 4.

Needs rebase ;P
but I support moving forward here

Kubernetes 1.23 has introduced native health probes for gRPC which can replace grpc_health_probe utility. This commit removes baking in grpc_health_probe binary into the image and updates related health checks to use k8s native gRPC. Signed-off-by: Muyassarov, Feruzjon <feruzjon.muyassarov@intel.com>

fmuyassarov · 2023-09-20T09:26:49Z

@fmuyassarov @ArangoGutierrez I think we could merge this one. K8s v1.22 was EOL'd nearly a year ago and at least all the bigger CSPs (like GKE, EKS, AKS) have officially stopped supporting it, also OpenShift 4.

Sounds good. Just rebased.

ArangoGutierrez · 2023-09-20T09:32:03Z

/hold cancel

ArangoGutierrez

/lgtm

k8s-ci-robot · 2023-09-20T09:43:06Z

LGTM label has been added.

Git tree hash: e8d4ef68ca0c62969da35dec89b865018602b614

k8s-ci-robot · 2023-09-20T09:43:07Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ArangoGutierrez, fmuyassarov

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [ArangoGutierrez]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

codecov · 2023-09-20T09:50:50Z

Codecov Report

Merging #1046 (06036a6) into master (a4cea16) will not change coverage.
The diff coverage is n/a.

Additional details and impacted files

@@           Coverage Diff           @@
##           master    #1046   +/-   ##
=======================================
  Coverage   30.02%   30.02%           
=======================================
  Files          56       56           
  Lines        7457     7457           
=======================================
  Hits         2239     2239           
  Misses       4974     4974           
  Partials      244      244

k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Jan 30, 2023

k8s-ci-robot requested review from jjacobelli and zvonkok January 30, 2023 16:10

k8s-ci-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Jan 30, 2023

ArangoGutierrez approved these changes Jan 31, 2023

View reviewed changes

k8s-ci-robot assigned ArangoGutierrez Jan 31, 2023

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jan 31, 2023

k8s-ci-robot assigned marquiz Jan 31, 2023

k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jan 31, 2023

k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 3, 2023

fmuyassarov force-pushed the native-grpc-probe branch from bdaa725 to c71f26c Compare May 1, 2023 03:24

k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 1, 2023

k8s-ci-robot requested review from ArangoGutierrez and marquiz May 1, 2023 03:25

k8s-ci-robot added approved Indicates a PR has been approved by an approver from all required OWNERS files. and removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels May 1, 2023

k8s-ci-robot added this to the v0.15 milestone Jun 7, 2023

k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Sep 5, 2023

fmuyassarov force-pushed the native-grpc-probe branch from c71f26c to 06036a6 Compare September 20, 2023 09:26

k8s-ci-robot removed do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Sep 20, 2023

ArangoGutierrez approved these changes Sep 20, 2023

View reviewed changes

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Sep 20, 2023

k8s-ci-robot merged commit 7847edb into kubernetes-sigs:master Sep 20, 2023
5 checks passed

marquiz mentioned this pull request Dec 20, 2023

Release v0.15 #1483

Closed

24 tasks

marquiz mentioned this pull request Jan 3, 2024

Healthcheck fails when tls is enabled in the helm chart on v0.15.0 #1533

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Replace gRPC health probe utility with k8s built-in health probe #1046

Replace gRPC health probe utility with k8s built-in health probe #1046

fmuyassarov commented Jan 30, 2023

netlify bot commented Jan 30, 2023 •

edited

Loading

ArangoGutierrez left a comment

k8s-ci-robot commented Jan 31, 2023

ArangoGutierrez commented Jan 31, 2023

marquiz commented Jan 31, 2023

fmuyassarov commented Jan 31, 2023

ArangoGutierrez commented Jan 31, 2023

fmuyassarov commented Mar 13, 2023

fmuyassarov commented Mar 13, 2023

k8s-ci-robot commented Mar 13, 2023

fmuyassarov commented Mar 13, 2023

ArangoGutierrez commented Mar 16, 2023

fmuyassarov commented May 1, 2023

ArangoGutierrez commented Jun 6, 2023

marquiz commented Jun 7, 2023

fmuyassarov commented Jun 7, 2023

k8s-ci-robot commented Jun 7, 2023

ArangoGutierrez commented Jun 7, 2023

ArangoGutierrez commented Aug 28, 2023

marquiz commented Aug 28, 2023

ArangoGutierrez commented Aug 28, 2023

marquiz commented Aug 28, 2023

ArangoGutierrez commented Aug 28, 2023

k8s-ci-robot commented Sep 5, 2023

marquiz commented Sep 20, 2023

ArangoGutierrez commented Sep 20, 2023

fmuyassarov commented Sep 20, 2023

ArangoGutierrez commented Sep 20, 2023

ArangoGutierrez left a comment

k8s-ci-robot commented Sep 20, 2023

k8s-ci-robot commented Sep 20, 2023

codecov bot commented Sep 20, 2023

Replace gRPC health probe utility with k8s built-in health probe #1046

Replace gRPC health probe utility with k8s built-in health probe #1046

Conversation

fmuyassarov commented Jan 30, 2023

netlify bot commented Jan 30, 2023 • edited Loading

✅ Deploy Preview for kubernetes-sigs-nfd ready!

ArangoGutierrez left a comment

Choose a reason for hiding this comment

k8s-ci-robot commented Jan 31, 2023

ArangoGutierrez commented Jan 31, 2023

marquiz commented Jan 31, 2023

fmuyassarov commented Jan 31, 2023

ArangoGutierrez commented Jan 31, 2023

fmuyassarov commented Mar 13, 2023

fmuyassarov commented Mar 13, 2023

k8s-ci-robot commented Mar 13, 2023

fmuyassarov commented Mar 13, 2023

ArangoGutierrez commented Mar 16, 2023

fmuyassarov commented May 1, 2023

ArangoGutierrez commented Jun 6, 2023

marquiz commented Jun 7, 2023

fmuyassarov commented Jun 7, 2023

k8s-ci-robot commented Jun 7, 2023

ArangoGutierrez commented Jun 7, 2023

ArangoGutierrez commented Aug 28, 2023

marquiz commented Aug 28, 2023

ArangoGutierrez commented Aug 28, 2023

marquiz commented Aug 28, 2023

ArangoGutierrez commented Aug 28, 2023

k8s-ci-robot commented Sep 5, 2023

marquiz commented Sep 20, 2023

ArangoGutierrez commented Sep 20, 2023

fmuyassarov commented Sep 20, 2023

ArangoGutierrez commented Sep 20, 2023

ArangoGutierrez left a comment

Choose a reason for hiding this comment

k8s-ci-robot commented Sep 20, 2023

k8s-ci-robot commented Sep 20, 2023

codecov bot commented Sep 20, 2023

Codecov Report

netlify bot commented Jan 30, 2023 •

edited

Loading