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.

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

namespace-scope configuration of manageJobsWithoutQueueName #2119

Open
3 tasks
dgrove-oss opened this issue May 3, 2024 · 17 comments
Open
3 tasks

namespace-scope configuration of manageJobsWithoutQueueName #2119

dgrove-oss opened this issue May 3, 2024 · 17 comments
Labels
kind/feature Categorizes issue or PR as related to a new feature.

Comments

@dgrove-oss
Copy link
Contributor

What would you like to be added:

Kueue supports a global configuration manageJobsWithoutQueueName. We would like to be able
to set this configuration flag to be true only for specific namespaces.

Why is this needed:

We would like to be able to use Kueue to manage batch-oriented production clusters where we can
enforce that all workloads submitted in certain namespaces (ie, user namespaces) must be submitted
through a Kueue-managed queue. Setting manageJobsWithoutQueueName to true globally is not viable
without also disabling the batch/job integration because when it is enabled it interferes with system
operations that create Jobs to perform administrative tasks.

Our workaround in Kueue 0.6 is to not use manageJobsWithoutQueueName and instead use RBACs
to deny users in those namespaces the ability to directly create batchv1.Jobs (forcing them to submit
them via AppWrappers, which can be configured separately from Kueue to act as-if manageJobsWithoutQueueName
were true just for AppWrappers). This is not optimal, because the extra level of wrapping for a resource type
that Kueue could support natively adds friction to the user experience.

Completion requirements:

It should be possible to provide a list of namespaces where manageJobsWithoutQueueName is true as part
of the Kueue configuration. Ideally it would be possible to update this list dynamically without needing to
restart the Kueue controller.

This enhancement requires the following artifacts:

  • Design doc
  • API change
  • Docs update

The artifacts should be linked in subsequent comments.

@dgrove-oss dgrove-oss added the kind/feature Categorizes issue or PR as related to a new feature. label May 3, 2024
@vladikkuzn
Copy link
Contributor

/assign

@alculquicondor
Copy link
Contributor

I'm on the fence on this one.

One of the original principles of Kueue was to be lean, which in this topic means not to be a policy enforcer. We provided some basic namespace selectors in the ClusterQueues, but it's preferable not to expand into more policies.

Have you tried implementing this using a native validating admission policy? https://kubernetes.io/docs/reference/access-authn-authz/validating-admission-policy/

@dgrove-oss
Copy link
Contributor Author

I think the high-level feedback is that in our usage of Kueue, we've come to realize that a global manageJobsWithoutQueueNames is not useful for us as currently implemented. Globally enabling suspend-by-default behavior for batch/Jobs breaks too many things (especially on OpenShift).

We can see two possible approaches to making queuing-by-default useful to us:

  1. Using a namespace selector to have finer grained-control on whether manageJobsWithoutQueueNames is true/false as proposed in this issue.
  2. Adding a jobOptions to the Kueue configuration with the same functionality as the existing podOptions to enable finer-grained control over queuing of batch/Jobs. If we had this capability, then I believe the existing global scope for manageJobsWithoutQueueName could do what we need.

If you think option 2 is a better option for Kueue's design point, I'm happy to pursue that instead.

@dgrove-oss
Copy link
Contributor Author

Have you tried implementing this using a native validating admission policy? https://kubernetes.io/docs/reference/access-authn-authz/validating-admission-policy/

Thanks for the pointer. This could be useful to us eventually, but we currently need to support Kubernetes versions back to 1.27.

@alculquicondor
Copy link
Contributor

ValidatingAdmissionPolicy is alpha since v1.27 (disabled by default), and beta since v1.28 (enabled by default).
v1.27 end-of-life is a month from now https://kubernetes.io/releases/

So it doesn't feel like it's the right time to implement this functionality in Kueue.

Any chance you can enable the alpha feature in v1.27?

@dgrove-oss
Copy link
Contributor Author

dgrove-oss commented May 24, 2024

Turing on an alpha feature isn't viable for us. But, the long term solution should be viable, so we don't have to do something in Kueue.

I guess the key question is do you ever intend to have manageJobsWithoutQueueNames be true and have the batch/jobs integration enabled in Kueue? Because that combination is broken in subtle and hard to debug ways on clusters where system operations are performed using batch Jobs.

Is it useful for us to open an issue about that combination in particular?

@mimowo
Copy link
Contributor

mimowo commented May 24, 2024

I would be leaning towards option (1.), and say that being able to exclude, by namespace selector, some administrative namespaces from manageJobsWithoutQueueName: true via Kueue API can be handy. It is similar to what we do for pod integrations, where we exclude some of the namespaces.

@alculquicondor
Copy link
Contributor

I agree that the functionality is useful. But (1) there is an alternative in 1.28+ and (2) it's hard to say when to stop implementing policies. What might be enough for your use case might not be enough for everyone.

I'm kind of ok with implementing this now, but I just do not want to set a precedent for future feature requests in this area.

@alculquicondor
Copy link
Contributor

I guess the key question is do you ever intend to have manageJobsWithoutQueueNames be true and have the batch/jobs integration enabled in Kueue? Because that combination is broken in subtle and hard to debug ways on clusters where system operations are performed using batch Jobs.

It works for some basic clusters that don't have system operations using jobs. But I'm tempted to remove this functionality altogether once we offer a v1 API.

@vladikkuzn
Copy link
Contributor

/unassign

@tenzen-y
Copy link
Member

tenzen-y commented May 24, 2024

I have similar use cases. The option (1) allows us to forcefully suspend all Jobs in the arbitrary namespaces (in general, only user namespaces), right?
But, the ValidatingAdmissionPolicy doesn't bring functionality to forcefully suspend Jobs; that just rejects creation.

I guess that the ValidatingAdmissionPolicy could not replace with option (1), maybe the native Kubernetes need to provide MutatingAdmissionPolicy something.

So, I think that providing option (1) is still valid.
@alculquicondor WDYT?

@alculquicondor
Copy link
Contributor

But, the ValidatingAdmissionPolicy doesn't bring functionality to forcefully suspend Jobs; that just rejects creation.

You could reject the creation of Jobs that should have a queue name but don't.

And then you would use manageJobsWithoutQueueName: false.

@tenzen-y
Copy link
Member

But, the ValidatingAdmissionPolicy doesn't bring functionality to forcefully suspend Jobs; that just rejects creation.

You could reject the creation of Jobs that should have a queue name but don't.

And then you would use manageJobsWithoutQueueName: false.

Uhm, it sounds reasonable. Even if we forcefully suspend Jobs without queueName, Job has never been inserted into queue since the Job doesn't have queueName. It means that the result is the same as the approach of rejecting of creation by ValidatingAdmissionPolicy.

@alculquicondor
Copy link
Contributor

And it's even better for the user, as it provides immediate feedback.

@tenzen-y
Copy link
Member

And it's even better for the user, as it provides immediate feedback.

I agree with you. Here, I'm curious about whether we can provide APIs to create ValidatingAdmissionPolicy something like blockJobsWithoutQueuName. Once the feature is enabled, the kueue-manager creates the ValidatingAdmissionPolicy to reject Jobs without queueName.

This has the advantage of easily introducing policies from the batch admins perspective, but it would bring us (kueue upstream) to increase maintenance costs.

@dgrove-oss
Copy link
Contributor Author

And it's even better for the user, as it provides immediate feedback.

I agree with you. Here, I'm curious about whether we can provide APIs to create ValidatingAdmissionPolicy something like blockJobsWithoutQueuName. Once the feature is enabled, the kueue-manager creates the ValidatingAdmissionPolicy to reject Jobs without queueName.

This has the advantage of easily introducing policies from the batch admins perspective, but it would bring us (kueue upstream) to increase maintenance costs.

My gut would be that Kueue should just document this (possibly with an example) as a way to achieve a particular kind of cluster configuration. I think it will be easier for the batch admin if the actual creation of the ValidatingAdmissionPolicy is done using "vanilla" Kubernetes concepts and operations (as opposed to needing to know how to configure Kueue to create it).

@tenzen-y
Copy link
Member

And it's even better for the user, as it provides immediate feedback.

I agree with you. Here, I'm curious about whether we can provide APIs to create ValidatingAdmissionPolicy something like blockJobsWithoutQueuName. Once the feature is enabled, the kueue-manager creates the ValidatingAdmissionPolicy to reject Jobs without queueName.
This has the advantage of easily introducing policies from the batch admins perspective, but it would bring us (kueue upstream) to increase maintenance costs.

My gut would be that Kueue should just document this (possibly with an example) as a way to achieve a particular kind of cluster configuration. I think it will be easier for the batch admin if the actual creation of the ValidatingAdmissionPolicy is done using "vanilla" Kubernetes concepts and operations (as opposed to needing to know how to configure Kueue to create it).

That makes sense, but even if we don't provide the dedicated API like blockJobsWithoutQueueName, I think that providing the ValidatingAdmissionPolicy and ValidatingAdmissionPolicyRoleBinding would be worth it.

Because batch admins can define Validations using upstream ValidatingAdmissionPolicyRoleBinding for the custom jobs.
Additionally, we (upstream Kueue) provide the validations for native supporting Jobs (batch/job, rayjob...) using ValidatingAdmissionPolicyRoleBinding.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/feature Categorizes issue or PR as related to a new feature.
Projects
None yet
Development

No branches or pull requests

5 participants