test: enable OCP mode and add auto case for uiplugin dashboard #531

lihongyan1 · 2024-07-17T08:16:00Z

#COO-237 Change OCP script to enable OCP mode
#COO-160 Add uiplugin-DB auto testing

jan--f · 2024-07-30T12:11:15Z

test/run-e2e.sh

@@ -209,6 +213,10 @@ parse_args() {
 			RUN_REGEX=$1
 			shift
 			;;
+		--ocp)


I would prefer to have the tests determine whether they run on OCP or not. I think checking for the presence of the ClusterVersion or ClusterOperator CRD should be good enough. We can get this up once during test setup and the framework can pass it to all tests. Then a test can t.Skip if it can't run.

jan--f · 2024-08-12T11:59:21Z

test/e2e/framework/framework.go

@@ -156,3 +170,17 @@ func (f *Framework) CleanUp(t *testing.T, cleanupFunc func()) {
 		}
 	})
 }
+
+func (f *Framework) IsOpenshiftCluster() bool {


Can you add memoization here too like in getOpenShiftClient? I think we can assume this won't change during a test run so we can safely store the result of this and return the cached value on subsequent calls.

jan--f · 2024-08-12T12:15:36Z

test/e2e/framework/framework.go

@@ -156,3 +170,17 @@ func (f *Framework) CleanUp(t *testing.T, cleanupFunc func()) {
 		}
 	})
 }
+
+func (f *Framework) IsOpenshiftCluster() bool {
+	c, err := f.getOpenshiftClient()


I think we might be better off to initialize this in

observability-operator/test/e2e/main_test.go

Line 84 in 62bcd73

func setupFramework() error {

and rely on the client being there. Otherwise we have to remember to initialize this in future functions that needs the openshift client.

jan--f · 2024-08-12T12:30:40Z

test/e2e/framework/framework.go

+		return false
+	}
+	_, err = c.ConfigV1().ClusterVersions().Get(context.TODO(), "version", metav1.GetOptions{})
+	if err != nil {


Not sure about this one. To me this reads we implicitly assume the error is a 404, in which case we can skip a test. If this returns a different error (say a 5xx) we would want to propagate an error. Otherwise we risk skipping the openshift tests even if we are in an openshift cluster. @simonpasquier @machine424 wdyt?

We might be better off to only return false if apierrors.IsNotFound(err).

openshift-ci · 2024-08-20T05:27:43Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: lihongyan1

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:

~~test/OWNERS~~ [lihongyan1]

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

lihongyan1 · 2024-08-20T06:56:46Z

/test images

lihongyan1 · 2024-08-20T08:42:38Z

ci/prow/images job failed not for obo, ocp-testplatform team solve in progress

lihongyan1 · 2024-09-02T02:00:23Z

/test images

lihongyan1 · 2024-09-11T05:37:36Z

@jan--f @simonpasquier @jgbernalp please help review, I would like this to merge so that we can run on IBM P/Z prow job in future.

simonpasquier · 2024-09-11T14:18:31Z

test/e2e/main_test.go

@@ -83,17 +86,28 @@ func main(m *testing.M) int {

 func setupFramework() error {
 	cfg := config.GetConfigOrDie()
+	scheme := operator.NewScheme(&operator.OperatorConfiguration{})
+	err := configv1.AddToScheme(scheme)


The doc says that AddToScheme is deprecated.

Suggested change

err := configv1.AddToScheme(scheme)

err := configv1.Install(scheme)

simonpasquier · 2024-09-11T14:20:13Z

test/e2e/main_test.go

+
+func isOpenshiftCluster(k8sClient client.Client) (bool, error) {
+	clusterVersion := &configv1.ClusterVersion{}
+	err := k8sClient.Get(context.TODO(), client.ObjectKey{Name: "version"}, clusterVersion)


Background() is fine to use here.

Suggested change

err := k8sClient.Get(context.TODO(), client.ObjectKey{Name: "version"}, clusterVersion)

err := k8sClient.Get(context.Background(), client.ObjectKey{Name: "version"}, clusterVersion)

simonpasquier · 2024-09-11T14:25:34Z

test/e2e/main_test.go

+	err := k8sClient.Get(context.TODO(), client.ObjectKey{Name: "version"}, clusterVersion)
+	if err == nil {
+		return true, nil
+	} else if strings.Contains(err.Error(), `no matches for kind "ClusterVersion" in version "config.openshift.io/v1"`) {


Can we use https://pkg.go.dev/k8s.io/apimachinery/pkg/api/meta#IsNoMatchError to check the error type rather than looking at the error string?

Change OCP script to enable OCP mode Add uiplugin auto testing

Check if cluster is OCP by checking for the presence of the ClusterVersion

remove openshift client go

lihongyan1 · 2024-09-30T01:45:33Z

/retest

jan--f · 2024-09-30T14:04:13Z

/lgtm

lihongyan1 requested a review from a team as a code owner July 17, 2024 08:16

lihongyan1 requested review from danielmellado and simonpasquier and removed request for a team July 17, 2024 08:16

openshift-ci bot requested review from jgbernalp and periklis July 17, 2024 08:16

openshift-ci bot added the approved label Jul 17, 2024

lihongyan1 force-pushed the test-uiplugin-db branch 5 times, most recently from bc2d54f to 11f3282 Compare July 19, 2024 05:13

lihongyan1 changed the title ~~test: add auto case for uiplugin dashboard~~ test: enable OCP mode and add auto case for uiplugin dashboard Jul 19, 2024

lihongyan1 mentioned this pull request Jul 29, 2024

OCP feature gate openshift/release#54662

Merged

jan--f reviewed Jul 30, 2024

View reviewed changes

lihongyan1 force-pushed the test-uiplugin-db branch from 11f3282 to 69a1a7f Compare August 6, 2024 08:45

openshift-ci bot removed the approved label Aug 6, 2024

lihongyan1 force-pushed the test-uiplugin-db branch 5 times, most recently from 5f7ba2d to 62bcd73 Compare August 7, 2024 03:11

jan--f reviewed Aug 12, 2024

View reviewed changes

lihongyan1 force-pushed the test-uiplugin-db branch from 449e630 to 05f347c Compare August 20, 2024 05:27

openshift-ci bot added the approved label Aug 20, 2024

lihongyan1 force-pushed the test-uiplugin-db branch 4 times, most recently from 29e90f5 to a43e6bb Compare August 20, 2024 06:26

simonpasquier reviewed Sep 11, 2024

View reviewed changes

lihongyan1 added 3 commits September 18, 2024 15:50

test: add auto case for uiplugin dashboard

89073d0

Change OCP script to enable OCP mode Add uiplugin auto testing

fix: check if cluster is OCP

18c35c7

Check if cluster is OCP by checking for the presence of the ClusterVersion

fix: use controller runtime client and memorize ocp cluster check

4a9f5b6

remove openshift client go

lihongyan1 force-pushed the test-uiplugin-db branch 2 times, most recently from 31c727b to 24b7494 Compare September 18, 2024 08:57

fix: review comments

a187ddb

lihongyan1 force-pushed the test-uiplugin-db branch from 24b7494 to a187ddb Compare September 18, 2024 09:24

openshift-ci bot assigned jan--f Sep 30, 2024

openshift-ci bot added the lgtm label Sep 30, 2024

openshift-merge-bot bot merged commit 6cbfed6 into rhobs:main Sep 30, 2024
11 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

test: enable OCP mode and add auto case for uiplugin dashboard #531

test: enable OCP mode and add auto case for uiplugin dashboard #531

Uh oh!

lihongyan1 commented Jul 17, 2024 •

edited by openshift-ci bot

Loading

Uh oh!

jan--f Jul 30, 2024

Uh oh!

jan--f Aug 12, 2024

Uh oh!

jan--f Aug 12, 2024

Uh oh!

jan--f Aug 12, 2024

Uh oh!

openshift-ci bot commented Aug 20, 2024

Uh oh!

lihongyan1 commented Aug 20, 2024

Uh oh!

lihongyan1 commented Aug 20, 2024

Uh oh!

lihongyan1 commented Sep 2, 2024

Uh oh!

lihongyan1 commented Sep 11, 2024

Uh oh!

simonpasquier Sep 11, 2024

Uh oh!

simonpasquier Sep 11, 2024

Uh oh!

simonpasquier Sep 11, 2024

Uh oh!

lihongyan1 commented Sep 30, 2024

Uh oh!

jan--f commented Sep 30, 2024

Uh oh!

Uh oh!

Uh oh!

	err := configv1.AddToScheme(scheme)
	err := configv1.Install(scheme)

	err := k8sClient.Get(context.TODO(), client.ObjectKey{Name: "version"}, clusterVersion)
	err := k8sClient.Get(context.Background(), client.ObjectKey{Name: "version"}, clusterVersion)

test: enable OCP mode and add auto case for uiplugin dashboard #531

test: enable OCP mode and add auto case for uiplugin dashboard #531

Uh oh!

Conversation

lihongyan1 commented Jul 17, 2024 • edited by openshift-ci bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jan--f Jul 30, 2024

Choose a reason for hiding this comment

Uh oh!

jan--f Aug 12, 2024

Choose a reason for hiding this comment

Uh oh!

jan--f Aug 12, 2024

Choose a reason for hiding this comment

Uh oh!

jan--f Aug 12, 2024

Choose a reason for hiding this comment

Uh oh!

openshift-ci bot commented Aug 20, 2024

Uh oh!

lihongyan1 commented Aug 20, 2024

Uh oh!

lihongyan1 commented Aug 20, 2024

Uh oh!

lihongyan1 commented Sep 2, 2024

Uh oh!

lihongyan1 commented Sep 11, 2024

Uh oh!

simonpasquier Sep 11, 2024

Choose a reason for hiding this comment

Uh oh!

simonpasquier Sep 11, 2024

Choose a reason for hiding this comment

Uh oh!

simonpasquier Sep 11, 2024

Choose a reason for hiding this comment

Uh oh!

lihongyan1 commented Sep 30, 2024

Uh oh!

jan--f commented Sep 30, 2024

Uh oh!

Uh oh!

Uh oh!

lihongyan1 commented Jul 17, 2024 •

edited by openshift-ci bot

Loading