add e2e testing. #121

morvencao · 2024-06-11T11:21:08Z

Add spec resync testing for agent restart
Add status resync testing for server restart
Add gRPC API testing
Enable MTLS auth for MQTT broker.
Add resync test after reconnecting for Maestro agent.

morvencao · 2024-06-13T15:20:22Z

clyang82 · 2024-06-17T04:02:05Z

test/e2e/setup/e2e_setup.sh

+    tar -xzvf step_0.26.2_amd64.tar.gz
+    chmod +x ./step_0.26.2/bin/step
+    sudo mv ./step_0.26.2/bin/step /usr/local/bin/step
+    rm -rf ./step_0.26.2_amd64.tar.gz ./step_0.26.2


It is worth extracting a version variable to define 0.26.2.

clyang82 · 2024-06-17T06:38:49Z

test/e2e/setup/e2e_setup.sh

+
+# create secret containing the client certs to mqtt broker and patch the maestro deployment
+kubectl create secret generic maestro-server-certs -n $namespace --from-file=ca.crt=${certDir}/ca.crt --from-file=client.crt=${certDir}/server-client.crt --from-file=client.key=${certDir}/server-client.key
+kubectl create secret generic maestro-server-invalid-certs -n $namespace --from-file=ca.crt=${certDir}/ca.crt --from-file=client.crt=${certDir}/invalid-client.crt --from-file=client.key=${certDir}/invalid-client.key


why do you need maestro-server-invalid-certs?

This certificate will be expired in 1 minute, it is used to replace the client certificate, to simulate the case:

the MQTT client certificate is expired, the connection to broker will be broken

then we replace the expired certificate with the valid certificate, after that the maestro agent should be able to reload the valid certificate and then resync the resources during disconnected time.

This is mainly used to test:

agent reload certificate.

agent resync spec after reconnect to broker.

as we discussed, removed this for now.

clyang82 · 2024-06-17T06:41:39Z

cmd/maestro/agent/cmd.go

@@ -70,4 +77,6 @@ func addFlags(fs *pflag.FlagSet) {
 		commonOptions.SpokeClusterName, "Name of the consumer")
 	fs.BoolVar(&commonOptions.CommoOpts.CmdConfig.DisableLeaderElection, "disable-leader-election",
 		true, "Disable leader election.")
+	fs.DurationVar(&certRefreshDuration, "cert-refresh-duration",
+		certRefreshDuration, "Client certificate refresh duration for MQTT broker.")


add default value 5 minutes in the message.

looks like cobra flag will add this, if you try --help, you will see:

# go run ./cmd/maestro/main.go agent --help Start the Maestro Agent Usage: maestro agent [flags] Flags: --add-dir-header If true, adds the file directory to the header of the log messages --alsologtostderr log to standard error as well as files (no effect when -logtostderr=true) --appliedmanifestwork-eviction-grace-period duration Grace period for appliedmanifestwork eviction (default 1h0m0s) --cert-refresh-duration duration Client certificate refresh duration for MQTT broker. (default 5m0s)

clyang82 · 2024-06-17T06:57:02Z

test/e2e/pkg/certificate_test.go

+				}
+
+				return fmt.Errorf("maestro-agent logs does not have 'expired certificate'")
+			}, 3*time.Minute, 3*time.Second).ShouldNot(HaveOccurred())


will not refresh the certificates if they are expired?

you mean deploying a cert-manager for certificate rotation?

no. i mean the current behaviour. we do not refresh the certificate. correct?

we don't refresh the certificate, only replace it.

as we discussed, removed the certificate test for now.

clyang82 · 2024-06-17T07:08:32Z

test/e2e/pkg/resources_test.go

@@ -175,13 +193,13 @@ var _ = Describe("Resources", Ordered, Label("e2e-tests-resources"), func() {
 			Consistently(func() error {


Here is using Consistently. Consistently blocks when called for a specified duration. During that duration Consistently repeatedly polls its matcher and ensures that it is satisfied. If the matcher is consistently satisfied, then Consistently will pass. Otherwise Consistently will fail.

yes, I updated the Consistently check to avoid intermittent error to break the check when getting nginx deployment.

clyang82 · 2024-06-17T07:30:49Z

test/e2e/pkg/spec_resync_test.go

+			}, 30*time.Second, 2*time.Second).ShouldNot(HaveOccurred())
+		})
+
+		It("start maestro agent", func() {


why do you need to shutdown maestro agent repeatedly? to trigger re-sync?

if so, how about use Consistently?

Updated this to only restart agent once, to resync spec for created resource, updated resource and deleted resource.

Signed-off-by: morvencao <[email protected]>

morvencao · 2024-06-20T03:51:05Z

@clyang82 Another look?

clyang82 · 2024-06-20T08:57:20Z

/ok-to-test

clyang82 · 2024-06-20T09:10:55Z

Status resync Resource resync resource status after maestro server reconnects ensure the resource status is resynced [e2e-tests-status-resync]
/home/runner/work/maestro/maestro/test/e2e/pkg/status_resync_test.go:336
• [117.135 seconds]

Resources Resource CreateOnly UpdateStrategy Tests patch the nginx resource [e2e-tests-resources]
/home/runner/work/maestro/maestro/test/e2e/pkg/resources_test.go:184
• [30.017 seconds]

Spec resync Resource resync resource spec after maestro agent restarts ensure the nginx-1 resource is not updated [e2e-tests-spec-resync]
/home/runner/work/maestro/maestro/test/e2e/pkg/spec_resync_test.go:108
• [30.001 seconds]

Spec resync Resource resync resource spec after maestro agent restarts ensure the nginx-2 resource is not deleted [e2e-tests-spec-resync]
/home/runner/work/maestro/maestro/test/e2e/pkg/spec_resync_test.go:130
• [30.001 seconds]

Spec resync Resource resync resource spec after maestro agent restarts ensure the nginx-3 resource is not created [e2e-tests-spec-resync]
/home/runner/work/maestro/maestro/test/e2e/pkg/spec_resync_test.go:155
• [30.001 seconds]

Spec resync Resource resync resource spec after maestro agent reconnects ensure the nginx-1 resource is not updated [e2e-tests-spec-resync]
/home/runner/work/maestro/maestro/test/e2e/pkg/spec_resync_test.go:402
• [30.001 seconds]
------------------------------
Spec resync Resource resync resource spec after maestro agent reconnects ensure the nginx-2 resource is not deleted [e2e-tests-spec-resync]
/home/runner/work/maestro/maestro/test/e2e/pkg/spec_resync_test.go:424
• [30.000 seconds]
------------------------------
Spec resync Resource resync resource spec after maestro agent reconnects ensure the nginx-3 resource is not created [e2e-tests-spec-resync]
/home/runner/work/maestro/maestro/test/e2e/pkg/spec_resync_test.go:449
• [30.001 seconds]

I listed some cases that take a longer time to complete. Is this desired? Can the time be reduced?

Signed-off-by: morvencao <[email protected]>

clyang82

LGTM

morvencao force-pushed the br_e2e_test branch 6 times, most recently from 3a1e6e0 to f70ca5d Compare June 13, 2024 15:08

morvencao force-pushed the br_e2e_test branch 5 times, most recently from 9faf013 to 8ebc1d1 Compare June 15, 2024 05:00

clyang82 reviewed Jun 17, 2024

View reviewed changes

morvencao force-pushed the br_e2e_test branch 4 times, most recently from 6ecf1a7 to a46b394 Compare June 17, 2024 05:32

clyang82 reviewed Jun 17, 2024

View reviewed changes

morvencao force-pushed the br_e2e_test branch 5 times, most recently from 75c704a to 768daa5 Compare June 19, 2024 02:17

morvencao added 3 commits June 19, 2024 02:19

add e2e testing.

b7eb3aa

Signed-off-by: morvencao <[email protected]>

enable mqtt auth for e2e testing.

84afef3

Signed-off-by: morvencao <[email protected]>

remove certificate rotation test.

521ff28

Signed-off-by: morvencao <[email protected]>

morvencao force-pushed the br_e2e_test branch 4 times, most recently from 8f38127 to d48b2a6 Compare June 19, 2024 09:49

morvencao force-pushed the br_e2e_test branch 3 times, most recently from e3339e1 to fc3620a Compare June 20, 2024 03:34

morvencao mentioned this pull request Jun 20, 2024

Add e2e test for broadcast subscription type of resource status update #137

Merged

morvencao force-pushed the br_e2e_test branch 5 times, most recently from eec9217 to 1cfc614 Compare June 20, 2024 10:55

add resync test after reconnecting.

7a343fa

Signed-off-by: morvencao <[email protected]>

morvencao force-pushed the br_e2e_test branch from 1cfc614 to 7a343fa Compare June 20, 2024 11:08

clyang82 approved these changes Jun 20, 2024

View reviewed changes

clyang82 merged commit 85524b1 into openshift-online:main Jun 20, 2024
5 checks passed

morvencao deleted the br_e2e_test branch June 20, 2024 13:13

morvencao mentioned this pull request Jun 21, 2024

fix maestro server resync after reconnecting. #139

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add e2e testing. #121

add e2e testing. #121

morvencao commented Jun 11, 2024 •

edited

Loading

morvencao commented Jun 13, 2024 •

edited

Loading

clyang82 Jun 17, 2024

morvencao Jun 17, 2024

clyang82 Jun 17, 2024

morvencao Jun 17, 2024

morvencao Jun 17, 2024

morvencao Jun 17, 2024

clyang82 Jun 17, 2024

morvencao Jun 17, 2024

clyang82 Jun 17, 2024

morvencao Jun 17, 2024

clyang82 Jun 17, 2024

morvencao Jun 17, 2024

morvencao Jun 17, 2024

clyang82 Jun 17, 2024

morvencao Jun 17, 2024

clyang82 Jun 17, 2024

clyang82 Jun 17, 2024

morvencao Jun 17, 2024 •

edited

Loading

morvencao commented Jun 20, 2024 •

edited

Loading

clyang82 commented Jun 20, 2024

clyang82 commented Jun 20, 2024

clyang82 left a comment

		@@ -175,13 +193,13 @@ var _ = Describe("Resources", Ordered, Label("e2e-tests-resources"), func() {
		Consistently(func() error {

add e2e testing. #121

add e2e testing. #121

Conversation

morvencao commented Jun 11, 2024 • edited Loading

morvencao commented Jun 13, 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

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

Choose a reason for hiding this comment

morvencao Jun 17, 2024 • edited Loading

Choose a reason for hiding this comment

morvencao commented Jun 20, 2024 • edited Loading

clyang82 commented Jun 20, 2024

clyang82 commented Jun 20, 2024

clyang82 left a comment

Choose a reason for hiding this comment

morvencao commented Jun 11, 2024 •

edited

Loading

morvencao commented Jun 13, 2024 •

edited

Loading

morvencao Jun 17, 2024 •

edited

Loading

morvencao commented Jun 20, 2024 •

edited

Loading