[WIP] KEP: MultiKueue external custom Job support #2458

mszadkow · 2024-06-20T13:58:10Z

What type of PR is this?

/kind documentation

What this PR does / why we need it:

New KEP that explains the way to introduce MultiKueue external custom Job support.

Which issue(s) this PR fixes:

Fixes #2349

Special notes for your reviewer:

Does this PR introduce a user-facing change?

NONE

k8s-ci-robot · 2024-06-20T13:58:15Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: mszadkow
Once this PR has been reviewed and has the lgtm label, please assign alculquicondor for approval. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

k8s-ci-robot · 2024-06-20T13:58:19Z

Hi @mszadkow. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

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.

netlify · 2024-06-20T13:58:58Z

✅ Deploy Preview for kubernetes-sigs-kueue ready!

Name	Link
🔨 Latest commit	`ec73442`
🔍 Latest deploy log	https://app.netlify.com/sites/kubernetes-sigs-kueue/deploys/668d159967d169000886f87b
😎 Deploy Preview	https://deploy-preview-2458--kubernetes-sigs-kueue.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

keps/2349-multikueue-external-custom-job-support/README.md

trasc · 2024-06-25T10:31:56Z

/cc @alculquicondor @tenzen-y

alculquicondor · 2024-06-26T18:24:19Z

/ok-to-test

alculquicondor · 2024-06-26T18:25:39Z

@dgrove-oss is this something that interests you and do you have any thoughts on the proposal?

alculquicondor · 2024-06-26T18:26:16Z

keps/2349-multikueue-external-custom-job-support/README.md

+
+## Motivation
+
+Many Kueue adaptors have company specific CustomResources.


Suggested change

Many Kueue adaptors have company specific CustomResources.

Many Kueue adopters have company specific CustomResources.

?

alculquicondor · 2024-06-26T18:36:56Z

keps/2349-multikueue-external-custom-job-support/README.md

+Provide a way to integrate external controllers to manage Jobs in the Kueue.
+MulikueueCluster API needs to be modified to ensure that one multikueue cluster is only managed by one admission check controller.
+
+1. Add ControllerName to the `MultiKueueClusterSpec`


This is a non-starting point for this proposal.

Asking users to create one MultiKueueCluster object for each integration is too much burden.

Sure, they just need one for all of their CRDs, but what if they need to support a native jobs + in-house jobs + third-party jobs that offer MK support. Now they have at least 3 configurations for the same thing.

How does a MultiKueueConfig that supports all of these jobs look like?

IIUC, your intention with a separate object is to have conditions separate for each external admission check. Then let's maybe add an object just for that, that only one controller processes each time, separate from MultiKueueCluster.

One alternative to this, described later, is to have a custom condition type for the MultiKueueCluster that reflects the connection stale for each ControllerName using it.

Asking users to create one MultiKueueCluster object for each integration is too much burden.

It should not be for each integration, is for each configured controller , the controller can support any number of adapters.

How does a MultiKueueConfig that supports all of these jobs look like?

Nothing changes for MultiKueueConfig, they are just used by an AdmissionCheck (which already has a ControllerName)

One alternative to this, described later, is to have a custom condition type for the MultiKueueCluster that reflects the connection stale for each ControllerName using it.

This sounds much better.

alculquicondor · 2024-06-26T18:39:17Z

keps/2349-multikueue-external-custom-job-support/README.md

+	}
+}
+```
+


Please expand on:

which subcomponents of MK each external controller is implementing (even if provided by a framework)

how does a configuration for multiple of these controllers look like from the ClusterQueue point of view.

dgrove-oss

The integration test scenario seemed reasonable to me.

Providing a mechanism to register an external controller and have it coordinate with the builtin components about which GVKs each is responsible seems right.

There was not sufficient detail for me as a MultiKueue outsider to understand how the pieces fit together and what I'd need to do to add an external custom Job. Is this KEP also supposed to cover external Job adapters too?

dgrove-oss · 2024-06-26T18:29:23Z

keps/2349-multikueue-external-custom-job-support/README.md

+
+Internally the MultiKueue uses a set of controller-runtime client, one for each MultiKueueCluster, the clients are used both by the MultiKueue internals and passed to the job adapters for the custom jobs synchronization.
+
+The clients cannot be share with an external controller and assuming the external controller can get the same connectivity to the worker clusters might be wrong, especially when we are taking about FS mounted kubeconfigs.


Suggested change

The clients cannot be share with an external controller and assuming the external controller can get the same connectivity to the worker clusters might be wrong, especially when we are taking about FS mounted kubeconfigs.

The clients cannot be shared with an external controller and assuming the external controller can get the same connectivity to the worker clusters might be wrong, especially when we are taking about FS mounted kubeconfigs.

alculquicondor · 2024-06-26T18:51:24Z

Is this KEP also supposed to cover external Job adapters too?

Yes, it's supposed to enable the easy creation of them. I agree that the proposal needs more detail.

tenzen-y · 2024-06-28T13:21:40Z

/cc @alculquicondor @tenzen-y

ACK
Let me check this KEP

alculquicondor · 2024-06-28T14:43:39Z

/retitle [WIP] KEP: MultiKueue external custom Job support

k8s-ci-robot · 2024-07-09T10:55:40Z

@mszadkow: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
pull-kueue-verify-main	`ec73442`	link	true	`/test pull-kueue-verify-main`

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

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. I understand the commands that are listed here.

k8s-ci-robot added release-note-none Denotes a PR that doesn't merit a release note. do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. kind/documentation Categorizes issue or PR as related to documentation. labels Jun 20, 2024

k8s-ci-robot requested review from kerthcet and trasc June 20, 2024 13:58

k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Jun 20, 2024

k8s-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Jun 20, 2024

mimowo mentioned this pull request Jun 20, 2024

Provide a way to implement a MultiKueue support for the custom Jobs #2349

Open

5 tasks

mszadkow force-pushed the doc/add-custom-job-support branch from a6227d9 to 10fc776 Compare June 21, 2024 13:43

trasc reviewed Jun 25, 2024

View reviewed changes

mszadkow marked this pull request as ready for review June 25, 2024 10:10

k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jun 25, 2024

k8s-ci-robot requested review from denkensk and mimowo June 25, 2024 10:10

k8s-ci-robot requested review from alculquicondor and tenzen-y June 25, 2024 10:31

k8s-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Jun 26, 2024

alculquicondor reviewed Jun 26, 2024

View reviewed changes

dgrove-oss reviewed Jun 26, 2024

View reviewed changes

k8s-ci-robot changed the title ~~KEP: MultiKueue external custom Job support.~~ [WIP] KEP: MultiKueue external custom Job support Jun 28, 2024

k8s-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jun 28, 2024

mszadkow added 4 commits July 8, 2024 14:55

KEP: MultiKueue external custom Job support.

d9701a0

review updates

bb01244

remove comments

f1f1716

update after review

ec73442

mszadkow force-pushed the doc/add-custom-job-support branch from 72d5e1e to ec73442 Compare July 9, 2024 10:48

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[WIP] KEP: MultiKueue external custom Job support #2458

[WIP] KEP: MultiKueue external custom Job support #2458

mszadkow commented Jun 20, 2024

k8s-ci-robot commented Jun 20, 2024

k8s-ci-robot commented Jun 20, 2024

netlify bot commented Jun 20, 2024 •

edited

Loading

trasc commented Jun 25, 2024

alculquicondor commented Jun 26, 2024

alculquicondor commented Jun 26, 2024

alculquicondor Jun 26, 2024

alculquicondor Jun 26, 2024

trasc Jun 28, 2024 •

edited

Loading

alculquicondor Jun 28, 2024

alculquicondor Jun 26, 2024

dgrove-oss left a comment

dgrove-oss Jun 26, 2024

alculquicondor commented Jun 26, 2024

tenzen-y commented Jun 28, 2024

alculquicondor commented Jun 28, 2024

k8s-ci-robot commented Jul 9, 2024


		## Motivation

		Many Kueue adaptors have company specific CustomResources.

	Many Kueue adaptors have company specific CustomResources.
	Many Kueue adopters have company specific CustomResources.


		Internally the MultiKueue uses a set of controller-runtime client, one for each MultiKueueCluster, the clients are used both by the MultiKueue internals and passed to the job adapters for the custom jobs synchronization.

		The clients cannot be share with an external controller and assuming the external controller can get the same connectivity to the worker clusters might be wrong, especially when we are taking about FS mounted kubeconfigs.

	The clients cannot be share with an external controller and assuming the external controller can get the same connectivity to the worker clusters might be wrong, especially when we are taking about FS mounted kubeconfigs.
	The clients cannot be shared with an external controller and assuming the external controller can get the same connectivity to the worker clusters might be wrong, especially when we are taking about FS mounted kubeconfigs.

[WIP] KEP: MultiKueue external custom Job support #2458

Are you sure you want to change the base?

[WIP] KEP: MultiKueue external custom Job support #2458

Conversation

mszadkow commented Jun 20, 2024

What type of PR is this?

What this PR does / why we need it:

Which issue(s) this PR fixes:

Special notes for your reviewer:

Does this PR introduce a user-facing change?

k8s-ci-robot commented Jun 20, 2024

k8s-ci-robot commented Jun 20, 2024

netlify bot commented Jun 20, 2024 • edited Loading

✅ Deploy Preview for kubernetes-sigs-kueue ready!

trasc commented Jun 25, 2024

alculquicondor commented Jun 26, 2024

alculquicondor commented Jun 26, 2024

alculquicondor Jun 26, 2024

Choose a reason for hiding this comment

alculquicondor Jun 26, 2024

Choose a reason for hiding this comment

trasc Jun 28, 2024 • edited Loading

Choose a reason for hiding this comment

alculquicondor Jun 28, 2024

Choose a reason for hiding this comment

alculquicondor Jun 26, 2024

Choose a reason for hiding this comment

dgrove-oss left a comment

Choose a reason for hiding this comment

dgrove-oss Jun 26, 2024

Choose a reason for hiding this comment

alculquicondor commented Jun 26, 2024

tenzen-y commented Jun 28, 2024

alculquicondor commented Jun 28, 2024

k8s-ci-robot commented Jul 9, 2024

netlify bot commented Jun 20, 2024 •

edited

Loading

trasc Jun 28, 2024 •

edited

Loading