📝 Parallelize the Config Policy controller E2E tests #145

yiraeChristineKim · 2023-06-21T22:16:02Z

The Config Policy controller's E2E tests in its repo takes roughly 45 minutes to run. This is due to all the test running serially. These could be optimized to run in parallel for faster execution.

Acceptance Criteria:

The config-policy-controller E2E tests run in under 20 minutes or Christine declares it impossible.

JustinKuli

Looks good overall! Just a few small questions. I'd also prefer to see a more descriptive commit message.

JustinKuli · 2023-06-22T14:23:08Z

Makefile

+ $(GINKGO) -v -p --fail-fast -procs=7 $(E2E_TEST_ARGS) test/e2e
+# 


Do we need to specify -procs here? From the docs, it sounds like it would pick an "ideal" number based on the machine it's running on. Since this same target will run in both the github action run and locally, I feel like it could be better to let it choose.

Also, is there an accidental empty comment here?

cuz our CI default is 2

I wonder limitation looks more procs faster also I didn't request a review because I am not finished yet so weird.. I cannot even remove you from the reviewer list. you are the default reviewer now

I wonder limitation looks more procs faster also I didn't request a review because I am not finished yet so weird.. I cannot even remove you from the reviewer list. you are the default reviewer now

FYI for next time, if you open a Draft PR, it doesn't get assigned reviewers but I think workflows still run. 🙂

oh!! Draft PR I will ask you next time Thanks for letting know good info!!! @dhaiducek

JustinKuli · 2023-06-22T14:28:21Z

test/resources/case28_evaluation_metric/case28_policy.yaml

@@ -11,7 +11,7 @@ spec:
 - complianceType: musthave
 objectDefinition:
 apiVersion: v1
- kind: ConfigMap
+ kind: case28-ConfigMap


What is this change for? It seems like the test just needs any policy (it doesn't care if it's compliant or not) but this seems odd.

good catch!!

yiraeChristineKim · 2023-06-22T17:46:23Z

test/e2e/case11_apiserver_config_test.go

@@ -24,8 +24,8 @@ const (
 tlsProfileInformYaml string = "../resources/case11_apiserver_config/tls_profile_inform.yaml"
 )

-var _ = Describe("Test APIServer Config policy", func() {
- Describe("Test etcd encryption and tls profile", func() {
+var _ = Describe("Test APIServer Config policy", Serial, func() {


This test is removing API so affect others

yiraeChristineKim · 2023-06-22T21:56:28Z

Makefile

@@ -205,7 +205,7 @@ install-resources:

 .PHONY: e2e-test
 e2e-test: e2e-dependencies
- $(GINKGO) -v --fail-fast $(E2E_TEST_ARGS) test/e2e
+ $(GINKGO) -v -p -procs=7 --fail-fast $(E2E_TEST_ARGS) test/e2e


CI default is 2 so I had to do this

yiraeChristineKim · 2023-06-22T21:58:36Z

test/e2e/case13_templatization_test.go

 deleteConfigPolicies([]string{case13Unterminated, case13WrongArgs})
 })
 })
 // Though the Bugzilla bug #2007575 references a different incorrect behavior, it's the same
 // underlying bug and this behavior is easier to test.
- Describe("RHBZ#2007575: Test that the template updates when a referenced resource object is updated", func() {
- const configMapName = "configmap-update-referenced-object"


I just added Describe around describe

dhaiducek

Thanks! This is going to be so great to not have to wait an hour for the E2E tests! I have some comments/questions.

.github/workflows/kind.yml

test/e2e/case13_templatization_test.go

test/e2e/case6_no_ns_test.go

test/e2e/case25_related_object_metric_test.go

test/utils/utils.go

test/e2e/case13_templatization_test.go

test/resources/case20_delete_objects/case20_change_config_policy.yaml

test/e2e/case20_delete_objects_test.go

test/resources/case20_delete_objects/case20_change_config_policy_not_prune.yaml

mprahl

Just one minor comment but otherwise, this looks great to me! Nice job.

mprahl · 2023-06-28T00:36:18Z

@yiraeChristineKim could you please squash the commits?

Signed-off-by: Yi Rae Kim <yikim@redhat.com>

openshift-ci · 2023-06-28T17:45:28Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: mprahl, yiraeChristineKim

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~~ [mprahl,yiraeChristineKim]

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

openshift-ci bot requested review from JustinKuli and mprahl June 21, 2023 22:16

openshift-ci bot added dco-signoff: yes approved labels Jun 21, 2023

yiraeChristineKim removed request for mprahl and JustinKuli June 21, 2023 22:16

JustinKuli reviewed Jun 22, 2023

View reviewed changes

JustinKuli mentioned this pull request Jun 22, 2023

Bug: ACM-5052, Policy gets shortly into non-compliant state #144

Merged

yiraeChristineKim commented Jun 22, 2023

View reviewed changes

yiraeChristineKim force-pushed the ACM-2542 branch from 21a8601 to e2e72bb Compare June 22, 2023 22:07

yiraeChristineKim requested review from dhaiducek, gparvin, mprahl and JustinKuli June 23, 2023 13:26

dhaiducek reviewed Jun 23, 2023

View reviewed changes

mprahl mentioned this pull request Jun 26, 2023

Refactor object template status generation #143

Merged

yiraeChristineKim force-pushed the ACM-2542 branch from d376659 to a205045 Compare June 26, 2023 14:09

mprahl reviewed Jun 26, 2023

View reviewed changes

test/e2e/case13_templatization_test.go Show resolved Hide resolved

mprahl reviewed Jun 26, 2023

View reviewed changes

test/resources/case20_delete_objects/case20_change_config_policy.yaml Outdated Show resolved Hide resolved

yiraeChristineKim requested review from mprahl and dhaiducek June 26, 2023 18:51

yiraeChristineKim force-pushed the ACM-2542 branch from 4c89fbf to 84a5b3e Compare June 26, 2023 19:00

mprahl reviewed Jun 27, 2023

View reviewed changes

test/e2e/case20_delete_objects_test.go Show resolved Hide resolved

mprahl reviewed Jun 27, 2023

View reviewed changes

test/resources/case20_delete_objects/case20_change_config_policy_not_prune.yaml Outdated Show resolved Hide resolved

mprahl reviewed Jun 27, 2023

View reviewed changes

openshift-merge-robot added the needs-rebase label Jun 27, 2023

yiraeChristineKim force-pushed the ACM-2542 branch from 8de8560 to c21f11e Compare June 27, 2023 14:43

openshift-merge-robot removed the needs-rebase label Jun 27, 2023

yiraeChristineKim force-pushed the ACM-2542 branch from c21f11e to 0b7955c Compare June 27, 2023 16:52

yiraeChristineKim requested a review from mprahl June 27, 2023 18:25

yiraeChristineKim force-pushed the ACM-2542 branch from 79b28d9 to df14335 Compare June 28, 2023 14:09

Parallelize the Config Policy controller E2E tests

5278607

Signed-off-by: Yi Rae Kim <yikim@redhat.com>

yiraeChristineKim force-pushed the ACM-2542 branch from df14335 to 5278607 Compare June 28, 2023 16:31

mprahl approved these changes Jun 28, 2023

View reviewed changes

openshift-ci bot assigned mprahl Jun 28, 2023

openshift-ci bot added the lgtm label Jun 28, 2023

openshift-merge-robot merged commit 60761a4 into open-cluster-management-io:main Jun 28, 2023
2 checks passed

magic-mirror-bot bot mentioned this pull request Jun 28, 2023

😿 Failed to sync the upstream PRs: #145 stolostron/config-policy-controller#527

Closed

yiraeChristineKim deleted the ACM-2542 branch February 29, 2024 20:05

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

📝 Parallelize the Config Policy controller E2E tests #145

📝 Parallelize the Config Policy controller E2E tests #145

yiraeChristineKim commented Jun 21, 2023

JustinKuli left a comment

JustinKuli Jun 22, 2023

yiraeChristineKim Jun 22, 2023

yiraeChristineKim Jun 22, 2023 •

edited

Loading

dhaiducek Jun 23, 2023

yiraeChristineKim Jun 23, 2023

JustinKuli Jun 22, 2023

yiraeChristineKim Jun 22, 2023

yiraeChristineKim Jun 22, 2023

yiraeChristineKim Jun 22, 2023

yiraeChristineKim Jun 22, 2023

dhaiducek left a comment

mprahl left a comment

mprahl commented Jun 28, 2023

openshift-ci bot commented Jun 28, 2023

		$(GINKGO) -v -p --fail-fast -procs=7 $(E2E_TEST_ARGS) test/e2e
		#

📝 Parallelize the Config Policy controller E2E tests #145

📝 Parallelize the Config Policy controller E2E tests #145

Conversation

yiraeChristineKim commented Jun 21, 2023

JustinKuli left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yiraeChristineKim Jun 22, 2023 • 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

dhaiducek left a comment

Choose a reason for hiding this comment

mprahl left a comment

Choose a reason for hiding this comment

mprahl commented Jun 28, 2023

openshift-ci bot commented Jun 28, 2023

yiraeChristineKim Jun 22, 2023 •

edited

Loading