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

Use private network interfaces as default for ring and frontend #4961

Closed
wants to merge 1 commit into from

Conversation

chaudum
Copy link
Contributor

@chaudum chaudum commented Dec 17, 2021

Proof of concept for using private network interfaces as default for ring configurations.

What this PR does / why we need it:

At the moment, the default network interfaces for the ring configurations are eth0 and en0, which works for probably everyone running Loki inside Docker.
However, if you run the Loki binary directly (which is a valid first use-case for Loki), it is very likely that Loki won't start because the primary network interface has a different name.

Checklist

  • Documentation added
  • Tests updated
  • Add an entry in the CHANGELOG.md about the changes.

@chaudum chaudum requested a review from a team as a code owner December 17, 2021 14:24
Copy link
Contributor

@DylanGuedes DylanGuedes left a comment

Choose a reason for hiding this comment

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

Although I consider this a better default, I think it is considered a breaking change. That said, WDYT of documenting this in the upgrading guide?

@@ -158,54 +155,3 @@ func searchToken(tokens []uint32, key uint32) int {
}
return i
}

// GetFirstAddressOf returns the first IPv4 address of the supplied interface names, omitting any 169.254.x.x automatic private IPs if possible.
func getFirstAddressOf(names []string, logger log.Logger) (string, error) {
Copy link
Contributor

Choose a reason for hiding this comment

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

hmm here you're changing vendor code 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry, I wanted to open it as a draft PR. The corresponding upstream PR in dskit is grafana/dskit#100
It would also need to be used in Cortex.

Copy link
Contributor

Choose a reason for hiding this comment

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

oops, my bad.

@chaudum chaudum marked this pull request as draft December 17, 2021 14:48
@stale
Copy link

stale bot commented Mar 2, 2022

Hi! This issue has been automatically marked as stale because it has not had any
activity in the past 30 days.

We use a stalebot among other tools to help manage the state of issues in this project.
A stalebot can be very useful in closing issues in a number of cases; the most common
is closing issues or PRs where the original reporter has not responded.

Stalebots are also emotionless and cruel and can close issues which are still very relevant.

If this issue is important to you, please add a comment to keep it open. More importantly, please add a thumbs-up to the original issue entry.

We regularly sort for closed issues which have a stale label sorted by thumbs up.

We may also:

  • Mark issues as revivable if we think it's a valid issue but isn't something we are likely
    to prioritize in the future (the issue will still remain closed).
  • Add a keepalive label to silence the stalebot if the issue is very common/popular/important.

We are doing our best to respond, organize, and prioritize all issues but it can be a challenging task,
our sincere apologies if you find yourself at the mercy of the stalebot.

@stale stale bot added the stale A stale issue or PR that will automatically be closed. label Mar 2, 2022
@kavirajk
Copy link
Contributor

Closing this as no activities for long time. Feel free to send new PR if anyone want's to revive the work

@kavirajk kavirajk closed this Mar 18, 2022
@JStickler JStickler deleted the chaudum/use-private-interfaces branch May 22, 2023 19:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size/XS stale A stale issue or PR that will automatically be closed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants