Connectivity test factory component. #2322

viktor-kurchenko · 2024-02-21T10:34:56Z

Introducing connectivity tests factory component.

The PR goals are:

Extract connectivity test definitions in separate files.
Implement a factory component that is responsible for keeping test creation logic and order.
The first refactoring step related to the CFP: CFP: Cilium CLI connectivity tests speedup. design-cfps#15

connectivity/factory/factory.go

tklauser

Could you please add entries to CODEOWNERS for the newly created files in connectivity/facrtory where applicable? Where the owner is not obvious, keeping @cilium/ci-structure is probably fine for now, but some tests are quite feature-specific so the relevant team should probably be in the loop for any future changes.

viktor-kurchenko · 2024-02-23T14:48:00Z

Could you please add entries to CODEOWNERS for the newly created files in connectivity/facrtory where applicable? Where the owner is not obvious, keeping @cilium/ci-structure is probably fine for now, but some tests are quite feature-specific so the relevant team should probably be in the loop for any future changes.

Done, thanks!

CODEOWNERS

michi-covalent · 2024-02-26T20:24:34Z

i'll review this today

michi-covalent

💯 for splitting tests into separate files 🥰

at a very high level we need to think about how to express the order of tests. it will be difficult to maintain if we have a single primaryTests. we also need to think about tests being added from outside using the hook interface.

... stepping back even further, it doesn't have to be fixed in this PR but why does the order matter?

CODEOWNERS

connectivity/check/test.go

connectivity/factory/factory.go

michi-covalent

thanks viktor it looks much better now. i have a couple of minor nits but i'm approving it now. feel free to merge it once these nits get addressed 🚀🙏

CODEOWNERS

connectivity/check/test.go

connectivity/factory/factory.go

tklauser

Code changes LGTM.

Some suggestions for CODEOWNERS inline. Please also squash your commits into a single one.

tklauser · 2024-03-05T13:21:11Z

CODEOWNERS

+/connectivity/builder/egress_gateway.go @cilium/sig-clustermesh
+/connectivity/builder/egress_gateway_excluded_cidrs.go @cilium/sig-clustermesh


These should probably be owned by @cilium/egress-gateway.

Fixed, thanks!

tklauser · 2024-03-05T13:22:18Z

CODEOWNERS

+/connectivity/builder/health.go @cilium/sig-clustermesh
+/connectivity/builder/host_entity_egress.go @cilium/sig-clustermesh
+/connectivity/builder/host_entity_ingress.go @cilium/sig-clustermesh


Not sure why these are assigned to @cilium/sig-clustermesh. The features they're testing don't seem related. Probably best to drop these entries and let @cilium/ci-structure take care of them for now.

I just deleted them because @cilium/ci-structure is the default for the package.

tklauser · 2024-03-05T13:22:41Z

CODEOWNERS

+/connectivity/builder/echo_ingress.go @cilium/sig-clustermesh
+/connectivity/builder/echo_ingress_auth_always_fail.go @cilium/sig-clustermesh
+/connectivity/builder/echo_ingress_from_other_client_deny.go @cilium/sig-clustermesh
+/connectivity/builder/echo_ingress_from_outside.go @cilium/sig-clustermesh
+/connectivity/builder/echo_ingress_knp.go @cilium/sig-clustermesh
+/connectivity/builder/echo_ingress_l7.go @cilium/sig-clustermesh
+/connectivity/builder/echo_ingress_l7_named_port.go @cilium/sig-clustermesh
+/connectivity/builder/echo_ingress_mutual_auth_spiffe.go @cilium/sig-encryption


Based on the features these are testing, I think they should be assigned to @cilium/sig-servicemesh instead.

Signed-off-by: viktor-kurchenko <[email protected]>

viktor-kurchenko · 2024-03-05T14:53:50Z

Some suggestions for CODEOWNERS inline. Please also squash your commits into a single one.

Commits squashed, thank you!

tklauser

LGTM, thanks Viktor!

viktor-kurchenko temporarily deployed to ci February 21, 2024 10:35 — with GitHub Actions Inactive

viktor-kurchenko force-pushed the pr/vk/connectivity/test/factory branch from 1e7938f to afb1a8a Compare February 21, 2024 10:48

viktor-kurchenko temporarily deployed to ci February 21, 2024 10:49 — with GitHub Actions Inactive

nickolaev reviewed Feb 21, 2024

View reviewed changes

connectivity/factory/factory.go Outdated Show resolved Hide resolved

viktor-kurchenko marked this pull request as ready for review February 21, 2024 10:58

viktor-kurchenko requested a review from a team as a code owner February 21, 2024 10:58

viktor-kurchenko requested a review from tklauser February 21, 2024 10:58

viktor-kurchenko marked this pull request as draft February 21, 2024 11:59

viktor-kurchenko force-pushed the pr/vk/connectivity/test/factory branch from afb1a8a to 2b90c15 Compare February 21, 2024 12:48

viktor-kurchenko temporarily deployed to ci February 21, 2024 12:48 — with GitHub Actions Inactive

viktor-kurchenko force-pushed the pr/vk/connectivity/test/factory branch from 2b90c15 to 090b36e Compare February 21, 2024 13:04

viktor-kurchenko temporarily deployed to ci February 21, 2024 13:05 — with GitHub Actions Inactive

viktor-kurchenko temporarily deployed to ci February 21, 2024 15:08 — with GitHub Actions Inactive

viktor-kurchenko marked this pull request as ready for review February 21, 2024 15:08

viktor-kurchenko requested review from asauber, brlbil, michi-covalent and nickolaev February 21, 2024 15:08

tklauser requested changes Feb 23, 2024

View reviewed changes

tklauser added the needs-rebase This PR needs to be rebased because it has merge conflicts. label Feb 23, 2024

viktor-kurchenko force-pushed the pr/vk/connectivity/test/factory branch from f534f32 to a5ace62 Compare February 23, 2024 14:47

viktor-kurchenko requested a review from a team as a code owner February 23, 2024 14:47

viktor-kurchenko temporarily deployed to ci February 23, 2024 14:47 — with GitHub Actions Inactive

viktor-kurchenko requested a review from tklauser February 23, 2024 14:48

nickolaev reviewed Feb 23, 2024

View reviewed changes

CODEOWNERS Outdated Show resolved Hide resolved

viktor-kurchenko force-pushed the pr/vk/connectivity/test/factory branch from a5ace62 to 0b46aa4 Compare February 23, 2024 17:03

viktor-kurchenko temporarily deployed to ci February 23, 2024 17:03 — with GitHub Actions Inactive

tklauser removed the needs-rebase This PR needs to be rebased because it has merge conflicts. label Feb 23, 2024

viktor-kurchenko force-pushed the pr/vk/connectivity/test/factory branch from 0b46aa4 to f5402d6 Compare February 25, 2024 12:52

viktor-kurchenko requested a review from asauber February 26, 2024 19:00

asauber approved these changes Feb 26, 2024

View reviewed changes

michi-covalent added the dont-merge/waiting-for-review label Feb 26, 2024

michi-covalent requested changes Feb 27, 2024

View reviewed changes

michi-covalent removed the dont-merge/waiting-for-review label Feb 27, 2024

viktor-kurchenko temporarily deployed to ci February 27, 2024 13:57 — with GitHub Actions Inactive

viktor-kurchenko requested a review from michi-covalent February 29, 2024 16:45

viktor-kurchenko temporarily deployed to ci March 2, 2024 10:44 — with GitHub Actions Inactive

viktor-kurchenko temporarily deployed to ci March 2, 2024 11:31 — with GitHub Actions Inactive

viktor-kurchenko temporarily deployed to ci March 2, 2024 12:20 — with GitHub Actions Inactive

michi-covalent approved these changes Mar 4, 2024

View reviewed changes

CODEOWNERS Outdated Show resolved Hide resolved

connectivity/check/test.go Show resolved Hide resolved

connectivity/check/test.go Outdated Show resolved Hide resolved

connectivity/check/test.go Show resolved Hide resolved

connectivity/factory/factory.go Outdated Show resolved Hide resolved

viktor-kurchenko removed the request for review from brlbil March 5, 2024 06:48

viktor-kurchenko force-pushed the pr/vk/connectivity/test/factory branch from 3d92508 to 620e130 Compare March 5, 2024 07:13

viktor-kurchenko temporarily deployed to ci March 5, 2024 07:13 — with GitHub Actions Inactive

nickolaev approved these changes Mar 5, 2024

View reviewed changes

viktor-kurchenko force-pushed the pr/vk/connectivity/test/factory branch from 620e130 to a3d9aab Compare March 5, 2024 08:43

viktor-kurchenko temporarily deployed to ci March 5, 2024 08:43 — with GitHub Actions Inactive

tklauser requested changes Mar 5, 2024

View reviewed changes

Connectivity test builder component implemented.

e11fc05

Signed-off-by: viktor-kurchenko <[email protected]>

viktor-kurchenko force-pushed the pr/vk/connectivity/test/factory branch from a3d9aab to e11fc05 Compare March 5, 2024 14:52

viktor-kurchenko temporarily deployed to ci March 5, 2024 14:52 — with GitHub Actions Inactive

viktor-kurchenko requested a review from tklauser March 5, 2024 14:53

tklauser approved these changes Mar 5, 2024

View reviewed changes

maintainer-s-little-helper bot added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Mar 5, 2024

tklauser merged commit 486eb99 into main Mar 5, 2024
13 checks passed

tklauser deleted the pr/vk/connectivity/test/factory branch March 5, 2024 16:19

tklauser mentioned this pull request Mar 6, 2024

add message send size to perf testing #2314

Closed

viktor-kurchenko mentioned this pull request Apr 2, 2024

connectivity: introduce host firewall tests #2464

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Connectivity test factory component. #2322

Connectivity test factory component. #2322

viktor-kurchenko commented Feb 21, 2024 •

edited

Loading

tklauser left a comment

viktor-kurchenko commented Feb 23, 2024

michi-covalent commented Feb 26, 2024

michi-covalent left a comment

michi-covalent left a comment

tklauser left a comment

tklauser Mar 5, 2024

viktor-kurchenko Mar 5, 2024

tklauser Mar 5, 2024

viktor-kurchenko Mar 5, 2024

tklauser Mar 5, 2024

viktor-kurchenko Mar 5, 2024

viktor-kurchenko commented Mar 5, 2024

tklauser left a comment

		/connectivity/builder/egress_gateway.go @cilium/sig-clustermesh
		/connectivity/builder/egress_gateway_excluded_cidrs.go @cilium/sig-clustermesh

Connectivity test factory component. #2322

Connectivity test factory component. #2322

Conversation

viktor-kurchenko commented Feb 21, 2024 • edited Loading

tklauser left a comment

Choose a reason for hiding this comment

viktor-kurchenko commented Feb 23, 2024

michi-covalent commented Feb 26, 2024

michi-covalent left a comment

Choose a reason for hiding this comment

michi-covalent left a comment

Choose a reason for hiding this comment

tklauser left a comment

Choose a reason for hiding this comment

tklauser Mar 5, 2024

Choose a reason for hiding this comment

viktor-kurchenko Mar 5, 2024

Choose a reason for hiding this comment

tklauser Mar 5, 2024

Choose a reason for hiding this comment

viktor-kurchenko Mar 5, 2024

Choose a reason for hiding this comment

tklauser Mar 5, 2024

Choose a reason for hiding this comment

viktor-kurchenko Mar 5, 2024

Choose a reason for hiding this comment

viktor-kurchenko commented Mar 5, 2024

tklauser left a comment

Choose a reason for hiding this comment

viktor-kurchenko commented Feb 21, 2024 •

edited

Loading