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

kubectl create job #60084

Merged
merged 2 commits into from Feb 23, 2018

Conversation

@soltysh
Contributor

soltysh commented Feb 20, 2018

What this PR does / why we need it:
This add kubectl create job command, and is a followup to #60039.

Special notes for your reviewer:

Release note:

Add kubectl create job command
@soltysh

This comment has been minimized.

Contributor

soltysh commented Feb 20, 2018

@erhudy fyi

# Test kubectl create job from cronjob
# Pre-Condition: create a cronjob
kubectl run pi --schedule="* */5 * * *" --generator=cronjob/v1beta1 "--image=$IMAGE_PERL" --restart=OnFailure -- perl -Mbignum=bpi -wle 'print bpi(20)'

This comment has been minimized.

@deads2k

deads2k Feb 21, 2018

Contributor

ick. This is the best way to make a cronjob at the moment?

This comment has been minimized.

@soltysh

soltysh Feb 21, 2018

Contributor

This or kubectl create -f ...

# Test kubectl create job from cronjob
# Pre-Condition: create a cronjob
kubectl run pi --schedule="* */5 * * *" --generator=cronjob/v1beta1 "--image=$IMAGE_PERL" --restart=OnFailure -- perl -Mbignum=bpi -wle 'print bpi(20)'

This comment has been minimized.

@deads2k

deads2k Feb 21, 2018

Contributor

different value for print bpi here so we can be sure we create this one

@@ -1279,7 +1312,7 @@ run_kubectl_run_tests() {
# Pre-Condition: no Job exists
kube::test::get_object_assert jobs "{{range.items}}{{$id_field}}:{{end}}" ''
# Command
kubectl run pi --generator=job/v1 "--image=$IMAGE_PERL" --restart=OnFailure -- perl -Mbignum=bpi -wle 'print bpi(20)' "${kube_flags[@]}"
kubectl run pi --generator=job/v1 "--image=$IMAGE_PERL" --restart=OnFailure -- perl -Mbignum=bpi -wle 'print bpi(20)'

This comment has been minimized.

@deads2k

deads2k Feb 21, 2018

Contributor

odd change. Why did kube_flags go away?

This comment has been minimized.

@soltysh

soltysh Feb 21, 2018

Contributor

They are after -- which means they will be passed to the perl command, not to the kube. I don't think they are needed, and if they are they should eventually be before --. I'll double check.

kubectl create job --from-cronjob=a-cronjob`))
)
type CreateJobOptions struct {

This comment has been minimized.

@deads2k

deads2k Feb 21, 2018

Contributor

I really liked his struct here. How about we keep this bit.

Unstructured().
NamespaceParam(namespace).DefaultNamespace().
ResourceTypeOrNameArgs(false, from).
Latest()

This comment has been minimized.

@deads2k

deads2k Feb 21, 2018

Contributor

flatten

for k, v := range cronjob.Spec.JobTemplate.Labels {
labels[k] = v
var command []string
if len(args) > 1 {

This comment has been minimized.

@deads2k

deads2k Feb 21, 2018

Contributor

args and --from should fail

This comment has been minimized.

@soltysh

soltysh Feb 21, 2018

Contributor

Good catch.

if err != nil {
return fmt.Errorf("failed to fetch job: %v", err)
var jobTemplate *batchv1beta1.JobTemplateSpec
if len(from) != 0 {

This comment has been minimized.

@deads2k

deads2k Feb 21, 2018

Contributor

contain this in a method, so the Run is easier to read.

return fmt.Errorf("from must be an existing cronjob")
}
var ok bool
c.FromCronJob, ok = infos[0].AsVersioned().(*batchv1beta1.CronJob)

This comment has been minimized.

@deads2k

deads2k Feb 21, 2018

Contributor

Yeah, you could grab this in the Run

# Create a Job from a CronJob named "a-cronjob"
kubectl create job --from-cronjob=a-cronjob`))
# Create a job from a CronJob named "a-cronjob"
kubectl create job --from=cronjoba-cronjob`))

This comment has been minimized.

@deads2k

deads2k Feb 21, 2018

Contributor

there should be a slash here: cronjob/name

@deads2k

This comment has been minimized.

Contributor

deads2k commented Feb 21, 2018

Some comments on location. Idea is sound

@soltysh

This comment has been minimized.

Contributor

soltysh commented Feb 22, 2018

Rebased to pick up the recent printer changes.

@soltysh

This comment has been minimized.

Contributor

soltysh commented Feb 22, 2018

/retest

func (c *CreateJobOptions) RunCreateJob() (err error) {
cronjob, err := c.V1Beta1Client.CronJobs(c.Namespace).Get(c.FromCronJob, metav1.GetOptions{})
func (c *CreateJobOptions) RunCreateJob(f cmdutil.Factory) error {

This comment has been minimized.

@deads2k

deads2k Feb 22, 2018

Contributor

We really need to avoid passing the factory. Some places pass a builder around in their struct which I can live with.

Client clientbatchv1.BatchV1Interface
Out io.Writer
DryRun bool
PrintObject func(obj runtime.Object) error

This comment has been minimized.

@deads2k

deads2k Feb 22, 2018

Contributor

This is no longer a point of variability. cmdutil.PrintOptions

@deads2k

This comment has been minimized.

Contributor

deads2k commented Feb 22, 2018

Down to small stuff. Looks good otherwise

/approve

erhudy and others added some commits Feb 15, 2018

Fixes #47538: Add functionality for manually creating a Job instance …
…from a CronJob

This changeset adds the command `kubectl create job` with the flag `--from-cronjob`, which allows a user to create a Job from a CronJob via the CLI.
@deads2k

This comment has been minimized.

Contributor

deads2k commented Feb 22, 2018

/lgtm

@soltysh

This comment has been minimized.

Contributor

soltysh commented Feb 22, 2018

/approve

Adding the label manually since this was not full command needed for bot.

@k8s-ci-robot

This comment has been minimized.

Contributor

k8s-ci-robot commented Feb 22, 2018

[APPROVALNOTIFIER] This PR is APPROVED

Approval requirements bypassed by manually added approval.

This pull-request has been approved by: deads2k, soltysh

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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-merge-robot

This comment has been minimized.

Contributor

k8s-merge-robot commented Feb 23, 2018

Automatic merge from submit-queue (batch tested with PRs 60208, 60084, 60183, 59713, 60096). If you want to cherry-pick this change to another branch, please follow the instructions here.

@k8s-merge-robot k8s-merge-robot merged commit 32fbec0 into kubernetes:master Feb 23, 2018

13 checks passed

Submit Queue Queued to run github e2e tests a second time.
Details
cla/linuxfoundation soltysh authorized
Details
pull-kubernetes-bazel-build Job succeeded.
Details
pull-kubernetes-bazel-test Job succeeded.
Details
pull-kubernetes-cross Skipped
pull-kubernetes-e2e-gce Job succeeded.
Details
pull-kubernetes-e2e-gce-device-plugin-gpu Job succeeded.
Details
pull-kubernetes-e2e-gke Skipped
pull-kubernetes-e2e-kops-aws Job succeeded.
Details
pull-kubernetes-kubemark-e2e-gce Job succeeded.
Details
pull-kubernetes-node-e2e Job succeeded.
Details
pull-kubernetes-unit Job succeeded.
Details
pull-kubernetes-verify Job succeeded.
Details

@soltysh soltysh deleted the soltysh:create_job branch Feb 23, 2018

@MrBlaise

This comment has been minimized.

MrBlaise commented Apr 4, 2018

The Usage suggest that --from-cronjob=CRONJOB should work, however it will throw an unknown flag error. The --from=cronjob/CRONJOB argument works.

k8s-merge-robot added a commit that referenced this pull request Apr 7, 2018

Merge pull request #62215 from soltysh/fix_example
Automatic merge from submit-queue (batch tested with PRs 60900, 62215, 62196). If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

Fix create job usage

**Which issue(s) this PR fixes** *(optional, in `fixes #<issue number>(, fixes #<issue_number>, ...)` format, will close the issue(s) when PR gets merged)*:
Fixes the issue mentioned by @MrBlaise in #60084 (comment)

/assign @juanvallejo 

**Special notes for your reviewer**:

**Release note**:
```release-note
NONE
```

hypnoglow added a commit to hypnoglow/kubernetes that referenced this pull request Apr 11, 2018

Support kubectl create job --from=cronjob/<name> for CronJob v2alpha1
Currently, when trying to create a job from CronJob version v2alpha1, it fails.

Example:

```
$ kubectl get cronjobs
NAME      SCHEDULE    SUSPEND   ACTIVE    LAST SCHEDULE   AGE
foo       0 * * * *   False     0         22m             1h

$ kubectl create job --from cronjob/foo bar
error: from must be an existing cronjob
```

This commit adds support for creating job from CronJob version v2alpha1

References:
- kubernetes#60084

hypnoglow added a commit to hypnoglow/kubernetes that referenced this pull request Apr 11, 2018

Support kubectl create job --from=cronjob/<name> for CronJob v2alpha1
Currently, when trying to create a job from CronJob version v2alpha1, it fails.

Example:

```
$ kubectl get cronjobs
NAME      SCHEDULE    SUSPEND   ACTIVE    LAST SCHEDULE   AGE
foo       0 * * * *   False     0         22m             1h

$ kubectl create job --from cronjob/foo bar
error: from must be an existing cronjob
```

This commit adds support for creating job from CronJob version v2alpha1

References:
- kubernetes#60084

hypnoglow added a commit to hypnoglow/kubernetes that referenced this pull request Apr 13, 2018

Support kubectl create job --from=cronjob/<name> for CronJob v2alpha1
Currently, when trying to create a job from CronJob version v2alpha1, it fails.

Example:

```
$ kubectl get cronjobs
NAME      SCHEDULE    SUSPEND   ACTIVE    LAST SCHEDULE   AGE
foo       0 * * * *   False     0         22m             1h

$ kubectl create job --from cronjob/foo bar
error: from must be an existing cronjob
```

This commit adds support for creating job from CronJob version v2alpha1

References:
- kubernetes#60084

hypnoglow added a commit to hypnoglow/kubernetes that referenced this pull request Apr 13, 2018

Support kubectl create job --from=cronjob/<name> for CronJob v2alpha1
Currently, when trying to create a job from CronJob version v2alpha1, it fails.

Example:

```
$ kubectl get cronjobs
NAME      SCHEDULE    SUSPEND   ACTIVE    LAST SCHEDULE   AGE
foo       0 * * * *   False     0         22m             1h

$ kubectl create job --from cronjob/foo bar
error: from must be an existing cronjob
```

This commit adds support for creating job from CronJob version v2alpha1

References:
- kubernetes#60084

hypnoglow added a commit to hypnoglow/kubernetes that referenced this pull request Apr 13, 2018

Support kubectl create job --from=cronjob/<name> for CronJob v2alpha1
Currently, when trying to create a job from CronJob version v2alpha1, it fails.

Example:

```
$ kubectl get cronjobs
NAME      SCHEDULE    SUSPEND   ACTIVE    LAST SCHEDULE   AGE
foo       0 * * * *   False     0         22m             1h

$ kubectl create job --from cronjob/foo bar
error: from must be an existing cronjob
```

This commit adds support for creating job from CronJob version v2alpha1

References:
- kubernetes#60084

k8s-merge-robot added a commit that referenced this pull request Aug 1, 2018

Merge pull request #60316 from soltysh/create_job
Automatic merge from submit-queue. If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

Full blown kubectl create job

**What this PR does / why we need it**:
This is a followup to #60084 which adds full blown `create job` command

/assign @deads2k @juanvallejo 

**Release note**:

```release-note
Add kubectl create job command
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment