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.

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

Add ScheduledJob to validation #1159

Merged
merged 1 commit into from
Sep 7, 2016

Conversation

johscheuer
Copy link
Contributor

@johscheuer johscheuer commented Sep 5, 2016

Fix the tests for release-1.4 in #1123 @janetkuo added a file for a ScheduledJob this results in a fail:

=== RUN   TestExampleObjectSchemas
--- FAIL: TestExampleObjectSchemas (0.02s)
    examples_test.go:293: ../docs/user-guide/sj.yaml: sj does not have a test case defined

See #1067 for example. I added the file and a validation for the ScheduledJob.


This change is Reviewable

@johscheuer
Copy link
Contributor Author

One question in the tests the following validation step is defined:

    case *batch.Job:
        if t.Namespace == "" {
            t.Namespace = api.NamespaceDefault
        }
        // Job needs generateSelector called before validation, and job.Validate does this.
        // See: https://github.com/kubernetes/kubernetes/issues/20951#issuecomment-187787040
        t.ObjectMeta.UID = types.UID("fakeuid")
        errors = job.Strategy.Validate(nil, t)

We could replace the last line with:

errors = batch_validation.ValidateJob(t)

First attempt resulted in a Fail:

    --- FAIL: TestExampleObjectSchemas (0.01s)
        examples_test.go:326: ../docs/user-guide/job.yaml did not validate correctly: [spec.selector: Required value spec.template.metadata.labels: Invalid value: null: `selector` does not match template `labels`]
        examples_test.go:326: ../docs/user-guide/jobs/work-queue-1/job.yaml did not validate correctly: [spec.selector: Required value spec.template.metadata.labels: Invalid value: null: `selector` does not match template `labels`]
        examples_test.go:326: ../docs/user-guide/jobs/work-queue-2/job.yaml did not validate correctly: [spec.selector: Required value spec.template.metadata.labels: Invalid value: null: `selector` does not match template `labels`]
FAIL

@caesarxuchao
Copy link
Member

LGTM. Thanks @johscheuer.

@pwittrock can we merge this? It's fixing a test that affects all PRs.

@caesarxuchao
Copy link
Member

@johscheuer the validation error you mentioned is expected (see https://github.com/kubernetes/kubernetes/blob/master/pkg/apis/batch/validation/validation.go#L74). In real cluster, the selector and labels will be automatically generated by the server if user doesn't specify them in config. We should mimic this behavior in test.

scheduledjob validation does not verify the selector matches labels, which is a bug IMO, we should fix it. @janetkuo @erictune

@janetkuo
Copy link
Member

janetkuo commented Sep 7, 2016

The test file wasn't in the release-1.4 when the PR #1123 was merged. This change is the same as the one I had here 91a40c0

@janetkuo
Copy link
Member

janetkuo commented Sep 7, 2016

scheduledjob validation does not verify the selector matches labels, which is a bug IMO, we should fix it.

scheduledjob doesn't have selectors

@janetkuo janetkuo merged commit ee28f8d into kubernetes:release-1.4 Sep 7, 2016
@caesarxuchao
Copy link
Member

Thanks @janetkuo !

mikutas pushed a commit to mikutas/k8s-website that referenced this pull request Sep 22, 2022
This commit updates "cluster" to "cross-cluster" in the service
mirroring section.

Fixes kubernetes#1157

Signed-off-by: Ujjwal Goyal <importujjwal@gmail.com>
Okabe-Junya pushed a commit to Okabe-Junya/website that referenced this pull request Dec 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants