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

[liqoctl] set gateway server Service location in peer command #2913

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

fra98
Copy link
Member

@fra98 fra98 commented Jan 31, 2025

Description

This PR introduces the support to specify the gateway client and server location in a peering.
The --server-service-location flag in liqoctl peer allows to place the gateway server service in the provider (default) or consumer.
This can be useful if you cannot expose Services (or don't have LB UDP support) in the provider cluster or if the provider is behind a NAT (without going through the manual peering procedure or setting up the networking module individually).

@adamjensenbot
Copy link
Collaborator

Hi @fra98. Thanks for your PR!

I am @adamjensenbot.
You can interact with me issuing a slash command in the first line of a comment.
Currently, I understand the following commands:

  • /rebase: Rebase this PR onto the master branch (You can add the option test=true to launch the tests
    when the rebase operation is completed)
  • /merge: Merge this PR into the master branch
  • /build Build Liqo components
  • /test Launch the E2E and Unit tests
  • /hold, /unhold Add/remove the hold label to prevent merging with /merge

Make sure this PR appears in the liqo changelog, adding one of the following labels:

  • feat: 🚀 New Feature
  • fix: 🐛 Bug Fix
  • refactor: 🧹 Code Refactoring
  • docs: 📝 Documentation
  • style: 💄 Code Style
  • perf: 🐎 Performance Improvement
  • test: ✅ Tests
  • chore: 🚚 Dependencies Management
  • build: 📦 Builds Management
  • ci: 👷 CI/CD
  • revert: ⏪ Reverts Previous Changes

@github-actions github-actions bot added feat Adds a new feature to the codebase fix Fixes a bug in the codebase. docs Updates or adds documentation labels Jan 31, 2025
@fra98 fra98 changed the title fix: wggateway template ports when metrics not enabled [Liqoctl peer] set gateway server Service position Jan 31, 2025
@fra98 fra98 changed the title [Liqoctl peer] set gateway server Service position [liqoctl] set gateway server Service position in peer command Jan 31, 2025
@fra98 fra98 requested review from cheina97 and claudiolor January 31, 2025 14:55
@fra98
Copy link
Member Author

fra98 commented Jan 31, 2025

/rebase

@adamjensenbot adamjensenbot force-pushed the frt/liqoctl-gw-position branch from 91aad33 to cb2934c Compare January 31, 2025 16:45
@github-actions github-actions bot removed the fix Fixes a bug in the codebase. label Jan 31, 2025
@fra98
Copy link
Member Author

fra98 commented Jan 31, 2025

/test

@fra98 fra98 force-pushed the frt/liqoctl-gw-position branch from cb2934c to 9741cc1 Compare February 3, 2025 11:58
@fra98 fra98 requested a review from aleoli February 3, 2025 11:59
@fra98 fra98 changed the title [liqoctl] set gateway server Service position in peer command [liqoctl] set gateway server Service location in peer command Feb 3, 2025
@fra98
Copy link
Member Author

fra98 commented Feb 3, 2025

/test

@fra98
Copy link
Member Author

fra98 commented Feb 4, 2025

/test

@fra98 fra98 force-pushed the frt/liqoctl-gw-position branch from 9741cc1 to 2048e52 Compare February 4, 2025 12:02
@fra98
Copy link
Member Author

fra98 commented Feb 4, 2025

/test

1 similar comment
@fra98
Copy link
Member Author

fra98 commented Feb 5, 2025

/test

@fra98 fra98 force-pushed the frt/liqoctl-gw-position branch from 2048e52 to 62f1883 Compare February 5, 2025 15:01
@fra98
Copy link
Member Author

fra98 commented Feb 5, 2025

/test

@fra98
Copy link
Member Author

fra98 commented Feb 7, 2025

/rebase test=true

@adamjensenbot adamjensenbot force-pushed the frt/liqoctl-gw-position branch from 62f1883 to 00f95f2 Compare February 7, 2025 10:43
@frisso
Copy link
Member

frisso commented Feb 7, 2025

Difficult for me to understand what "server" means in the command line, "--server-service-location".
I would suggest to propose a more intuitive name (e.g., --gateway-server-location", perhaps?)

@@ -36,7 +36,8 @@ To perform a peering without having access to both clusters, you need to manuall
The peering command enables all 3 liqo modules and performs the following steps:

1. **enables networking**.
Exchanges network configurations and creates the two **gateways** (server in the provider, client in the consumer) to let the two clusters communicate over a secure tunnel.
Exchanges network configurations and creates the two **gateways** (one server in a cluster and one client in the other) to let the two clusters communicate over a secure tunnel.
By default the gateway server in placed in the provider, while the gateway client in the consumer, but you can configure it with the `--server-service-location` flag.
Copy link
Member

@frisso frisso Feb 7, 2025

Choose a reason for hiding this comment

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

Suggested change
By default the gateway server in placed in the provider, while the gateway client in the consumer, but you can configure it with the `--server-service-location` flag.
The location of the client/server gateway can be customized when creating the peering using the `--server-service-location` flag in `liqoctl`.

@@ -36,7 +36,8 @@ To perform a peering without having access to both clusters, you need to manuall
The peering command enables all 3 liqo modules and performs the following steps:

1. **enables networking**.
Exchanges network configurations and creates the two **gateways** (server in the provider, client in the consumer) to let the two clusters communicate over a secure tunnel.
Exchanges network configurations and creates the two **gateways** (one server in a cluster and one client in the other) to let the two clusters communicate over a secure tunnel.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Exchanges network configurations and creates the two **gateways** (one server in a cluster and one client in the other) to let the two clusters communicate over a secure tunnel.
Exchanges network configurations and creates the two **gateways** (one acting as _server_ and located in the provider cluster, another acting as _client_ in the consumer cluster) to let the two clusters communicate over a secure tunnel.

@@ -90,8 +91,11 @@ func newPeerCommand(ctx context.Context, f *factory.Factory) *cobra.Command {

// Networking flags
cmd.Flags().BoolVar(&options.NetworkingDisabled, "networking-disabled", false, "Disable networking between the two clusters")
cmd.Flags().Var(options.ServerServiceLocation, "server-service-location",
fmt.Sprintf("Location of the service to expose the Gateway Server (%q or %q). Default: %q",
Copy link
Member

Choose a reason for hiding this comment

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

Not sure I would understand correctly.
I would say this way:
fmt.Sprintf("Cluster that acts as gateway server when setting up the network between two Liqo instances (%q or %q). Default: %q",

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs Updates or adds documentation feat Adds a new feature to the codebase size/M
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants