Generate a schema for the ProwJob crd #19522

alvaroaleman · 2020-10-11T23:46:44Z

This PR adds codegeneration that uses controller-gen to generate a CRD
schema for ProwJobs based on the go types. This provides us with two
advantages:

It is not possible anymore to submit jobs with an invalid schema.
Doing this could result in Prow going down completely.
kubectl explain can be used to get info about the fields in a
prowjob

The only drawback of this change is that we lose the validation that .status.completionTime must be set if .status.state is in success, error, failure,as controller-gen doesn't seem to support cross-field validation.

Fixes #14925

alvaroaleman · 2020-10-12T17:06:34Z

/hold
It seems the generated schema is not actually valid:

The CustomResourceDefinition "prowjobs.prow.k8s.io" is invalid: 
* metadata.annotations: Too long: must have at most 262144 bytes
* spec.validation.openAPIV3Schema.properties[spec].properties[pipeline_run_spec].properties[params].items.properties[value].type: Unsupported value: "Any": supported values: "array", "boolean", "integer", "number", "object", "string"
* spec.validation.openAPIV3Schema.properties[spec].properties[pipeline_run_spec].properties[pipelineSpec].properties[params].items.properties[default].type: Unsupported value: "Any": supported values: "array", "boolean", "integer", "number", "object", "string"
* spec.validation.openAPIV3Schema.properties[spec].properties[pipeline_run_spec].properties[pipelineSpec].properties[tasks].items.properties[params].items.properties[value].type: Unsupported value: "Any": supported values: "array", "boolean", "integer", "number", "object", "string"
* spec.validation.openAPIV3Schema.properties[spec].properties[pipeline_run_spec].properties[pipelineSpec].properties[tasks].items.properties[conditions].items.properties[params].items.properties[value].type: Unsupported value: "Any": supported values: "array", "boolean", "integer", "number", "object", "string"
* spec.validation.openAPIV3Schema.properties[spec].properties[pipeline_run_spec].properties[pipelineSpec].properties[tasks].items.properties[taskSpec].properties[steps].items.properties[ports].items.properties[protocol].default: Required value: this property is in x-kubernetes-list-map-keys, so it must have a default or be a required property
* spec.validation.openAPIV3Schema.properties[spec].properties[pipeline_run_spec].properties[pipelineSpec].properties[tasks].items.properties[taskSpec].properties[stepTemplate].properties[ports].items.properties[protocol].default: Required value: this property is in x-kubernetes-list-map-keys, so it must have a default or be a required property
* spec.validation.openAPIV3Schema.properties[spec].properties[pipeline_run_spec].properties[pipelineSpec].properties[tasks].items.properties[taskSpec].properties[inputs].properties[params].items.properties[default].type: Unsupported value: "Any": supported values: "array", "boolean", "integer", "number", "object", "string"
* spec.validation.openAPIV3Schema.properties[spec].properties[pipeline_run_spec].properties[pipelineSpec].properties[tasks].items.properties[taskSpec].properties[params].items.properties[default].type: Unsupported value: "Any": supported values: "array", "boolean", "integer", "number", "object", "string"
* spec.validation.openAPIV3Schema.properties[spec].properties[pipeline_run_spec].properties[pipelineSpec].properties[tasks].items.properties[taskSpec].properties[sidecars].items.properties[ports].items.properties[protocol].default: Required value: this property is in x-kubernetes-list-map-keys, so it must have a default or be a required property
* spec.validation.openAPIV3Schema.properties[spec].properties[pod_spec].properties[containers].items.properties[ports].items.properties[protocol].default: Required value: this property is in x-kubernetes-list-map-keys, so it must have a default or be a required property
* spec.validation.openAPIV3Schema.properties[spec].properties[pod_spec].properties[initContainers].items.properties[ports].items.properties[protocol].default: Required value: this property is in x-kubernetes-list-map-keys, so it must have a default or be a required property
* spec.validation.openAPIV3Schema.properties[spec].properties[decoration_config].properties[timeout].type: Unsupported value: "Any": supported values: "array", "boolean", "integer", "number", "object", "string"
* spec.validation.openAPIV3Schema.properties[spec].properties[decoration_config].properties[grace_period].type: Unsupported value: "Any": supported values: "array", "boolean", "integer", "number", "object", "string"

fejta-bot · 2021-07-07T05:13:43Z

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-contributor-experience at kubernetes/community.
/lifecycle stale

k8s-triage-robot · 2021-08-06T05:15:58Z

Stale issues rot after 30d of inactivity.
Mark the issue as fresh with /remove-lifecycle rotten.
Rotten issues close after an additional 30d of inactivity.

If this issue is safe to close now please do so with /close.

Send feedback to sig-contributor-experience at kubernetes/community.
/lifecycle rotten

alvaroaleman · 2021-08-28T01:31:24Z

/label tide/merge-method-squash

alvaroaleman · 2021-09-10T16:08:59Z

@chaodaiG @cjwagner ptal, I believe I have addressed all remarks now

chaodaiG · 2021-09-10T16:11:20Z

config/prow/cluster/BUILD.bazel

+    component(
+        "prowjob",
+        "customresourcedefinition",
+        args = ["--server-side=true"],


nit: we're just speculating that this will work, but not sure. might suggest move this change to a separate PR just in case revert is needed we don't need to revert this large PR

Well, not doing this will definitely not work as the PR changes the CRD file 😬

well, finger crossed then

chaodaiG

/hold

LGTM other than my nit

k8s-ci-robot · 2021-09-10T16:11:53Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: alvaroaleman, chaodaiG

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~~ [chaodaiG]

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

chaodaiG · 2021-09-10T16:26:14Z

/unhold

This reverts commit 914737a. Seems like server side apply didn't work: https://prow.k8s.io/view/gs/kubernetes-jenkins/logs/post-test-infra-deploy-prow/1436369391678656512

Revert "Generate a schema for the ProwJob crd (#19522)"

…)"" This reverts commit 0b1d48b.

k8s-ci-robot requested review from cblecker, cjwagner and wojtek-t October 11, 2020 23:47

alvaroaleman force-pushed the crd-schema branch 2 times, most recently from 493cae1 to a83dcad Compare October 12, 2020 02:20

k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Oct 12, 2020

alvaroaleman mentioned this pull request Oct 12, 2020

Issue generating CRDs with controller-gen 0.4.0 kubernetes-sigs/controller-tools#502

Closed

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

alvaroaleman force-pushed the crd-schema branch from a83dcad to ed86605 Compare February 4, 2021 14:25

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

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

alvaroaleman mentioned this pull request Apr 23, 2021

Updating the prowjob CRD to v1 #21805

Merged

k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Jul 7, 2021

k8s-ci-robot added lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Aug 6, 2021

alvaroaleman mentioned this pull request Aug 24, 2021

Improve admissions controller to prevent Prow jobs with bad config from taking down Prow. #14925

Closed

alvaroaleman force-pushed the crd-schema branch from ed86605 to 9c7b0e2 Compare August 28, 2021 00:36

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

alvaroaleman force-pushed the crd-schema branch 2 times, most recently from 4f9f4c2 to dca7991 Compare August 28, 2021 01:25

alvaroaleman added 11 commits September 10, 2021 11:40

SSA can not default the port?

69a7027

Protocols for everyone

491a322

Remove go 1.17 header

24c2a4c

Link the crd instead of duplicating it

dda496a

Update getting started docs

02565dd

Remove stale copyfiles

5328b15

Re-Add validation that completionTime must be set

5fa7390

Apply ProwJob crd with --server-side=true

e597c6e

Add announcement and schemaless manifest as fallback

ed7b897

Args is a list

e773593

Must specify protocol

429fc65

alvaroaleman force-pushed the crd-schema branch from 248599f to 429fc65 Compare September 10, 2021 15:42

chaodaiG reviewed Sep 10, 2021

View reviewed changes

chaodaiG approved these changes Sep 10, 2021

View reviewed changes

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

k8s-ci-robot assigned chaodaiG Sep 10, 2021

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

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Sep 10, 2021

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 10, 2021

k8s-ci-robot merged commit 914737a into kubernetes:master Sep 10, 2021

k8s-ci-robot added this to the v1.23 milestone Sep 10, 2021

k8s-ci-robot added a commit that referenced this pull request Sep 10, 2021

Merge pull request #23547 from chaodaiG/revert-crd

dd2e904

Revert "Generate a schema for the ProwJob crd (#19522)"

chaodaiG mentioned this pull request Sep 10, 2021

Revert "Generate a schema for the ProwJob crd (#19522)" #23547

Merged

alvaroaleman added a commit to alvaroaleman/test-infra that referenced this pull request Sep 16, 2021

Revert "Revert "Generate a schema for the ProwJob crd (kubernetes#19522…

e2f54b0

…)"" This reverts commit 0b1d48b.

alvaroaleman mentioned this pull request Sep 16, 2021

Re-Add prowjob crd with schema #23640

Merged

palnabarun mentioned this pull request Nov 15, 2021

Add merge restrictions for Kubernetes 1.23 Code Freeze palnabarun/test-infra#1

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Generate a schema for the ProwJob crd #19522

Generate a schema for the ProwJob crd #19522

alvaroaleman commented Oct 11, 2020

alvaroaleman commented Oct 12, 2020

fejta-bot commented Jul 7, 2021

k8s-triage-robot commented Aug 6, 2021

alvaroaleman commented Aug 28, 2021

alvaroaleman commented Sep 10, 2021

chaodaiG Sep 10, 2021

alvaroaleman Sep 10, 2021

chaodaiG Sep 10, 2021

chaodaiG left a comment

k8s-ci-robot commented Sep 10, 2021

chaodaiG commented Sep 10, 2021

Generate a schema for the ProwJob crd #19522

Generate a schema for the ProwJob crd #19522

Conversation

alvaroaleman commented Oct 11, 2020

alvaroaleman commented Oct 12, 2020

fejta-bot commented Jul 7, 2021

k8s-triage-robot commented Aug 6, 2021

alvaroaleman commented Aug 28, 2021

alvaroaleman commented Sep 10, 2021

chaodaiG Sep 10, 2021

Choose a reason for hiding this comment

alvaroaleman Sep 10, 2021

Choose a reason for hiding this comment

chaodaiG Sep 10, 2021

Choose a reason for hiding this comment

chaodaiG left a comment

Choose a reason for hiding this comment

k8s-ci-robot commented Sep 10, 2021

chaodaiG commented Sep 10, 2021