Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Make sure node has enough resources available for scheduling injector pod #2144

Closed
corang opened this issue Nov 14, 2023 · 4 comments · Fixed by #2220
Closed

Make sure node has enough resources available for scheduling injector pod #2144

corang opened this issue Nov 14, 2023 · 4 comments · Fixed by #2220
Labels
enhancement ✨ New feature or request good first issue 🥇 Good for newcomers

Comments

@corang
Copy link
Contributor

corang commented Nov 14, 2023

Is your feature request related to a problem? Please describe.

When zarf attempts to run the injector since it's tied to a specific node it should check if the node has sufficient resources available, it does not and can end up in a stuck state where the pod is never scheduled if the node doesn't have enough resources available.

Describe the solution you'd like

  • Given a cluster with workloads
  • When zarf is picking a node for the injector pod
  • Then zarf checks the resource availability on nodes before picking one
@corang corang added the enhancement ✨ New feature or request label Nov 14, 2023
@Racer159 Racer159 added the good first issue 🥇 Good for newcomers label Nov 14, 2023
@Racer159
Copy link
Contributor

Racer159 commented Nov 14, 2023 •
edited
Loading

The relevant code for this is in this function:

https://github.com/defenseunicorns/zarf/blob/8763b6b2bf47ded67fb33130161b46c85086ce32/src/pkg/k8s/images.go#L49

And a check that allocatable CPU and Memory for the node (nodeDetails.Status.Allocatable) is higher that the requests for the Pod (.5 / 64Mi respectively) would solve this.

https://github.com/defenseunicorns/zarf/blob/8763b6b2bf47ded67fb33130161b46c85086ce32/src/internal/cluster/injector.go#L357

@chrishorton
Copy link
Contributor

@Racer159 I added some code to images.go to fix this issue. Semantically - should the Allocatable amount be exclusively higher or greater than/equal to? I'm leaning towards higher for a fudge factor but would appreciate your thoughts.

@chrishorton chrishorton mentioned this issue Jan 9, 2024
Merged
5 tasks
@chrishorton
Copy link
Contributor

Opened #2220 for you to check out.

@Racer159
Copy link
Contributor

Racer159 commented Jan 9, 2024

Thanks for the PR! Added some comments / feedback over there.

Racer159 added a commit that referenced this issue Jan 10, 2024
@chrishorton @Racer159
feat: check for available node resources before building injector pod (…
98a19b4 
…#2220)

## Description

Addresses #2144 to check
if node has enough resources before the injector runs

## Related Issue

Fixes #2144 

## Type of change

- [x] Bug fix (non-breaking change which fixes an issue)
- [ ] New feature (non-breaking change which adds functionality)
- [ ] Other (security config, docs update, etc)

## Checklist before merging

- [x] Test, docs, adr added or updated as needed
- [x] [Contributor Guide
Steps](https://github.com/defenseunicorns/zarf/blob/main/CONTRIBUTING.md#developer-workflow)
followed

---------

Co-authored-by: Wayne Starr <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
enhancement ✨ New feature or request good first issue 🥇 Good for newcomers
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

feat: check for available node resources before building injector pod chrishorton/zarf
3 participants
@corang @Racer159 @chrishorton