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.

Sign up for GitHub

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

Jump to bottom

Add test for CronJob generator #38929

Merged
merged 1 commit into from
Dec 20, 2016
Merged

Add test for CronJob generator #38929

merged 1 commit into from
Dec 20, 2016
+98 −0

Conversation

soltysh
Copy link
Contributor

@soltysh soltysh commented Dec 17, 2016

Per @janetkuo request this was split from #38614, it adds test for CronJob generator, and additionally copy labels to JobTemplate, to be consistent with how Job generator works.

@soltysh soltysh added area/batch area/kubectl sig/cli Categorizes an issue or PR as relevant to SIG CLI. labels Dec 17, 2016
@soltysh soltysh requested a review from janetkuo December 17, 2016 20:01
@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Dec 17, 2016
@k8s-reviewable
Copy link

This change is Reviewable

@k8s-github-robot k8s-github-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. release-note-label-needed labels Dec 17, 2016
@soltysh soltysh assigned janetkuo and unassigned smarterclayton Dec 17, 2016
@janetkuo janetkuo added release-note-none Denotes a PR that doesn't merit a release note. and removed release-note-label-needed labels Dec 19, 2016
janetkuo
janetkuo reviewed Dec 20, 2016
View reviewed changes
pkg/kubectl/run.go
@@ -368,6 +368,9 @@ func (CronJobV2Alpha1) Generate(genericParams map[string]interface{}) (runtime.O
Schedule: params["schedule"],
ConcurrencyPolicy: batch.AllowConcurrent,
JobTemplate: batch.JobTemplateSpec{
ObjectMeta: api.ObjectMeta{
Labels: labels,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we need this change? Note that by default cronjob controller doesn't set jobTemplate.labels, and it will be copied from jobTemplate.spec.template.labels upon job creation

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't check that, in that case this is not needed, indeed. Will remove.

@soltysh
Copy link
Contributor Author

soltysh commented Dec 20, 2016

@janetkuo updated, ptal

@k8s-ci-robot
Copy link
Contributor

Jenkins Bazel Build failed for commit 4855ae2. Full PR test history.

The magic incantation to run this job again is @k8s-bot bazel test this. Please help us cut down flakes by linking to an open flake issue when you hit one in your PR.

@k8s-github-robot k8s-github-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Dec 20, 2016
@k8s-ci-robot
Copy link
Contributor

Jenkins GCE etcd3 e2e failed for commit 4855ae2. Full PR test history.

The magic incantation to run this job again is @k8s-bot gce etcd3 e2e test this. Please help us cut down flakes by linking to an open flake issue when you hit one in your PR.

@soltysh
Copy link
Contributor Author

soltysh commented Dec 20, 2016

@k8s-bot test this #38944

@janetkuo
Copy link
Member

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Dec 20, 2016
@k8s-github-robot
Copy link

@k8s-bot test this [submit-queue is verifying that this PR is safe to merge]

@k8s-github-robot
Copy link

Automatic merge from submit-queue

@k8s-github-robot k8s-github-robot merged commit cc21520 into kubernetes:master Dec 20, 2016
@soltysh soltysh deleted the cronjob_gen_test branch December 21, 2016 09:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@janetkuo janetkuo janetkuo approved these changes

Assignees

@janetkuo janetkuo

Labels
area/batch area/kubectl cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. release-note-none Denotes a PR that doesn't merit a release note. sig/cli Categorizes an issue or PR as relevant to SIG CLI. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@soltysh @k8s-reviewable @k8s-ci-robot @janetkuo @k8s-github-robot @smarterclayton