BZ 2087504: Add Early test to check for CRD subresource.status by DennisPeriquet · Pull Request #26954 · openshift/origin

DennisPeriquet · 2022-03-31T00:31:45Z

The purpose of this test is to enforce (at least for the ones that have it now), spec/status separation by ensuring:

there is a "status" element in the openapiv3 spec
there is a subresource.status defined in the CRD (which will enable the .../status endpoint) which can be used for the controller to write status.

The CRDs that do not pass are treated as "exceptions" for now. We can open up a BZ for them.

DennisPeriquet · 2022-04-01T12:19:33Z

/retest

dgoodwin · 2022-04-01T16:25:54Z

+
+// check_crds returns a list of names of CRDs that don't have subresource.status
+// For now, it ignores the ones that are currently failing.
+func check_crds(crd_item_list []v1.CustomResourceDefinition) []string {


checkCRDs is the general go format.

dgoodwin · 2022-04-01T16:28:21Z

+	}
+
+	failures := []string{}
+	for _, crd_item := range crd_item_list {


crdItem, crdItemList typically in go. (for these and any other vars/funcs in here, see https://go.dev/doc/effective_go#mixed-caps

dgoodwin · 2022-04-01T16:31:31Z

+		}
+
+		// Use the latest version
+		index := len(crd_item.Spec.Versions) - 1


Is the latest version always the last in the list?

Hm... I looked at the version name field and it can be something like v1, v1beta, etc. and so we can't tell which one is latest. So I added a loop to check all versions. It turns out today, there is only one version in all of the CRDs ending in openshift.io.

Ah good, that makes sense, I recall it was a pretty huge pain to support more than one version at a time.

dgoodwin · 2022-04-01T16:32:49Z

+	})
+})
+
+func get_crd_item_list(crdClient apiextensionsclientset.Clientset) ([]v1.CustomResourceDefinition, error) {


Suggest getCRDs

dgoodwin · 2022-04-01T16:44:06Z

+// check_crd_subresource_status gets the list of CustomeResourceDefinitions and
+// checks if the subresource.status stanza is present.  It also does a count.
+// This is meant to be a test you can run outside of CI.
+//


We don't want to commit this second variant correct?

It should be possible to run a specific ginko test, assuming you've oc login'd to the correct cluster under test,

$ with make && ./openshift-tests run-test "[sig-arch] ClusterOperators should define valid related objects [Suite:openshift/conformance/parallel]"

I tried the command above (but just built openshift-tests separately and ran it) and it works. So I remove it. Thanks for the tip.

DennisPeriquet · 2022-04-01T20:39:06Z

/retest-required

DennisPeriquet · 2022-04-02T13:34:38Z

/retest-required

dgoodwin · 2022-04-04T11:05:30Z

/lgtm

deads2k · 2022-04-04T12:10:30Z

+	e2e "k8s.io/kubernetes/test/e2e/framework"
+
+	exutil "github.com/openshift/origin/test/extended/util"
+	v1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"


alias as apiextensionsv1 for consistency

deads2k · 2022-04-04T12:11:47Z

+		for i := 0; i < len(crdItem.Spec.Versions); i++ {
+
+			versionName = crdItem.Spec.Versions[i].Name
+			if crdItem.Spec.Versions[i].Subresources == nil {


if any version has a status, that will be good enough. As opposed to requiring all versions to have a status.

This doesn't look addressed, still looks like we fail for any version missing status.

I'd expect a bool foundVersionWithStatusSubresource, add to failures outside the loop if it never got set to true.

dgoodwin · 2022-04-07T18:23:04Z

+		}
+
+		// Iterate through all versions of the Spec.
+		var versionName string


No need to define this outside the loop.

dgoodwin · 2022-04-08T10:45:22Z

/lgtm

DennisPeriquet · 2022-04-20T21:29:05Z

/retest-required

DennisPeriquet · 2022-04-24T22:59:50Z

/retest-required

openshift-bot · 2022-05-17T18:37:59Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2022-05-17T19:16:58Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2022-05-17T20:21:56Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2022-05-17T20:35:04Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

DennisPeriquet · 2022-05-17T20:39:16Z

/bugzilla refresh

openshift-ci · 2022-05-17T20:39:36Z

@DennisPeriquet: No Bugzilla bug is referenced in the title of this pull request.
To reference a bug, add 'Bug XXX:' to the title of this pull request and request another bug refresh with /bugzilla refresh.

Retaining the bugzilla/valid-bug label as it was manually added.

Details

In response to this:

/bugzilla refresh

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.

openshift-bot · 2022-05-17T20:58:56Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2022-05-17T21:10:56Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2022-05-17T21:34:56Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2022-05-17T22:22:55Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2022-05-17T22:46:56Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2022-05-17T23:58:55Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2022-05-18T00:10:56Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2022-05-18T01:34:56Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2022-05-18T02:34:56Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2022-05-18T02:58:56Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-ci · 2022-05-18T03:41:58Z

@DennisPeriquet: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
ci/prow/e2e-agnostic-cmd	`477306d`	link	false	`/test e2e-agnostic-cmd`
ci/prow/e2e-gcp-ovn-rt-upgrade	`477306d`	link	false	`/test e2e-gcp-ovn-rt-upgrade`
ci/prow/e2e-aws-single-node-upgrade	`477306d`	link	false	`/test e2e-aws-single-node-upgrade`
ci/prow/e2e-aws-cgroupsv2	`477306d`	link	false	`/test e2e-aws-cgroupsv2`

Full PR test history. Your PR dashboard.

Details

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.

openshift-bot · 2022-05-18T03:58:57Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2022-05-18T05:25:05Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2022-05-18T05:49:03Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2022-05-18T06:01:09Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2022-05-18T07:16:05Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2022-05-18T10:44:05Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2022-05-18T12:54:10Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2022-05-18T13:33:06Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

DennisPeriquet changed the title ~~Add Early test to check for CRD subresource.status~~ WIP:Add Early test to check for CRD subresource.status Mar 31, 2022

openshift-ci Bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Mar 31, 2022

DennisPeriquet changed the title ~~WIP:Add Early test to check for CRD subresource.status~~ WIP: Add Early test to check for CRD subresource.status Mar 31, 2022

openshift-ci Bot requested review from deads2k and sttts March 31, 2022 00:32

DennisPeriquet force-pushed the crd_subresource_status branch from 4347b5a to 9af899c Compare April 1, 2022 01:26

DennisPeriquet changed the title ~~WIP: Add Early test to check for CRD subresource.status~~ Add Early test to check for CRD subresource.status Apr 1, 2022

openshift-ci Bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Apr 1, 2022

dgoodwin reviewed Apr 1, 2022

View reviewed changes

DennisPeriquet force-pushed the crd_subresource_status branch from 9af899c to c1968d6 Compare April 1, 2022 16:52

DennisPeriquet force-pushed the crd_subresource_status branch from 91498a7 to c83df77 Compare April 1, 2022 20:46

openshift-ci Bot assigned dgoodwin Apr 4, 2022

openshift-ci Bot added the lgtm Indicates that a PR is ready to be merged. label Apr 4, 2022

deads2k reviewed Apr 4, 2022

View reviewed changes

Comment thread test/extended/operators/check_crd_status.go Outdated

openshift-ci Bot removed the lgtm Indicates that a PR is ready to be merged. label Apr 4, 2022

DennisPeriquet force-pushed the crd_subresource_status branch 3 times, most recently from a5ef5f9 to f43e620 Compare April 7, 2022 18:20

dgoodwin reviewed Apr 7, 2022

View reviewed changes

DennisPeriquet force-pushed the crd_subresource_status branch from f43e620 to 774ec60 Compare April 7, 2022 18:37

openshift-ci Bot added the lgtm Indicates that a PR is ready to be merged. label Apr 8, 2022

deads2k added the bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. label May 17, 2022

DennisPeriquet changed the title ~~Add Early test to check for CRD subresource.status~~ BZ 2087504: Add Early test to check for CRD subresource.status May 17, 2022

openshift-merge-robot merged commit 2210e7a into openshift:master May 18, 2022

DennisPeriquet mentioned this pull request May 19, 2022

BZ2088533: Add subresource.status check exceptions for techpreview jobs #27156

Merged

deads2k mentioned this pull request Apr 2, 2025

MCO-1631: Remove "[sig-arch][Early] CRDs for openshift.io should have a status in the CRD schema" #29643

Merged

Conversation

DennisPeriquet commented Mar 31, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

DennisPeriquet commented Apr 1, 2022

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

DennisPeriquet commented Apr 1, 2022

Uh oh!

DennisPeriquet commented Apr 2, 2022

Uh oh!

dgoodwin commented Apr 4, 2022

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

dgoodwin commented Apr 8, 2022

Uh oh!

DennisPeriquet commented Apr 20, 2022

Uh oh!

DennisPeriquet commented Apr 24, 2022

Uh oh!

openshift-bot commented May 17, 2022

Uh oh!

openshift-bot commented May 17, 2022

Uh oh!

openshift-bot commented May 17, 2022

Uh oh!

openshift-bot commented May 17, 2022

Uh oh!

DennisPeriquet commented May 17, 2022

Uh oh!

openshift-ci Bot commented May 17, 2022

Uh oh!

openshift-bot commented May 17, 2022

Uh oh!

openshift-bot commented May 17, 2022

Uh oh!

openshift-bot commented May 17, 2022

Uh oh!

openshift-bot commented May 17, 2022

Uh oh!

openshift-bot commented May 17, 2022

Uh oh!

openshift-bot commented May 17, 2022

Uh oh!

openshift-bot commented May 18, 2022

Uh oh!

openshift-bot commented May 18, 2022

Uh oh!

openshift-bot commented May 18, 2022

Uh oh!

openshift-bot commented May 18, 2022

Uh oh!

openshift-ci Bot commented May 18, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

DennisPeriquet commented Mar 31, 2022 •

edited

Loading

openshift-ci Bot commented May 18, 2022 •

edited

Loading