Add ElasticJob support #465
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: kannon92 The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
k8s-ci-robot
added
cncf-cla: yes
✅ Deploy Preview for kubernetes-sigs-jobset canceled.
|
|
/hold tests are failing and we should discuss validation for ReplicatedJobs in more detail. |
k8s-ci-robot
added
the
do-not-merge/hold
|
@kannon92: The following test failed, say
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/test-infra repository. I understand the commands that are listed here. |
| @@ -178,8 +178,7 @@ func (js *JobSet) ValidateUpdate(old runtime.Object) (admission.Warnings, error) | |||
| } | |||
| } | |||
| // Note that SucccessPolicy and failurePolicy are made immutable via CEL. | |||
| errs := apivalidation.ValidateImmutableField(mungedSpec.ReplicatedJobs, oldJS.Spec.ReplicatedJobs, field.NewPath("spec").Child("replicatedJobs")) | |||
There was a problem hiding this comment.
@ahg-g this is probably not what we want.
If we drop all validation of a replicated job, updates could add/remove entire replicated jobs.
I think we wouldn't want that.
| }, | ||
| }, | ||
| { | ||
| name: "dropping a replicated job is valid", |
There was a problem hiding this comment.
@ahg-g @danielvegamyhre added this here so we can discuss what kind of validation do we want on updates for replicated jobs.
I think this is probably not what we want.. OTOH if a jobset is suspended maybe someone can drop/add replicated jobs..
| @@ -1125,6 +1125,25 @@ var _ = ginkgo.Describe("JobSet controller", func() { | |||
| }, | |||
| }, | |||
| }), | |||
| ginkgo.Entry("elastic jobset; scale up replicated jobs", &testCase{ | |||
There was a problem hiding this comment.
note: this isn't working yet. Once I figure out validation, I'll come back to this.
k8s-ci-robot
added
the
needs-rebase
|
PR needs rebase. 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. |
|
/close |
|
@kannon92: Closed this PR. 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/test-infra repository. |
Fixes #463
Elastic JobSet provides a way to downscale/upscale replicated jobs. The goal of this PR is to support ElasticJobSet sets.
In order to support this, we need to relax our validation of ReplicatedJobs.