MON-3934: Clean up and parallelize some e2e tests for faster runs #2397

machine424 · 2024-06-27T10:14:19Z

MON-3934: Parallelize some readonly/non disruptive e2e tests for faster runs

Isolate specific tests to enable parallel execution

Enhance the resilience of some tests and fix those prone to errors.

Fix some tests that were running wrong checks.

Make some the tests idempotent to be easily debugged and run locally

I added CHANGELOG entry for this change.
No user facing changes, so no entry in CHANGELOG was needed.

openshift-ci · 2024-06-27T10:16:34Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: machine424

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

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

test/e2e/framework/client_test.go

test/e2e/image_registry_test.go

openshift-ci-robot · 2024-07-10T16:59:51Z

@machine424: This pull request references MON-3934 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the task to target the "4.17.0" version, but no target version was set.

In response to this:

I added CHANGELOG entry for this change.

No user facing changes, so no entry in CHANGELOG was needed.

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 openshift-eng/jira-lifecycle-plugin repository.

machine424 · 2024-07-11T08:32:07Z

/retest

openshift-ci-robot · 2024-08-02T07:31:57Z

@machine424: This pull request references MON-3934 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the task to target the "4.17.0" version, but no target version was set.

In response to this:

I added CHANGELOG entry for this change.

No user facing changes, so no entry in CHANGELOG was needed.

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 openshift-eng/jira-lifecycle-plugin repository.

machine424 · 2024-08-02T10:22:03Z

test/e2e/alertmanager_test.go

@@ -747,51 +752,6 @@ func TestAlertmanagerDisabling(t *testing.T) {
 	})
 }

-func TestAlertManagerHasAdditionalAlertRelabelConfigs(t *testing.T) {


moved to TestMultinamespacePrometheusRule where we're sure to get an alert with the appropriate labels.

machine424 · 2024-08-02T10:22:31Z

test/e2e/alertmanager_user_workload_test.go

 	err = framework.Poll(5*time.Second, time.Minute, func() error {
+		// The uwm alerts port (9095) is only exposed in-cluster, so we need to use
+		// port forwarding to access kube-rbac-proxy.
+		host, cleanUp, err := f.ForwardPort(t, f.UserWorkloadMonitoringNs, "alertmanager-user-workload", 9095)


inside the poll for better resiliency

machine424 · 2024-08-02T10:23:35Z

test/e2e/image_registry_test.go

-			}
+func assertCMOImageRegistryIsUsed(t *testing.T, ns string) {
+	getRegistry := func(t *testing.T, image string) string {
+		// This first attempt is needed; otherwise, we may blindly add a second scheme,


make the test less error prone.

machine424 · 2024-08-02T10:24:35Z

test/e2e/thanos_ruler_test.go

-	t.Cleanup(cleanUp)
+func verifyAlertmanagerReceivedAlerts(t *testing.T, namespace, svc string) {
+	err := framework.Poll(time.Second, 5*time.Minute, func() error {
+		host, cleanUp, err := f.ForwardPort(t, namespace, svc, 9093)


inside the poll for better resiliency

machine424 · 2024-08-02T10:25:43Z

test/e2e/user_workload_monitoring_test.go

-	}
-}
-
-func TestUserWorkloadMonitoringAlerting(t *testing.T) {


merged them in TestUserWorkloadMonitoring to make use of the same setup and be able to run them in //

machine424 · 2024-08-02T10:26:47Z

test/e2e/user_workload_monitoring_test.go

-	}
-}
-
-func TestUserWorkloadMonitoringOptOut(t *testing.T) {


merged them in TestUserWorkloadMonitoring to make use of the same setup and be able to run them in //

This check will happen on a standalone namespace userWorkloadOptOutTestNs so it can run in //

machine424 · 2024-08-02T10:27:02Z

test/e2e/user_workload_monitoring_test.go

 	}
-}
-
-func TestUserWorkloadMonitoringGrpcSecrets(t *testing.T) {


merged them in TestUserWorkloadMonitoring to make use of the same setup and be able to run them in //

machine424 · 2024-08-02T10:28:06Z

test/e2e/user_workload_monitoring_test.go

@@ -436,6 +380,7 @@ func assertUserWorkloadMetrics(t *testing.T) {
 	err = framework.Poll(5*time.Second, 5*time.Minute, func() error {
 		body, err := f.AlertmanagerClient.GetAlertmanagerAlerts(
 			"filter", `alertname="VersionAlert"`,
+			"filter", fmt.Sprintf(`namespace="%s"`, userWorkloadTestNs),


we now create tow namespaces, see setupUserApplications
This also make the test better isolated and less error prone

machine424 · 2024-08-02T10:29:30Z

test/e2e/user_workload_monitoring_test.go

 			f.ThanosQuerierClient.WaitForQueryReturn(
-				t, time.Minute, fmt.Sprintf(`count(up{service="%s",namespace="openshift-user-workload-monitoring"} == 1)`, service),
+				t, 5*time.Minute, fmt.Sprintf(`count(up{service="%s",namespace="openshift-user-workload-monitoring"} == 1)`, service),


This runs in parallel now, some checks that used to run at the end may need more time now.

…er runs Isolate specific tests to enable parallel execution Enhance the resilience of some tests and fix those prone to errors.

openshift-ci-robot · 2024-08-02T10:43:02Z

@machine424: This pull request references MON-3934 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the task to target the "4.17.0" version, but no target version was set.

In response to this:

MON-3934: Parallelize some readonly/non disruptive e2e tests for faster runs

Isolate specific tests to enable parallel execution

Enhance the resilience of some tests and fix those prone to errors.

I added CHANGELOG entry for this change.

No user facing changes, so no entry in CHANGELOG was needed.

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 openshift-eng/jira-lifecycle-plugin repository.

machine424 · 2024-08-02T10:43:49Z

pkg/client/client.go

@@ -1488,7 +1488,7 @@ func (c *Client) CreateOrUpdateConfigMap(ctx context.Context, cm *v1.ConfigMap)
 	return err
 }

-func (c *Client) DeleteIfExists(ctx context.Context, nsName string) error {
+func (c *Client) DeleteNSIfExists(ctx context.Context, nsName string) error {


Yes, this is not related to the PR :)

(nit) the "IfExists" is implicit IMHO. Also the function could be moved to the framework package since it's not invoked by the operator.

Suggested change

func (c *Client) DeleteNSIfExists(ctx context.Context, nsName string) error {

func (c *Client) DeleteNamespace(ctx context.Context, nsName string) error {

openshift-ci-robot · 2024-08-02T10:48:57Z

@machine424: This pull request references MON-3934 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the task to target the "4.17.0" version, but no target version was set.

In response to this:

MON-3934: Parallelize some readonly/non disruptive e2e tests for faster runs

Isolate specific tests to enable parallel execution

Enhance the resilience of some tests and fix those prone to errors.

Fix some tests that were running wrong checks.

Make some the tests idempotent to be easily debugged and run locally

I added CHANGELOG entry for this change.

No user facing changes, so no entry in CHANGELOG was needed.

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 openshift-eng/jira-lifecycle-plugin repository.

machine424 · 2024-08-02T10:55:11Z

test/e2e/thanos_ruler_test.go

@@ -109,15 +119,14 @@ func createPrometheusRule(t *testing.T) {
 	}
 }

-func verifyAlertmanagerAlertReceived(t *testing.T) {
-
-	host, cleanUp, err := f.ForwardPort(t, f.Ns, "alertmanager-operated", 9093)


This was requesting the default AM instead of the additional one.

openshift-ci · 2024-08-02T13:05:01Z

@machine424: The following test 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-aws-ovn-single-node	`a1cd4c2`	link	false	`/test e2e-aws-ovn-single-node`

Full PR test history. Your PR dashboard.

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-sigs/prow repository. I understand the commands that are listed here.

simonpasquier · 2024-08-05T08:22:54Z

pkg/client/client.go

@@ -1488,7 +1488,7 @@ func (c *Client) CreateOrUpdateConfigMap(ctx context.Context, cm *v1.ConfigMap)
 	return err
 }

-func (c *Client) DeleteIfExists(ctx context.Context, nsName string) error {
+func (c *Client) DeleteNSIfExists(ctx context.Context, nsName string) error {


(nit) the "IfExists" is implicit IMHO. Also the function could be moved to the framework package since it's not invoked by the operator.

Suggested change

func (c *Client) DeleteNSIfExists(ctx context.Context, nsName string) error {

func (c *Client) DeleteNamespace(ctx context.Context, nsName string) error {

simonpasquier · 2024-08-05T08:29:44Z

test/e2e/image_registry_test.go

I'd keep this change for another PR if you don't mind.

simonpasquier · 2024-08-05T08:33:49Z

test/e2e/thanos_ruler_test.go

+		if err != nil {
+			return err
+		}
+		t.Cleanup(cleanUp)


It might leave many port forwards opened if there are retries?

simonpasquier · 2024-08-05T09:12:10Z

test/e2e/user_workload_monitoring_test.go

The change here is huge. Can you split it to another PR?

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 Jun 27, 2024

openshift-ci bot requested review from simonpasquier and slashpai June 27, 2024 10:16

openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jun 27, 2024

machine424 changed the title ~~WIP (REVIEW NOT NEEDED): Parallel e2e~~ WIP (REVIEW NOT NEEDED): make some e2e tests run in parallel to save some time Jun 27, 2024

machine424 commented Jun 27, 2024

View reviewed changes

test/e2e/framework/client_test.go Outdated Show resolved Hide resolved

machine424 commented Jun 27, 2024

View reviewed changes

test/e2e/image_registry_test.go Outdated Show resolved Hide resolved

openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 1, 2024

machine424 force-pushed the parallel-e2e branch from 8959b01 to d02dd8b Compare July 10, 2024 16:57

openshift-merge-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 10, 2024

machine424 force-pushed the parallel-e2e branch from d02dd8b to 06843e0 Compare July 10, 2024 16:59

machine424 changed the title ~~WIP (REVIEW NOT NEEDED): make some e2e tests run in parallel to save some time~~ MON-3934: Parallelize some readonly/non disruptive e2e tests for faster runs Jul 10, 2024

openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Jul 10, 2024

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 Jul 10, 2024

machine424 force-pushed the parallel-e2e branch from 06843e0 to ed679ce Compare July 10, 2024 19:54

machine424 changed the title ~~MON-3934: Parallelize some readonly/non disruptive e2e tests for faster runs~~ MON-3934: Clean up and parallelize some e2e tests for faster runs Jul 12, 2024

machine424 force-pushed the parallel-e2e branch 4 times, most recently from 3871d3d to f010c91 Compare August 2, 2024 07:28

openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 2, 2024

machine424 force-pushed the parallel-e2e branch from f010c91 to 0355cf1 Compare August 2, 2024 07:31

openshift-merge-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 2, 2024

machine424 commented Aug 2, 2024

View reviewed changes

MON-3934: Parallelize some readonly/non disruptive e2e tests for fast…

a1cd4c2

…er runs Isolate specific tests to enable parallel execution Enhance the resilience of some tests and fix those prone to errors.

machine424 force-pushed the parallel-e2e branch from 0355cf1 to a1cd4c2 Compare August 2, 2024 10:42

machine424 commented Aug 2, 2024

View reviewed changes

simonpasquier reviewed Aug 5, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

MON-3934: Clean up and parallelize some e2e tests for faster runs #2397

MON-3934: Clean up and parallelize some e2e tests for faster runs #2397

machine424 commented Jun 27, 2024 •

edited

Loading

openshift-ci bot commented Jun 27, 2024

openshift-ci-robot commented Jul 10, 2024 •

edited by openshift-ci bot

Loading

machine424 commented Jul 11, 2024

openshift-ci-robot commented Aug 2, 2024 •

edited by openshift-ci bot

Loading

machine424 Aug 2, 2024

machine424 Aug 2, 2024 •

edited

Loading

machine424 Aug 2, 2024

machine424 Aug 2, 2024

machine424 Aug 2, 2024

machine424 Aug 2, 2024

machine424 Aug 2, 2024

machine424 Aug 2, 2024

machine424 Aug 2, 2024

openshift-ci-robot commented Aug 2, 2024 •

edited by openshift-ci bot

Loading

machine424 Aug 2, 2024

simonpasquier Aug 5, 2024

openshift-ci-robot commented Aug 2, 2024 •

edited by openshift-ci bot

Loading

machine424 Aug 2, 2024

openshift-ci bot commented Aug 2, 2024

simonpasquier Aug 5, 2024

simonpasquier Aug 5, 2024

simonpasquier Aug 5, 2024

simonpasquier Aug 5, 2024

	func (c *Client) DeleteNSIfExists(ctx context.Context, nsName string) error {
	func (c *Client) DeleteNamespace(ctx context.Context, nsName string) error {

MON-3934: Clean up and parallelize some e2e tests for faster runs #2397

Are you sure you want to change the base?

MON-3934: Clean up and parallelize some e2e tests for faster runs #2397

Conversation

machine424 commented Jun 27, 2024 • edited Loading

openshift-ci bot commented Jun 27, 2024

openshift-ci-robot commented Jul 10, 2024 • edited by openshift-ci bot Loading

machine424 commented Jul 11, 2024

openshift-ci-robot commented Aug 2, 2024 • edited by openshift-ci bot Loading

Choose a reason for hiding this comment

machine424 Aug 2, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

openshift-ci-robot commented Aug 2, 2024 • edited by openshift-ci bot Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

openshift-ci-robot commented Aug 2, 2024 • edited by openshift-ci bot Loading

Choose a reason for hiding this comment

openshift-ci bot commented Aug 2, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

machine424 commented Jun 27, 2024 •

edited

Loading

openshift-ci-robot commented Jul 10, 2024 •

edited by openshift-ci bot

Loading

openshift-ci-robot commented Aug 2, 2024 •

edited by openshift-ci bot

Loading

machine424 Aug 2, 2024 •

edited

Loading

openshift-ci-robot commented Aug 2, 2024 •

edited by openshift-ci bot

Loading

openshift-ci-robot commented Aug 2, 2024 •

edited by openshift-ci bot

Loading