-
-
Notifications
You must be signed in to change notification settings - Fork 331
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
Add common wait conditions for Deployments, LoadBalancer Services, and Ingress #1710
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.
These all look great. Thank you very much for these.
Small suggestion on how to unit test them would be appreciated, but otherwise vhappy to merge this.
if spec.type_ != Some("LoadBalancer".to_string()) { | ||
return true; | ||
} |
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 guess this is the only questionable thing, and it's not really a fault of yours, but more about us conflating bools and options. i'll think a bit about #1498 after this pr.
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.
Yeah, I don't think there's any better way to handle this at the moment.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1710 +/- ##
=======================================
+ Coverage 75.8% 76.2% +0.4%
=======================================
Files 84 84
Lines 7743 7847 +104
=======================================
+ Hits 5866 5972 +106
+ Misses 1877 1875 -2
🚀 New features to boost your workflow:
|
These are common wait conditions for these resources, and are useful for waiting for exposed services to become available. Signed-off-by: Robert Detjens <[email protected]>
Signed-off-by: Robert Detjens <[email protected]>
These tests run the condition function against included yaml snippets of objects returned by the K8S API to check against known-state statuses. Signed-off-by: Robert Detjens <[email protected]>
cf2d6d0
to
31d6907
Compare
Motivation
I've been using this library to create a custom tool to deploy applications, and have been using these conditions via
await_condition
to wait for those applications to become ready or report failures back to the users.Since these are generally useful for waiting for these types of services to become available, I'm hoping to contribute these back upstream so that other client-esque applications have these ready to use.
Solution
This adds a few new wait conditions to
kube-runtime::wait::conditions
forDeployment
s,LoadBalancer
-typeService
s, andIngress
s to complete or become ready.The
Deployment
check (is_deployment_completed()
) matches K8S's definition of completed deployments.LoadBalancer
andIngress
resources don't have an associated status condition for 'ready', but they are updated with the LoadBalancer details the backing cloud/etc load balancer has been fully provisioned and is ready. The newis_ingress_provisioned()
/is_service_loadbalancer_provisioned()
checks wait for that update to set.status.loadBalancer.ingress.(ip|hostname)
. Non-loadbalancer services are ignored by always returning true i.e. immediately ready.I did not add tests for these since the existing condition checks do not have any.