-
Notifications
You must be signed in to change notification settings - Fork 173
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 scheduling logic to zarf injector #1731
feat: add scheduling logic to zarf injector #1731
Conversation
✅ Deploy Preview for zarf-docs canceled.
|
108debd
to
6dc6562
Compare
5c6ae3e
to
eb8de6d
Compare
eb8de6d
to
2935820
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.
I can't request it as a suggestion in this PR but one tiny thing after testing locally: Can you make the spinner for gathering images notify the user of the 5 minute timeout since they may hit that if all nodes have taints (similar to how waiting for cluster connection works)?
https://github.com/defenseunicorns/zarf/blob/main/src/internal/cluster/injector.go#L44
// Get all the images from the cluster
timeout := 5 * time.Minute
spinner.Updatef("Getting the list of existing cluster images (%s timeout)", timeout.String())")
if images, err = c.Kube.GetAllImages(timeout); err != nil {
https://github.com/defenseunicorns/zarf/blob/main/src/pkg/k8s/images.go#L22
func (k *K8s) GetAllImages(timeoutDuration time.Duration) (ImageNodeMap, error) {
timeout := time.After(timeoutDuration)
2935820
to
67397b1
Compare
67397b1
to
2e5b5c9
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
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
Description
Checks to ensure that a node does not have a
NoSchedule
taint before choosing that node/image for the Zarf injector podRelated Issue
Fixes #1730
Fixes #905
Type of change
Checklist before merging
Gave this a WAG. It seems to work locally, but am unsure if this is the direction you guys want to go. Let me know!