-
Notifications
You must be signed in to change notification settings - Fork 330
Add option for initial probe delay, timeout, and failure threshholds #1246
Add option for initial probe delay, timeout, and failure threshholds #1246
Conversation
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.
Hey there @frostbyte73 - thanks for the pull request! We should definitely add the option to configure this, but I think while we're here we should support the other various probe options too like TimeoutSeconds
and FailureThreshhold
. Ideally, I think these options should probably be grouped under their own probe
stanza for kubernetes and their variable config names match the values they are configuring since that's how they're configured in K8s. I think the easiest way to accomplish this would be defining a Probe *Probe
struct which would let users define the options like:
deploy {
use "kubernetes" {
probe_path = "/"
probe {
initial_delay_seconds = 30
timeout_seconds = 2
failure_threshhold = 3
// etc, other options could be configured here
}
}
}
An existing example of this could be the AutoScaling struct used in CloudRun platform configs:
waypoint/builtin/google/cloudrun/platform.go
Lines 591 to 592 in 71b07ea
// AutoScaling details. | |
AutoScaling *AutoScaling `hcl:"auto_scaling,block"` |
waypoint/builtin/google/cloudrun/platform.go
Lines 612 to 617 in 71b07ea
// AutoScaling defines the parameters which the Cloud Run instance can AutoScale. | |
// Currently only the maximum bound is supported | |
type AutoScaling struct { | |
//Min int `hcl:"min,attr"` // not yet supported by cloud run | |
Max int `hcl:"max,attr" validate:"gte=0"` | |
} |
Then if those options aren't defined in the config struct, we can use the defaults we already have been using when Waypoint goes to set them up here (and else where later inside this file):
waypoint/builtin/k8s/platform.go
Lines 252 to 282 in 71b07ea
deployment.Spec.Template.Spec = corev1.PodSpec{ | |
Containers: []corev1.Container{ | |
{ | |
Name: result.Name, | |
Image: img.Name(), | |
ImagePullPolicy: pullPolicy, | |
Ports: containerPorts, | |
LivenessProbe: &corev1.Probe{ | |
Handler: corev1.Handler{ | |
TCPSocket: &corev1.TCPSocketAction{ | |
Port: intstr.FromInt(defaultPort), | |
}, | |
}, | |
InitialDelaySeconds: 5, | |
TimeoutSeconds: 5, | |
FailureThreshold: 5, | |
}, | |
ReadinessProbe: &corev1.Probe{ | |
Handler: corev1.Handler{ | |
TCPSocket: &corev1.TCPSocketAction{ | |
Port: intstr.FromInt(defaultPort), | |
}, | |
}, | |
InitialDelaySeconds: 5, | |
TimeoutSeconds: 5, | |
}, | |
Env: env, | |
Resources: resourceRequirements, | |
}, | |
}, | |
} |
I'm happy to take this PR over to support the rest of the probe options and use your commit as a starting point, but if you want and can spare the time this week let us know.
We want to get this in soon, so if you don't have the time no worries, I'm more than happy to finish it out later this week 😄 Thanks!
@briancain Added the other probe fields - I'll leave the rest to you if you want to change any of the docs or comments. |
Awesome @frostbyte73 ! Looks great, thanks again for taking the time! ❤️ I'll finish this out this week. Appreciate it! |
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.
Awesome! This looks great to me now. I went ahead and fixed up the docs to use subfields. I think I won't add the other possible Probe options until we need them, since we're currently not using them at all when configuring k8s. So this looks good to go! Thanks again for the pull request 😄
If you have a project that takes more than 30 seconds to initialize, currently the only way to deploy it is by modifying the liveness initial delay seconds through kubectl. This adds the option to the kube hcl