refactor(wait): avoid shelling out to kubectl during wait#4567
refactor(wait): avoid shelling out to kubectl during wait#4567AustinAbro321 merged 99 commits intomainfrom
Conversation
Signed-off-by: Austin Abro <austinabro321@gmail.com>
Signed-off-by: Austin Abro <austinabro321@gmail.com>
Signed-off-by: Austin Abro <austinabro321@gmail.com>
Signed-off-by: Austin Abro <austinabro321@gmail.com>
Signed-off-by: Austin Abro <austinabro321@gmail.com>
Signed-off-by: Austin Abro <austinabro321@gmail.com>
Signed-off-by: Austin Abro <austinabro321@gmail.com>
Signed-off-by: Austin Abro <austinabro321@gmail.com>
Signed-off-by: Austin Abro <austinabro321@gmail.com>
Signed-off-by: Austin Abro <austinabro321@gmail.com>
Signed-off-by: Austin Abro <austinabro321@gmail.com>
Signed-off-by: Austin Abro <austinabro321@gmail.com>
Signed-off-by: Austin Abro <austinabro321@gmail.com>
Signed-off-by: Austin Abro <austinabro321@gmail.com>
Signed-off-by: Austin Abro <austinabro321@gmail.com>
Signed-off-by: Austin Abro <austinabro321@gmail.com>
Signed-off-by: Austin Abro <austinabro321@gmail.com>
Signed-off-by: Austin Abro <austinabro321@gmail.com>
Signed-off-by: Austin Abro <austinabro321@gmail.com>
Signed-off-by: Austin Abro <austinabro321@gmail.com>
Codecov Report❌ Patch coverage is
... and 1 file with indirect coverage changes 🚀 New features to boost your workflow:
|
Signed-off-by: Austin Abro <austinabro321@gmail.com>
Signed-off-by: Austin Abro <austinabro321@gmail.com>
Signed-off-by: Austin Abro <austinabro321@gmail.com>
Signed-off-by: Austin Abro <austinabro321@gmail.com>
Signed-off-by: Austin Abro <austinabro321@gmail.com>
Signed-off-by: Austin Abro <austinabro321@gmail.com>
Signed-off-by: Austin Abro <austinabro321@gmail.com>
Signed-off-by: Austin Abro <austinabro321@gmail.com>
Signed-off-by: Austin Abro <austinabro321@gmail.com>
Signed-off-by: Austin Abro <austinabro321@gmail.com>
Signed-off-by: Austin Abro <austinabro321@gmail.com>
Signed-off-by: Austin Abro <austinabro321@gmail.com>
Signed-off-by: Austin Abro <austinabro321@gmail.com>
Signed-off-by: Austin Abro <austinabro321@gmail.com>
brandtkeller
left a comment
There was a problem hiding this comment.
Few areas of interest but overall a serious improvement.
| waitInterval := time.Second | ||
| l.Info("waiting for any resource of kind to exist", "kind", resource.Resource, "namespace", namespace) | ||
|
|
||
| resourceClient := dynamicClient.Resource(resource).Namespace(namespace) |
There was a problem hiding this comment.
I need to test this - I am not sure this can delineate between cluster-scoped resources and a lack of a namespace for a namespaced resource.
if someone forgets to add a -n to a namespaced resource I believe this will unintuitively return on the existence of that resource in the cluster.
I think we could check for resource being cluster scoped and then otherwise get the default from the kube context?
There was a problem hiding this comment.
Really subtle and nice catch. Yeah release zarf tools wait-for pod will only pass if there is a pod in the default namespace, whereas this branchs zarf tools wait-for pod will pass if a pod is in the cluster. Will add a test and change the behavior
There was a problem hiding this comment.
Added in logic to get the users default namespace. An added bonus here is that even when a user doesn't give a namespace, their default will show up in the log
Signed-off-by: Austin Abro <austinabro321@gmail.com>
Signed-off-by: Austin Abro <austinabro321@gmail.com>
Signed-off-by: Austin Abro <austinabro321@gmail.com>
| }) | ||
|
|
||
| t.Run("wait for non-existent resource times out", func(t *testing.T) { | ||
| t.Parallel() |
There was a problem hiding this comment.
Decided to make these run in parallel, this brings the entire test to 6 seconds from 26. I think I'm fine with the risks here, essentially you would need to make sure another test isn't creating a resource with the same name. Definitely open to objections though
Signed-off-by: Austin Abro <austinabro321@gmail.com>
Signed-off-by: Austin Abro <austinabro321@gmail.com>
brandtkeller
left a comment
There was a problem hiding this comment.
thoughtfully executed - lgtm
Description
This entirely eliminates any shelling out in the wait command. Instead, we first get the kind directly from Kubernetes to replace the kubectl get calls, then call the equivalent of
kubectl waitdirectly through the kube go dependency.Went for parity with the current behavior of
zarf tools wait-forwhich includes waiting for a cluster to be up, even if it currently doesn't exist. This may be behavior we want to re-evaluate when we introducezarf tools wait-for resource#4550This does change the logging strategy, previously wait would log each second it waits like so
Now the message will not continuously update, and instead look like, unless the
-l debugflag is used. This is more consistent with the logging strategy used elsewhere in Zarfbuild/zarf tools wait-for -n kube-system pods metrics-server-5985cbc9d7-92mwh1 ready 2026-02-11 13:19:34 INF using config file location=/home/austin/code/zarf/zarf-config.toml 2026-02-11 13:19:34 INF waiting for resource kind=pods identifier=metrics-server-5985cbc9d7-92mwh1 condition=condition=ready namespace=kube-systemChecklist before merging