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

clustering: fix ipv6 advertise addresses #869

Merged
merged 1 commit into from
May 23, 2024
Merged

Conversation

matthewpi
Copy link
Contributor

PR Description

The previous logic for handling IPv6 addresses for clustering was flawed. This PR updates it so IPv6 addresses are properly joined with their ports, avoiding a too many colons in address error upon starting Alloy in cluster-mode.

Which issue(s) this PR fixes

There is not an open issue for this bug.

Notes to the Reviewer

PR Checklist

  • CHANGELOG.md updated
  • Tests updated

@CLAassistant
Copy link

CLAassistant commented May 15, 2024

CLA assistant check
All committers have signed the CLA.

Copy link
Member

@tpaschalis tpaschalis left a comment

Choose a reason for hiding this comment

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

Hey Matthew! I think this fix makes sense; is there any easy way to add a test so that we don't accidentally regress on this?

Signed-off-by: Matthew Penner <me@matthewp.io>
@matthewpi
Copy link
Contributor Author

Hey Matthew! I think this fix makes sense; is there any easy way to add a test so that we don't accidentally regress on this?

I've moved the logic to a separate function so it can be tested independently. Right now it just tests that the port is correctly joined with the IP and that the loopback fallback works. Without digging deeper into where the advertise address is actually used, I cannot easily add a test ensuring that the proper address is used. But the test I did add should help avoid issues if the IP selection and port join logic changes.

Copy link
Member

@tpaschalis tpaschalis left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@tpaschalis tpaschalis merged commit 3df2cd0 into grafana:main May 23, 2024
14 of 15 checks passed
rfratto pushed a commit to rfratto/alloy that referenced this pull request May 30, 2024
Signed-off-by: Matthew Penner <me@matthewp.io>
(cherry picked from commit 3df2cd0)
rfratto added a commit that referenced this pull request May 30, 2024
* Fix panic when component ID contains `/` in `otelcomponent.MustNewType(ID)` (#858)

Signed-off-by: Weifeng Wang <qclaogui@gmail.com>
(cherry picked from commit 7bae89c)

* No error when http fails (#841)

* Fail if we see the port is not available

* Update changelog

* cleanup message

* Update CHANGELOG.md

Co-authored-by: Erik Baranowski <39704712+erikbaranowski@users.noreply.github.com>

---------

Co-authored-by: Erik Baranowski <39704712+erikbaranowski@users.noreply.github.com>
(cherry picked from commit 4ca3f84)

* fix panic loki source docker (#875)

(cherry picked from commit 4fb1df9)

* clustering: fix ipv6 advertise addresses (#869)

Signed-off-by: Matthew Penner <me@matthewp.io>
(cherry picked from commit 3df2cd0)

* otelcol: decouple otel/alloy component IDs (#882)

Signed-off-by: Paschalis Tsilias <paschalist0@gmail.com>
(cherry picked from commit d018e6e)

* updates with latest snowflake prometheus exporter (fixes null issues) (#939)

* updates with latest snowflake prometheus exporter (fixes null issues)

* Update CHANGELOG.md

Co-authored-by: William Dumont <william.dumont@grafana.com>

---------

Co-authored-by: William Dumont <william.dumont@grafana.com>
(cherry picked from commit 551d407)

* feat(vcs): bubble up SSH key conversion error for better debugging experience (#943)

* feat(vcs): bubble up SSH key conversion error for better debugging experience

Signed-off-by: hainenber <dotronghai96@gmail.com>

* chore: refactor code to be more succinct

Signed-off-by: hainenber <dotronghai96@gmail.com>

---------

Signed-off-by: hainenber <dotronghai96@gmail.com>
(cherry picked from commit 037893f)

* prepare changelog for 1.1.1 (#958)

This includes all bugfixes found in main to date except for #703, which
is a more involved change that should probably wait for a minor release.

(cherry picked from commit 3bceb1a)

---------

Co-authored-by: Weifeng Wang <qclaogui@gmail.com>
Co-authored-by: mattdurham <mattdurham@ppog.org>
Co-authored-by: William Dumont <william.dumont@grafana.com>
Co-authored-by: Matthew Penner <me@matthewp.io>
Co-authored-by: Paschalis Tsilias <tpaschalis@users.noreply.github.com>
Co-authored-by: Stefan Kurek <stefan.kurek@observiq.com>
Co-authored-by: Đỗ Trọng Hải <41283691+hainenber@users.noreply.github.com>
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jun 25, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants