From c67c7f98b7b38a28009b34a275050e967fb3e1a3 Mon Sep 17 00:00:00 2001 From: Dennis Periquet Date: Mon, 2 May 2022 07:11:23 -0400 Subject: [PATCH 1/4] Add Early test to check for CRD subresource.status --- test/extended/operators/check_crd_status.go | 127 ++++++++++++++++++ .../operators/check_crd_status_test.go | 51 +++++++ .../generated/zz_generated.annotations.go | 2 + 3 files changed, 180 insertions(+) create mode 100644 test/extended/operators/check_crd_status.go create mode 100644 test/extended/operators/check_crd_status_test.go diff --git a/test/extended/operators/check_crd_status.go b/test/extended/operators/check_crd_status.go new file mode 100644 index 000000000000..d406a4fe210a --- /dev/null +++ b/test/extended/operators/check_crd_status.go @@ -0,0 +1,127 @@ +package operators + +import ( + "context" + "fmt" + "strings" + + g "github.com/onsi/ginkgo" + o "github.com/onsi/gomega" + "k8s.io/kube-openapi/pkg/util/sets" + e2e "k8s.io/kubernetes/test/e2e/framework" + + exutil "github.com/openshift/origin/test/extended/util" + apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" + apiextensionsclientset "k8s.io/apiextensions-apiserver/pkg/client/clientset/clientset" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +// checkCRDs returns a list of names of CRDs that don't have a "status" in the CRD schema +// and also a subresource.status defined. +// For now, it ignores the ones that are currently failing. +func checkCRDs(crdItemList []apiextensionsv1.CustomResourceDefinition) []string { + + // These CRDs, at the time this test was written, do not have a "status" in the CRD schema + // and subresource.status. + // These can be skipped for now but we don't want the number to increase. + // These CRDs should be tidied up over time. + // + exceptionsList := sets.NewString( + "builds.config.openshift.io", + "clusternetworks.network.openshift.io", + "consoleclidownloads.console.openshift.io", + "consoleexternalloglinks.console.openshift.io", + "consolelinks.console.openshift.io", + "consolenotifications.console.openshift.io", + "consoleplugins.console.openshift.io", + "consolequickstarts.console.openshift.io", + "consoleyamlsamples.console.openshift.io", + "egressnetworkpolicies.network.openshift.io", + "hostsubnets.network.openshift.io", + "imagecontentpolicies.config.openshift.io", + "imagecontentsourcepolicies.operator.openshift.io", + "machineconfigs.machineconfiguration.openshift.io", + "netnamespaces.network.openshift.io", + "networks.config.openshift.io", + "networks.operator.openshift.io", + "operatorpkis.network.operator.openshift.io", + "profiles.tuned.openshift.io", + "rangeallocations.security.internal.openshift.io", + "rolebindingrestrictions.authorization.openshift.io", + "securitycontextconstraints.security.openshift.io", + "tuneds.tuned.openshift.io", + ) + + failures := []string{} + for _, crdItem := range crdItemList { + + // This test is interested only in CRDs that end with "openshift.io". + if !strings.HasSuffix(crdItem.ObjectMeta.Name, "openshift.io") { + continue + } + + crdName := crdItem.ObjectMeta.Name + + // Skip CRDs in the exceptions list for now. + if exceptionsList.Has(crdName) { + continue + } + + // Iterate through all versions of the CustomResourceDefinition Spec looking for one with + // a schema status element, + foundStatusInSchema := false + var i int + for i = 0; i < len(crdItem.Spec.Versions); i++ { + if _, ok := crdItem.Spec.Versions[i].Schema.OpenAPIV3Schema.Properties["status"]; ok { + foundStatusInSchema = true + break + } + } + + if foundStatusInSchema { + if !(crdItem.Spec.Versions[i].Subresources != nil && crdItem.Spec.Versions[i].Subresources.Status != nil) { + failures = append(failures, fmt.Sprintf("CRD %s has a status in the schema but no subresource.status", crdName)) + } + } else { + failures = append(failures, fmt.Sprintf("CRD %s has no 'status' element in its schema", crdName)) + } + } + + return failures +} + +var _ = g.Describe("[sig-arch][Early]", func() { + + defer g.GinkgoRecover() + + var ( + crd_item_list []apiextensionsv1.CustomResourceDefinition + ) + + oc := exutil.NewCLI("subresource-status-check") + + g.BeforeEach(func() { + var err error + crdClient := apiextensionsclientset.NewForConfigOrDie(oc.AdminConfig()) + crd_item_list, err = getCRDItemList(*crdClient) + o.Expect(err).NotTo(o.HaveOccurred()) + }) + + g.Describe("CRDs for openshift.io", func() { + g.It("should have subresource.status", func() { + failures := checkCRDs(crd_item_list) + if len(failures) > 0 { + e2e.Fail(strings.Join(failures, "\n")) + } + }) + }) +}) + +func getCRDItemList(crdClient apiextensionsclientset.Clientset) ([]apiextensionsv1.CustomResourceDefinition, error) { + + crdList, err := crdClient.ApiextensionsV1().CustomResourceDefinitions().List(context.Background(), metav1.ListOptions{}) + if err != nil { + return nil, err + } + return crdList.Items, err +} diff --git a/test/extended/operators/check_crd_status_test.go b/test/extended/operators/check_crd_status_test.go new file mode 100644 index 000000000000..71d100cc3285 --- /dev/null +++ b/test/extended/operators/check_crd_status_test.go @@ -0,0 +1,51 @@ +package operators + +import ( + "fmt" + "os" + "strings" + "testing" + + exutil "github.com/openshift/origin/test/extended/util" + apiextensionsclientset "k8s.io/apiextensions-apiserver/pkg/client/clientset/clientset" +) + +func Test_checkCRDs(t *testing.T) { + + crdClient := setupLocalAPIClientset() + t.Run("check_crds test", func(t *testing.T) { + crdList, err := getCRDItemList(*crdClient) + if err != nil { + t.Errorf("Fail: %s", err) + } + failures := checkCRDs(crdList) + if len(failures) > 0 { + t.Error("There should be no failures") + for _, i := range failures { + if strings.Contains(i, "has no 'status' element in its schema") { + fmt.Println(i) + } + } + for _, i := range failures { + if !strings.Contains(i, "has no 'status' element in its schema") { + fmt.Println(i) + } + } + } + }) +} + +func setupLocalAPIClientset() *apiextensionsclientset.Clientset { + // Get the kubeconfig by creating an Openshift cluster with cluster-bot, downloading it, + // and using the filename for KUBECONFIG. + home_dir := os.Getenv("HOME") + err := os.Setenv("KUBECONFIG", fmt.Sprintf("%s/Downloads/cluster-bot-2022-04-07-164806.kubeconfig.txt", home_dir)) + kube_dir := os.Getenv("KUBECONFIG") + fmt.Println(kube_dir) + if err != nil { + fmt.Printf("Error setting KUBECONFIG: %s", err) + } + oc := exutil.NewCLI("default") + local_client := apiextensionsclientset.NewForConfigOrDie(oc.AdminConfig()) + return local_client +} diff --git a/test/extended/util/annotate/generated/zz_generated.annotations.go b/test/extended/util/annotate/generated/zz_generated.annotations.go index 2592787eb93b..688ecb88b59a 100644 --- a/test/extended/util/annotate/generated/zz_generated.annotations.go +++ b/test/extended/util/annotate/generated/zz_generated.annotations.go @@ -645,6 +645,8 @@ var annotations = map[string]string{ "[Top Level] [sig-arch] ocp payload should be based on existing source OLM version should contain the source commit id": "OLM version should contain the source commit id [Suite:openshift/conformance/parallel]", + "[Top Level] [sig-arch][Early] CRDs for openshift.io should have subresource.status": "should have subresource.status [Suite:openshift/conformance/parallel]", + "[Top Level] [sig-arch][Early] Managed cluster should start all core operators": "start all core operators [Skipped:Disconnected] [Suite:openshift/conformance/parallel]", "[Top Level] [sig-arch][Feature:ClusterUpgrade] Cluster should remain functional during upgrade [Disruptive]": "Cluster should remain functional during upgrade [Disruptive] [Serial]", From 7ee3e2a98b0c52f8cc08298f66b31fa214bbc08c Mon Sep 17 00:00:00 2001 From: Dennis Periquet Date: Mon, 9 May 2022 11:16:26 -0400 Subject: [PATCH 2/4] Two separate tests: one for status in schema, one for subresource.status --- test/extended/operators/check_crd_status.go | 103 +++++++++++++++--- .../operators/check_crd_status_test.go | 2 +- 2 files changed, 88 insertions(+), 17 deletions(-) diff --git a/test/extended/operators/check_crd_status.go b/test/extended/operators/check_crd_status.go index d406a4fe210a..64c04caa4ffa 100644 --- a/test/extended/operators/check_crd_status.go +++ b/test/extended/operators/check_crd_status.go @@ -16,10 +16,63 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) -// checkCRDs returns a list of names of CRDs that don't have a "status" in the CRD schema -// and also a subresource.status defined. +// checkSubresourceStatus returns a list of names of CRDs that have a "status" in the CRD schema +// but no subresource.status defined. // For now, it ignores the ones that are currently failing. -func checkCRDs(crdItemList []apiextensionsv1.CustomResourceDefinition) []string { +func checkSubresourceStatus(crdItemList []apiextensionsv1.CustomResourceDefinition) []string { + + // These CRDs, at the time this test was written, do not have a "status" in the CRD schema + // and subresource.status. + // These can be skipped for now but we don't want the number to increase. + // These CRDs should be tidied up over time. + // + exceptionsList := sets.NewString( + "networks.config.openshift.io", + "networks.operator.openshift.io", + "operatorpkis.network.operator.openshift.io", + "profiles.tuned.openshift.io", + "tuneds.tuned.openshift.io", + ) + + failures := []string{} + for _, crdItem := range crdItemList { + + // This test is interested only in CRDs that end with "openshift.io". + if !strings.HasSuffix(crdItem.ObjectMeta.Name, "openshift.io") { + continue + } + + crdName := crdItem.ObjectMeta.Name + + // Skip CRDs in the exceptions list for now. + if exceptionsList.Has(crdName) { + continue + } + + // Iterate through all versions of the CustomResourceDefinition Spec looking for one with + // a schema status element, + foundStatusInSchema := false + var i int + for i = 0; i < len(crdItem.Spec.Versions); i++ { + if _, ok := crdItem.Spec.Versions[i].Schema.OpenAPIV3Schema.Properties["status"]; ok { + foundStatusInSchema = true + break + } + } + + if foundStatusInSchema { + if !(crdItem.Spec.Versions[i].Subresources != nil && crdItem.Spec.Versions[i].Subresources.Status != nil) { + failures = append(failures, fmt.Sprintf("CRD %s has a status in the schema but no subresource.status", crdName)) + } + } + } + + return failures +} + +// checkStatusInSchema returns a list of names of CRDs that don't have a "status" in the CRD schema. +// For now, it ignores the ones that are currently failing. +func checkStatusInSchema(crdItemList []apiextensionsv1.CustomResourceDefinition) []string { // These CRDs, at the time this test was written, do not have a "status" in the CRD schema // and subresource.status. @@ -42,14 +95,9 @@ func checkCRDs(crdItemList []apiextensionsv1.CustomResourceDefinition) []string "imagecontentsourcepolicies.operator.openshift.io", "machineconfigs.machineconfiguration.openshift.io", "netnamespaces.network.openshift.io", - "networks.config.openshift.io", - "networks.operator.openshift.io", - "operatorpkis.network.operator.openshift.io", - "profiles.tuned.openshift.io", "rangeallocations.security.internal.openshift.io", "rolebindingrestrictions.authorization.openshift.io", "securitycontextconstraints.security.openshift.io", - "tuneds.tuned.openshift.io", ) failures := []string{} @@ -78,11 +126,7 @@ func checkCRDs(crdItemList []apiextensionsv1.CustomResourceDefinition) []string } } - if foundStatusInSchema { - if !(crdItem.Spec.Versions[i].Subresources != nil && crdItem.Spec.Versions[i].Subresources.Status != nil) { - failures = append(failures, fmt.Sprintf("CRD %s has a status in the schema but no subresource.status", crdName)) - } - } else { + if !foundStatusInSchema { failures = append(failures, fmt.Sprintf("CRD %s has no 'status' element in its schema", crdName)) } } @@ -95,7 +139,7 @@ var _ = g.Describe("[sig-arch][Early]", func() { defer g.GinkgoRecover() var ( - crd_item_list []apiextensionsv1.CustomResourceDefinition + crdItemList []apiextensionsv1.CustomResourceDefinition ) oc := exutil.NewCLI("subresource-status-check") @@ -103,13 +147,40 @@ var _ = g.Describe("[sig-arch][Early]", func() { g.BeforeEach(func() { var err error crdClient := apiextensionsclientset.NewForConfigOrDie(oc.AdminConfig()) - crd_item_list, err = getCRDItemList(*crdClient) + crdItemList, err = getCRDItemList(*crdClient) o.Expect(err).NotTo(o.HaveOccurred()) }) g.Describe("CRDs for openshift.io", func() { g.It("should have subresource.status", func() { - failures := checkCRDs(crd_item_list) + failures := checkSubresourceStatus(crdItemList) + if len(failures) > 0 { + e2e.Fail(strings.Join(failures, "\n")) + } + }) + }) +}) + +var _ = g.Describe("[sig-arch][Early]", func() { + + defer g.GinkgoRecover() + + var ( + crdItemList []apiextensionsv1.CustomResourceDefinition + ) + + oc := exutil.NewCLI("schema-status-check") + + g.BeforeEach(func() { + var err error + crdClient := apiextensionsclientset.NewForConfigOrDie(oc.AdminConfig()) + crdItemList, err = getCRDItemList(*crdClient) + o.Expect(err).NotTo(o.HaveOccurred()) + }) + + g.Describe("CRDs for openshift.io", func() { + g.It("should have a status in the CRD schema", func() { + failures := checkStatusInSchema(crdItemList) if len(failures) > 0 { e2e.Fail(strings.Join(failures, "\n")) } diff --git a/test/extended/operators/check_crd_status_test.go b/test/extended/operators/check_crd_status_test.go index 71d100cc3285..85d1ba66711d 100644 --- a/test/extended/operators/check_crd_status_test.go +++ b/test/extended/operators/check_crd_status_test.go @@ -18,7 +18,7 @@ func Test_checkCRDs(t *testing.T) { if err != nil { t.Errorf("Fail: %s", err) } - failures := checkCRDs(crdList) + failures := checkSubresourceStatus(crdList) if len(failures) > 0 { t.Error("There should be no failures") for _, i := range failures { From 3e7bdd854c08e656690dd900844568a884d68847 Mon Sep 17 00:00:00 2001 From: Dennis Periquet Date: Mon, 9 May 2022 11:19:47 -0400 Subject: [PATCH 3/4] run ./hack/update-generated.sh --- .../util/annotate/generated/zz_generated.annotations.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/extended/util/annotate/generated/zz_generated.annotations.go b/test/extended/util/annotate/generated/zz_generated.annotations.go index 688ecb88b59a..dfc28585eafd 100644 --- a/test/extended/util/annotate/generated/zz_generated.annotations.go +++ b/test/extended/util/annotate/generated/zz_generated.annotations.go @@ -645,6 +645,8 @@ var annotations = map[string]string{ "[Top Level] [sig-arch] ocp payload should be based on existing source OLM version should contain the source commit id": "OLM version should contain the source commit id [Suite:openshift/conformance/parallel]", + "[Top Level] [sig-arch][Early] CRDs for openshift.io should have a status in the CRD schema": "should have a status in the CRD schema [Suite:openshift/conformance/parallel]", + "[Top Level] [sig-arch][Early] CRDs for openshift.io should have subresource.status": "should have subresource.status [Suite:openshift/conformance/parallel]", "[Top Level] [sig-arch][Early] Managed cluster should start all core operators": "start all core operators [Skipped:Disconnected] [Suite:openshift/conformance/parallel]", From 11f92d888d461e67129b71f81db2eac84c17aa98 Mon Sep 17 00:00:00 2001 From: Dennis Periquet Date: Tue, 10 May 2022 07:47:17 -0400 Subject: [PATCH 4/4] update unit test --- .../operators/check_crd_status_test.go | 31 ++++++++++++------- 1 file changed, 20 insertions(+), 11 deletions(-) diff --git a/test/extended/operators/check_crd_status_test.go b/test/extended/operators/check_crd_status_test.go index 85d1ba66711d..de8b4ef720e0 100644 --- a/test/extended/operators/check_crd_status_test.go +++ b/test/extended/operators/check_crd_status_test.go @@ -3,17 +3,16 @@ package operators import ( "fmt" "os" - "strings" "testing" exutil "github.com/openshift/origin/test/extended/util" apiextensionsclientset "k8s.io/apiextensions-apiserver/pkg/client/clientset/clientset" ) -func Test_checkCRDs(t *testing.T) { +func Test_checkSubresourceStatus(t *testing.T) { crdClient := setupLocalAPIClientset() - t.Run("check_crds test", func(t *testing.T) { + t.Run("Test_checkSubresourceStatus test", func(t *testing.T) { crdList, err := getCRDItemList(*crdClient) if err != nil { t.Errorf("Fail: %s", err) @@ -22,24 +21,34 @@ func Test_checkCRDs(t *testing.T) { if len(failures) > 0 { t.Error("There should be no failures") for _, i := range failures { - if strings.Contains(i, "has no 'status' element in its schema") { - fmt.Println(i) - } + fmt.Println(i) } + } + }) +} + +func Test_checkStatusInSchema(t *testing.T) { + + crdClient := setupLocalAPIClientset() + t.Run("Test_checkStatusInSchema test", func(t *testing.T) { + crdList, err := getCRDItemList(*crdClient) + if err != nil { + t.Errorf("Fail: %s", err) + } + failures := checkStatusInSchema(crdList) + if len(failures) > 0 { + t.Error("There should be no failures") for _, i := range failures { - if !strings.Contains(i, "has no 'status' element in its schema") { - fmt.Println(i) - } + fmt.Println(i) } } }) } - func setupLocalAPIClientset() *apiextensionsclientset.Clientset { // Get the kubeconfig by creating an Openshift cluster with cluster-bot, downloading it, // and using the filename for KUBECONFIG. home_dir := os.Getenv("HOME") - err := os.Setenv("KUBECONFIG", fmt.Sprintf("%s/Downloads/cluster-bot-2022-04-07-164806.kubeconfig.txt", home_dir)) + err := os.Setenv("KUBECONFIG", fmt.Sprintf("%s/Downloads/cluster-bot-2022-05-10-100029.kubeconfig.txt", home_dir)) kube_dir := os.Getenv("KUBECONFIG") fmt.Println(kube_dir) if err != nil {