-
Notifications
You must be signed in to change notification settings - Fork 187
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
Support for TLS client settings for clustering #1724
Conversation
docs review Co-authored-by: Clayton Cornell <131809008+clayton-cornell@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Looks good to me, if you can resolve the conflict automerge will merge it. |
Head branch was pushed to by a user without write access
@mattdurham Much appreciated! Resolved the |
Head branch was pushed to by a user without write access
@mattdurham Could you give me a hand taking a look at the failing test? It doesn't seem related to any changes here, so I'm a bit puzzled. |
return net.DialTimeout(network, addr, timeout) | ||
}, | ||
httpTransport := &http2.Transport{ | ||
AllowHTTP: false, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@wildum perhaps this caused the outage we're seeing?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
according to https://github.com/grafana/ckit/blob/main/node.go#L141, AllowHTTP should be set to true for non-TLS connections
PR Description
Clustering in TLS environments was not possible before these changes.
Why is this change required
My organization runs alloy pods in environments that require all pod communication to be over HTTPS with TLS encryption. When enabling clustering, we noticed that communication between pods was blocked because it was going over HTTP.
Testing
Built
alloy
from this branch and used theenable-tls
CLI flag with the appropriate certificate material to configure the TLS client. Made sure that pods can still communicate amongst themselves and constantly converge on the cluster's state.Some logs that confirm pod communication still works:
Grafana Alloy debugging UI also shows that clustered pods can still recognize each other:
Which issue(s) this PR fixes
Fixes #367
Notes to the Reviewer
This PR depends on grafana/ckit#63, which has not yet been merged.
PR Checklist