Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

egressgw: use Resource[T] to consume CiliumEgressGatewayPolicy #26960

Merged
merged 3 commits into from Aug 2, 2023

Conversation

lmb
Copy link
Contributor

@lmb lmb commented Jul 20, 2023

egressgw: manually construct Manager in tests

Don't rely on hive to construct a Manager, it's more trouble than it's
worth. Instead create the necessary dependencies manually.

This gives us much more control over which types are used for certain
interfaces without having to fiddle with hive rules.

Signed-off-by: Lorenz Bauer <lmb@isovalent.com>

egressgw: use tb.Cleanup to tear down dummy interfaces

The testsuite leaves around dummy interfaces if the test aborts for some
reason. Use tb.Cleanup to avoid this behaviour.

Signed-off-by: Lorenz Bauer <lmb@isovalent.com>

egressgw: use Resource[T] to consume CiliumEgressGatewayPolicy

Replace the copy pasta'd k8s watcher for CiliumEgressGatewayPolicy with 
resource.Resource.

Signed-off-by: Lorenz Bauer <lmb@isovalent.com>

@lmb lmb added release-note/misc This PR makes changes that have no direct user impact. feature/egress-gateway Impacts the egress IP gateway feature. area/modularization labels Jul 20, 2023
@lmb lmb changed the title Egw cegp resource egressgw: use Resource[T] to consume CiliumEgressGatewayPolicy Jul 20, 2023
@lmb
Copy link
Contributor Author

lmb commented Jul 20, 2023

/test

@lmb lmb force-pushed the egw-cegp-resource branch 2 times, most recently from 7bf967e to 38b1d6e Compare July 24, 2023 11:59
@lmb
Copy link
Contributor Author

lmb commented Jul 24, 2023

/test-runtime

@lmb
Copy link
Contributor Author

lmb commented Jul 24, 2023

/privileged

@lmb lmb self-assigned this Jul 24, 2023
@lmb
Copy link
Contributor Author

lmb commented Jul 24, 2023

/test

@lmb lmb force-pushed the egw-cegp-resource branch 2 times, most recently from 3512bd7 to 1c7b154 Compare July 24, 2023 14:03
@lmb
Copy link
Contributor Author

lmb commented Jul 25, 2023

/test

@lmb lmb marked this pull request as ready for review July 25, 2023 14:14
@lmb lmb requested review from a team as code owners July 25, 2023 14:14
@lmb lmb requested review from ldelossa, youngnick and jibi July 25, 2023 14:14
@lmb lmb force-pushed the egw-cegp-resource branch 2 times, most recently from da30942 to 8095e86 Compare July 26, 2023 12:56
@lmb
Copy link
Contributor Author

lmb commented Jul 26, 2023

/test

@lmb
Copy link
Contributor Author

lmb commented Jul 27, 2023

/ci-e2e

@lmb
Copy link
Contributor Author

lmb commented Jul 27, 2023

/ci-e2e

@lmb
Copy link
Contributor Author

lmb commented Jul 31, 2023

/ci-e2e

@lmb
Copy link
Contributor Author

lmb commented Jul 31, 2023

/ci-e2e

@lmb
Copy link
Contributor Author

lmb commented Jul 31, 2023

/ci-e2e

@lmb
Copy link
Contributor Author

lmb commented Jul 31, 2023

/test

pkg/egressgateway/manager_privileged_test.go Outdated Show resolved Hide resolved
}
}

func (manager *Manager) handlePolicyEvent(event resource.Event[*Policy]) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: this looks like it could just be merged into the above function.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I split it out since processEvents is going to get more complicated as we migrate away from the monolithic k8s watchers. AFAIK we'll add at least Endpoint and Node. So I'd prefer to keep them split.

lmb added 3 commits August 1, 2023 10:08
Don't rely on hive to construct a Manager, it's more trouble than
it's worth. Instead create the necessary dependencies manually.

This gives us much more control over which types are used for
certain interfaces without having to fiddle with hive rules.

Signed-off-by: Lorenz Bauer <lmb@isovalent.com>
The testsuite leaves around dummy interfaces if the test aborts
for some reason. Use tb.Cleanup to avoid this behaviour.

Signed-off-by: Lorenz Bauer <lmb@isovalent.com>
Replace the copy pasta'd k8s watcher for CiliumEgressGatewayPolicy with
resource.Resource.

Signed-off-by: Lorenz Bauer <lmb@isovalent.com>
pkg/egressgateway/manager.go Show resolved Hide resolved
pkg/egressgateway/manager.go Show resolved Hide resolved
pkg/egressgateway/manager.go Show resolved Hide resolved
@jibi
Copy link
Member

jibi commented Aug 2, 2023

/test

@lmb lmb merged commit ebabe70 into cilium:main Aug 2, 2023
59 checks passed
@lmb lmb deleted the egw-cegp-resource branch August 2, 2023 09:00
@jibi jibi added the needs-backport/1.14 This PR / issue needs backporting to the v1.14 branch label Sep 4, 2023
@jibi jibi mentioned this pull request Sep 4, 2023
16 tasks
@jibi jibi removed the needs-backport/1.14 This PR / issue needs backporting to the v1.14 branch label Sep 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/modularization feature/egress-gateway Impacts the egress IP gateway feature. release-note/misc This PR makes changes that have no direct user impact.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants