-
Notifications
You must be signed in to change notification settings - Fork 93
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: Add readiness probe for Kmesh daemon #1238
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: Ravjot Singh <[email protected]>
Signed-off-by: Ravjot Singh <[email protected]>
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Signed-off-by: Ravjot Singh <[email protected]>
Signed-off-by: Ravjot Singh <[email protected]>
Codecov ReportAll modified and coverable lines are covered by tests ✅
see 1 file with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
Use: "readiness", | ||
Short: "Check if the Kmesh daemon is healthy and ready", | ||
Run: func(cmd *cobra.Command, args []string) { | ||
readinessURL := "http://localhost:8080/ready" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
8080 is a commonly used port. You can choose one after 15000+.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
okiee @LiZhenCheng9527 i will use port 15050
} | ||
|
||
func init() { | ||
rootCmd.AddCommand(readinessCmd) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove it.
Add it to ./ctl/common/common.go
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
okiee
* limitations under the License. | ||
*/ | ||
|
||
package cmd |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Change a more appropriate package name
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
okiee @LiZhenCheng9527
"kmesh.net/kmesh/pkg/logger" | ||
|
||
"github.com/sirupsen/logrus" | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changes to go-lint errors for all packages based on the error message from github action
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i will change them
var log = logger.NewLoggerScope("main") | ||
log := logger.NewLoggerScope("main") | ||
|
||
startReadinessServer(log) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The entrance should not be placed here. It should be placed after kmesh-daemon has finished starting.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i will update this
@@ -0,0 +1,25 @@ | |||
## kmeshctl authz status |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why does this commit include changes to these files? Did you include changes from another branch?
// startReadinessServer starts an HTTP server on :8080 with /ready | ||
func startReadinessServer(log *logrus.Entry) { | ||
mux := http.NewServeMux() | ||
mux.HandleFunc("/ready", readinessHandler) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Prefer embedding this into status_server.go
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
okiee @hzxuzhonghu i will update this one
@@ -25,4 +11,7 @@ kmeshctl authz [flags] | |||
### SEE ALSO | |||
|
|||
* [kmeshctl](kmeshctl.md) - Kmesh command line tools to operate and debug Kmesh | |||
* [kmeshctl authz disable](kmeshctl_authz_disable.md) - Disable xdp authz eBPF program for Kmesh's authz offloading | |||
* [kmeshctl authz enable](kmeshctl_authz_enable.md) - Enable xdp authz eBPF program for Kmesh's authz offloading | |||
* [kmeshctl authz status](kmeshctl_authz_status.md) - Display the current authorization status |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How could the other pr passed without updating this doc
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I missed to run gen check on other pr, i have raised a pr to solve this issue
/ready
endpoint.Test
Build kmeshctl
Ensure the daemon is running, then:
If the daemon is ready, you see “Kmesh daemon is ready!”, otherwise you see a non-zero exit.
Check Readiness Manually
You can manually check the readiness probe using curl:
If the daemon is ready, it will return:
OK
with HTTP 200 (OK).
If it is not ready, it will return:
Not Ready
with HTTP 503 (Service Unavailable).
Fixes #495