Add support for single pods #1072

achernevskii · 2023-08-18T20:05:22Z

What type of PR is this?

/kind feature

What this PR does / why we need it:

Add support for pods (k8s 1.26+)
Add PodIntegrationOptions configuration field containing
namespace and pod label selectors.
Create a new interface JobWithFinalize. Jobs implementing
it supports custom finalization logic.
Add namespace/pod label filtering for Default pod webhook.

Which issue(s) this PR fixes:

Related issue: #976

Special notes for your reviewer:

Does this PR introduce a user-facing change?

Add support for single plain Pods.

k8s-ci-robot · 2023-08-18T20:05:31Z

Hi @achernevskii. 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/test-infra repository.

netlify · 2023-08-18T20:05:37Z

✅ Deploy Preview for kubernetes-sigs-kueue canceled.

Name	Link
🔨 Latest commit	`aac3eb4`
🔍 Latest deploy log	https://app.netlify.com/sites/kubernetes-sigs-kueue/deploys/651368f833187a000811023c

trasc · 2023-08-19T07:31:11Z

/ok-to-test

apis/config/v1beta1/configuration_types.go

apis/config/v1beta1/configuration_types_test.go

apis/config/v1beta1/defaults.go

pkg/controller/jobs/pod/pod_webhook.go

pkg/controller/jobs/pod/pod_webhook_test.go

pkg/util/testing/wrappers.go

site/static/examples/pods-kueue/high-prio-pod.yaml

apis/config/v1beta1/validation.go

pkg/controller/jobs/pod/pod_controller.go

pkg/controller/jobs/pod/pod_webhook.go

apis/config/v1beta1/validation_test.go

main.go

pkg/config/validation.go

pkg/controller/jobs/pod/pod_controller.go

pkg/controller/jobs/pod/pod_controller_test.go

pkg/config/validation.go

test/integration/controller/pod/suite_test.go

test/integration/controller/pod/pod_controller_test.go

alculquicondor · 2023-09-07T16:54:04Z

please rebase instead of merge.

It might be ok to squash at this point

alculquicondor

/approve
/hold for nits

I will leave LGTM to @tenzen-y

pkg/controller/jobframework/reconciler.go

pkg/controller/jobs/kubeflow/jobs/pytorchjob/pytorchjob_controller.go

pkg/controller/jobs/kubeflow/jobs/tfjob/tfjob_controller.go

pkg/controller/jobs/kubeflow/jobs/xgboostjob/xgboostjob_controller.go

pkg/controller/jobs/pod/pod_controller_test.go

test/integration/controller/core/suite_test.go

tenzen-y

Also, can you rebase this PR and add IsManagingObjectsOwner to the PaddleJob?

kueue/pkg/controller/jobs/kubeflow/jobs/paddlejob/paddlejob_controller.go

Lines 38 to 44 in f9273be

    
           utilruntime.Must(jobframework.RegisterIntegration(FrameworkName, jobframework.IntegrationCallbacks{ 
        
           	SetupIndexes:  SetupIndexes, 
        
           	NewReconciler: NewReconciler, 
        
           	SetupWebhook:  SetupPaddleJobWebhook, 
        
           	JobType:       &kftraining.PaddleJob{}, 
        
           	AddToScheme:   kftraining.AddToScheme, 
        
           }))

pkg/controller/jobs/pod/pod_controller.go

pkg/controller/jobs/pod/pod_webhook.go

pkg/controller/jobs/pod/pod_webhook_test.go

test/integration/controller/jobs/pod/pod_controller_test.go

pkg/controller/jobs/kubeflow/jobs/tfjob/tfjob_controller.go

tenzen-y · 2023-09-22T22:36:18Z

test/integration/controller/jobs/pod/pod_controller_test.go

+			})
+		})
+	})
+})


Add test cases when starting manager with the scheduler like other integrations:

kueue/test/integration/controller/jobs/jobset/jobset_controller_test.go

Line 586 in 4e65928

var _ = ginkgo.Describe("JobSet controller interacting with scheduler", ginkgo.Ordered, ginkgo.ContinueOnFailure, func() {

tenzen-y · 2023-09-22T22:46:45Z

/release-note Add support for single pods

tenzen-y · 2023-09-22T22:47:49Z

/release-note Add support for single pods

@achernevskii Can you add a release note?

* Add namespace/pod label filtering for Default pod webhook. Add PodIntegrationOptions configuration field containing namespace and pod label selectors. * Simplify RunWithPodSetsInfo method for the pod controller. * Create a new interface JobWithFinalize. Jobs implementing it supports custom finalization logic. * Add k8s version check. If the pod integration is enabled on k8s server versions < 1.27, Kueue pod will stop with an error message. * Add JobWithSkip interface. Jobs that implement this interface can introduce custom reconciliation skip logic. * Add IsPodOwnerManagedByQueue function. Defaulting webhook will skip a pod if it's owner is managed by Kueue. Reconciler will skip such a pod even if 'managed' label is set. * Add integration tests for the pod controller and webhook.

* Add tests for pod controller interacting with scheduler * Change IsManagingObjectsOwner functions for Kubeflow jobs. * Update webhook paths for integration tests. * Add parent check for kubeflow PaddleJob resource * Update helm chart. * Replace patchesStrategicMerge with patches in webhook kustomization.yaml * Update unit/integration tests. * Change pod webhook failure policy from Ignore to fail, add webhook namespace selector patch. * Merge tests in validation_test.go into a single test for exported ValidateConfiguration function. * Update JobWithSkip interface. * Add missing licence comments. * Rewrite some of the integration tests messages and value validations. * Update unit tests.

tenzen-y

@achernevskii This looks great! Thank you!

/lgtm
/approve

k8s-ci-robot · 2023-09-27T07:05:40Z

LGTM label has been added.

Git tree hash: 305aa7f7f714fdb39db5225c75e94406e27ca817

k8s-ci-robot · 2023-09-27T07:05:42Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: achernevskii, alculquicondor, tenzen-y

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:

~~OWNERS~~ [alculquicondor,tenzen-y]

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

tenzen-y · 2023-09-27T07:05:49Z

/hold cancel

k8s-ci-robot requested review from ahg-g and denkensk August 18, 2023 20:05

k8s-ci-robot added needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Aug 18, 2023

k8s-ci-robot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Aug 18, 2023

k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 18, 2023

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 Aug 19, 2023

trasc mentioned this pull request Aug 19, 2023

WIP: [jobs/pod] Add support for pods #985

Closed

alculquicondor reviewed Aug 21, 2023

View reviewed changes

alculquicondor reviewed Aug 24, 2023

View reviewed changes

alculquicondor reviewed Aug 28, 2023

View reviewed changes

pkg/config/validation.go Outdated Show resolved Hide resolved

pkg/config/validation.go Show resolved Hide resolved

alculquicondor mentioned this pull request Aug 31, 2023

An alternate implementation of pod support with taints/tolerations #1092

Closed

alculquicondor reviewed Sep 1, 2023

View reviewed changes

k8s-ci-robot added needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. and removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Sep 2, 2023

achernevskii force-pushed the pod_support branch from b622135 to 31fd8e1 Compare September 8, 2023 01:31

achernevskii changed the title ~~WIP: Add support for pods~~ Add support for single pods Sep 8, 2023

achernevskii marked this pull request as ready for review September 8, 2023 01:36

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 Sep 8, 2023

k8s-ci-robot requested a review from alculquicondor September 8, 2023 01:36

achernevskii force-pushed the pod_support branch from bbd171f to 15ca7b3 Compare September 20, 2023 19:59

k8s-ci-robot added needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. and removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Sep 20, 2023

alculquicondor reviewed Sep 21, 2023

View reviewed changes

k8s-ci-robot added do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Sep 21, 2023

tenzen-y reviewed Sep 22, 2023

View reviewed changes

k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Sep 22, 2023

trasc and others added 3 commits September 25, 2023 22:28

[jobs/pod] Add support for pods

f8481fc

achernevskii force-pushed the pod_support branch from 4e65928 to 2d5ba77 Compare September 26, 2023 03:53

Add integration test, change stop pod condition

aac3eb4

achernevskii force-pushed the pod_support branch from 407abd4 to aac3eb4 Compare September 26, 2023 23:27

tenzen-y reviewed Sep 27, 2023

View reviewed changes

k8s-ci-robot assigned tenzen-y Sep 27, 2023

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Sep 27, 2023

k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Sep 27, 2023

k8s-ci-robot merged commit a68805f into kubernetes-sigs:main Sep 27, 2023
14 checks passed

k8s-ci-robot added this to the v0.5 milestone Sep 27, 2023

achernevskii mentioned this pull request Oct 26, 2023

REQUEST: New membership for achernevskii kubernetes/org#4547

Closed

9 tasks

trasc deleted the pod_support branch March 12, 2024 08:59

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for single pods #1072

Add support for single pods #1072

achernevskii commented Aug 18, 2023 •

edited by alculquicondor

k8s-ci-robot commented Aug 18, 2023

netlify bot commented Aug 18, 2023 •

edited

trasc commented Aug 19, 2023

alculquicondor commented Sep 7, 2023

alculquicondor left a comment

tenzen-y left a comment

tenzen-y Sep 22, 2023

tenzen-y commented Sep 22, 2023

tenzen-y commented Sep 22, 2023

tenzen-y left a comment

k8s-ci-robot commented Sep 27, 2023

k8s-ci-robot commented Sep 27, 2023

tenzen-y commented Sep 27, 2023

	utilruntime.Must(jobframework.RegisterIntegration(FrameworkName, jobframework.IntegrationCallbacks{
	SetupIndexes: SetupIndexes,
	NewReconciler: NewReconciler,
	SetupWebhook: SetupPaddleJobWebhook,
	JobType: &kftraining.PaddleJob{},
	AddToScheme: kftraining.AddToScheme,
	}))

Add support for single pods #1072

Add support for single pods #1072

Conversation

achernevskii commented Aug 18, 2023 • edited by alculquicondor

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 Aug 18, 2023

netlify bot commented Aug 18, 2023 • edited

✅ Deploy Preview for kubernetes-sigs-kueue canceled.

trasc commented Aug 19, 2023

alculquicondor commented Sep 7, 2023

alculquicondor left a comment

Choose a reason for hiding this comment

tenzen-y left a comment

Choose a reason for hiding this comment

tenzen-y Sep 22, 2023

Choose a reason for hiding this comment

tenzen-y commented Sep 22, 2023

tenzen-y commented Sep 22, 2023

tenzen-y left a comment

Choose a reason for hiding this comment

k8s-ci-robot commented Sep 27, 2023

k8s-ci-robot commented Sep 27, 2023

tenzen-y commented Sep 27, 2023

achernevskii commented Aug 18, 2023 •

edited by alculquicondor

netlify bot commented Aug 18, 2023 •

edited