-
Notifications
You must be signed in to change notification settings - Fork 262
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 Cohort Webhook #2982
Implement Cohort Webhook #2982
Conversation
✅ Deploy Preview for kubernetes-sigs-kueue canceled.
|
/assign @macsko |
Should we have that release note? It was already possible to create cohorts since #2693 |
Is it an option for me to write a large release note which covers all of the HierarchicalCohort changes? Then, we can link all of the relevant pull requests to it in the 0.9 release |
I actually prefer that. Or somehow preface all the changes with one common string, so that the release notes script puts them next to each other. |
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.
/approve
pkg/webhooks/clusterqueue_webhook.go
Outdated
var allErrs field.ErrorList | ||
allErrs = append(allErrs, ValidateClusterQueue(newObj)...) | ||
return allErrs |
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.
var allErrs field.ErrorList | |
allErrs = append(allErrs, ValidateClusterQueue(newObj)...) | |
return allErrs | |
return ValidateClusterQueue(newObj) |
Or just remove the function. I think we used to have mutability checks before, but now everything is editable.
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.
Changed to the suggestion, rather than removing. I think we still want validation, to make sure the values make sense upon update?
3b0b662
to
38396d5
Compare
/approve |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: alculquicondor, gabesaba 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 |
/lgtm |
LGTM label has been added. Git tree hash: 58635506a3e9a7203d7c8353edd25590a4f49821
|
/release-note-none Only attaching release note to a single PR, where we define the API #2693 |
@gabesaba: you can only set the release note label to release-note-none if the release-note block in the PR body text is empty or "none". In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
/release-note-edit
|
What type of PR is this?
/kind feature
What this PR does / why we need it:
Implements Cohort Webhook, allowing users to create Cohort API objects. Part of #79
Special notes for your reviewer:
Additional changes are needed in
cache
andqueue
before the hierarchical functionality is complete. In the current state, we have implemented first-class Cohorts: Cohorts which provide resources directly, not just from lending ClusterQueues.