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

optionally parse broker index from hostname #86

Closed
wants to merge 3 commits into from

Conversation

tpetr
Copy link
Contributor

@tpetr tpetr commented Sep 19, 2024

We should encourage folks to run multiple brokers in a StatefulSet, if they're running in Kubernetes. This PR adds an optional config field to parse the broker index from the hostname so that users don't have to deal with manually setting indexes.

Logic: (trying to find the right balance of explicit vs. "just works")

  1. if inbound.wireguard.brokerIndexHostnameRegex is set, compile the regex (fail fast if invalid)
  2. if --broker-index command line arg is set and >= 0, just use and ignore the other config settings
  3. if inbound.wireguard.brokerIndexHostnameRegex is set, attempt to match against the hostname.
    a. Fail fast if we get an anomalous result (e.g. regex is missing a capture group or we fail to convert the capture group to an integer)
    b. Fall through if there's simply no regex match (i.e. we shouldn't panic if the broker is not deployed as a StatefulSet)
  4. use whatever the inbound.wireguard.brokerIndex config value is (default 0)

(default value of inbound.wireguard.brokerIndexHostnameRegex is ^.+-([0-9]+)$ which should work for stateful sets.

@tpetr tpetr requested a review from zyannes September 19, 2024 15:35
zyannes
zyannes previously approved these changes Sep 19, 2024
zyannes
zyannes previously approved these changes Sep 19, 2024
@tpetr
Copy link
Contributor Author

tpetr commented Oct 22, 2024

Going to take a different approach to multiple brokers

@tpetr tpetr closed this Oct 22, 2024
@tpetr tpetr deleted the tom/parse-index-from-hostname branch October 22, 2024 15:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants