-
Notifications
You must be signed in to change notification settings - Fork 251
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
Implement ResourceFlavor API #133
Conversation
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: ahg-g The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
/hold Just so it doesn't get merged by mistake |
/retest |
Also update the sample yamls, please https://github.com/kubernetes-sigs/kueue/tree/main/config/samples |
done. |
can you rebase to see if that fixes verify? |
go.mod
Outdated
@@ -24,30 +24,50 @@ require ( | |||
github.com/Azure/go-autorest/autorest/date v0.3.0 // indirect | |||
github.com/Azure/go-autorest/logger v0.2.1 // indirect | |||
github.com/Azure/go-autorest/tracing v0.6.0 // indirect | |||
github.com/PuerkitoBio/purell v1.1.1 // indirect |
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.
@alculquicondor @ArangoGutierrez any idea where those changes came from? I had to run o get sigs.k8s.io/kustomize/kustomize/v4
to deploy, so probably from that; is it expected?
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.
you shouldn't use go get
, but go install
instead. The new makefile does that.
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.
It is not working for me, make deploy still fails with
no required module provides package sigs.k8s.io/kustomize/kustomize/v4; to add it:
go get sigs.k8s.io/kustomize/kustomize/v4
make: *** [Makefile:183: kustomize] Error 1
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.
did you figure it out or did you just revert the file?
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.
Just reverted the file
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
go.mod
Outdated
@@ -24,30 +24,50 @@ require ( | |||
github.com/Azure/go-autorest/autorest/date v0.3.0 // indirect | |||
github.com/Azure/go-autorest/logger v0.2.1 // indirect | |||
github.com/Azure/go-autorest/tracing v0.6.0 // indirect | |||
github.com/PuerkitoBio/purell v1.1.1 // indirect |
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.
Can you update your go to 1.18?
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.
updated but I also reverted this file
/lgtm |
/hold cancel |
What type of PR is this?
/kind feature
What this PR does / why we need it:
Implements the ResourceFlavor API. ClusterQueues now don't embed the labels/taints of a flavor, they are looked-up from a matching ResourceFlavor resource.
The scheduler will skip flavors with no matching ResourceFlavor. In a followup work we need to introduce a new "OutOfCommision/Frozen" state for ClusterQueue when this happens. ClusterQueues in this state should be taken out of the cohort and no jobs can schedule via them until the referenced flavors are defined. This will be tracked in #134.
Which issue(s) this PR fixes:
Fixes #59
Special notes for your reviewer: