-
Notifications
You must be signed in to change notification settings - Fork 182
Fix orchestrator and template manager health checks #1392
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
base: main
Are you sure you want to change the base?
Conversation
| port = "${port}" | ||
| name = "orchestrator" | ||
| port = "api" | ||
| provider = "nomad" |
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.
This enables the health checks to work. Without it, we default to consul, which doesn't function.
| grpc_use_tls = false | ||
| port = "${port}" |
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.
Don't need this, as we're using HTTP (not GRPC) and it inherits the port of the service block that surrounds the check.
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.
Just for clarity—it inherits the check from orchestrator-proxy, not the orchestrator grpc, right?
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.
It inherits the port, that's all:
job "orchestrator" {
group "client" {
service {
name = "orchestrator"
port = "1234"
check {
# inherits `port = "1234"`
}
}
service {
name = "proxy"
port = "5678"
check {
# inherits `port = "5678"`
}
}
}
}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.
Oh, so there are still two healthchecks, right?
13bf326 to
79bd503
Compare
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.
Lgtm, just wanted to wait if @jakubno does not have any notes about some possible bad interaction with the current orchestrator job deploy?
Before:

After:

Note
Set
provider="nomad", switch gRPC health checks to HTTP (/health), and add a TCP health check for the orchestrator proxy.iac/provider-gcp/nomad/jobs/orchestrator.hcl:orchestrator: addprovider = "nomad"; change health check fromgrpctohttpwithpath = "/health".orchestrator-proxy: addprovider = "nomad"; addtcphealth check.iac/provider-gcp/nomad/jobs/template-manager.hcl:template-manager: addprovider = "nomad"; change health check fromgrpctohttpwithpath = "/health".Written by Cursor Bugbot for commit d6c8455. This will update automatically on new commits. Configure here.