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

Egress gateway modularization #23046

Conversation

dylandreimerink
Copy link
Member

This PR converts the egress gateway into a Hive cell. To facilitate this change the ipcache.IPCache, daemon.CachingIdentityAllocator, policy.Repository, and certificatemanager have been modularized as well.

@dylandreimerink dylandreimerink added kind/cleanup This includes no functional changes. release-note/misc This PR makes changes that have no direct user impact. feature/egress-gateway Impacts the egress IP gateway feature. labels Jan 11, 2023
@github-actions github-actions bot added the kind/community-contribution This was a contribution made by a community member. label Jan 11, 2023
@dylandreimerink dylandreimerink removed the kind/community-contribution This was a contribution made by a community member. label Jan 11, 2023
pkg/ipcache/ipcache.go Outdated Show resolved Hide resolved
daemon/cmd/policy.go Outdated Show resolved Hide resolved
)

type Manager interface {
OnAddEgressPolicy(config PolicyConfig)
Copy link
Contributor

Choose a reason for hiding this comment

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

Are there interface definitions for these we can just embed here? e.g. subscriber.Node? Would make the purpose clear.

pkg/egressgateway/manager.go Outdated Show resolved Hide resolved

import "github.com/cilium/cilium/pkg/hive/cell"

var Cell = cell.Provide(NewIPCache)
Copy link
Contributor

Choose a reason for hiding this comment

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

Consider extracting the interface here. go doc ipcache.IPCache gives a good starting point. Also see if you can split the interface up by functionality (e.g. one for lookups, one for modifying etc.).

Copy link
Member Author

Choose a reason for hiding this comment

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

I gave this a shot, we can significantly simplify the public API of the ipcache but it results in quite a significant diff, so I would prefer to do that in a separate PR.

pkg/wireguard/agent/agent_test.go Show resolved Hide resolved
@dylandreimerink dylandreimerink force-pushed the feature/egressgw-modularization branch 2 times, most recently from fce8d96 to 18d20c0 Compare January 16, 2023 10:39
dylandreimerink and others added 4 commits January 17, 2023 11:19
This commit added a Cell for the certificate manager. The cell exposes
two new interfaces `CertificateManager` and `SecretManager` instead
of the manager directly.

The configuration/flags for the manager has been moved into the Cell.

Signed-off-by: Dylan Reimerink <dylan.reimerink@isovalent.com>
To also allow rejecting a promise in promise.Map add the error return
value to it.

Signed-off-by: Jussi Maki <jussi@isovalent.com>
This commit turns three circularly dependent components into Cells,
those being `ipcache.IPCache`, `daemon.CachingIdentityAllocator`, and
`policy.Repository`.

The main objective was to modularize the caching identity allocator but
its dependent on IPCache, which is dependent on the policy repo which
in turn is dependent on the identity allocator, thus making a cycle.

Promises are used to wire the CachingIdentityAllocator and policy repo
since import cycles aren't allowed in Hive.

Daemon was refactored to deal with the increase in passed parameters and
use daemon context for promise awaits (cancelled if start times out)

Signed-off-by: Dylan Reimerink <dylan.reimerink@isovalent.com>
Co-authored-by: Jussi Maki <jussi@isovalent.com>
This commit turns the egress gateway manager into a Cell. This builds
on the modularization of the CachingIdentityAllocator.

Until now the egress gateway would busy-poll the daemon to see if the
k8s caches are synced. This is now done via a channel on a new object
called `k8s.CacheSyncedChecker` which is shared via Hive to both the
daemon which will modify it and the egress gateway which will subscribe
to it.

This commit also abstracts the egress gateway manager behind an
interface to make it loosely coupled.

Threshold for too many arguments to newDaemon has been passed, so
just use daemonParams directly.

Also respect the start context by cancelling the daemon context if
it times out before start has finished.

Signed-off-by: Dylan Reimerink <dylan.reimerink@isovalent.com>
Co-authored-by: Jussi Maki <jussi@isovalent.com>
@github-actions
Copy link

This pull request has been automatically marked as stale because it
has not had recent activity. It will be closed if no further activity
occurs. Thank you for your contributions.

@github-actions github-actions bot added the stale The stale bot thinks this issue is old. Add "pinned" label to prevent this from becoming stale. label Feb 17, 2023
@joamaki joamaki mentioned this pull request Feb 20, 2023
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature/egress-gateway Impacts the egress IP gateway feature. kind/cleanup This includes no functional changes. release-note/misc This PR makes changes that have no direct user impact. stale The stale bot thinks this issue is old. Add "pinned" label to prevent this from becoming stale.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants