-
Notifications
You must be signed in to change notification settings - Fork 9.2k
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
[release 2.54] [BUGFIX] Docker SD: fix the issue of failing to match the first network when the container is reconnected to a new network #14654
Conversation
cc @bboreham |
8a57a14
to
197adfc
Compare
Should this be against release-2.54 branch? |
discovery/moby/docker.go
Outdated
// 3. Reconnect network with user defined networks. | ||
var first string | ||
firstNetworkMode = string(containerNetworkMode) | ||
firstNetwork = networks[firstNetworkMode] |
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.
I'm thinking about whether this step is still necessary because the next step sorts all non-nil networks in ascending order and gets the first one from the sorted list of networks, which I think is more consistent.
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.
If someone wants to use a specific network, they can use the following configuration:
scrape_configs:
- job_name: 'docker-containers'
docker_sd_configs:
- host: unix:///var/run/docker.sock
match_first_network: false
relabel_configs:
# Select containers whose name is prefixed by `exporter__`.
- source_labels: [__meta_docker_container_name]
regex: /exporter__. # For some reason, container names begin with a '/' character.
action: keep
# Create new label named 'container' with the value from the meta container name label.
- source_labels: [__meta_docker_container_name]
target_label: container
regex: /(. )
- source_labels: [__meta_docker_network_name]
regex: mysql_net1
action: keep
Yep. |
197adfc
to
6c95821
Compare
@bboreham Ready for review. |
6c95821
to
90d260d
Compare
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.
Is it worth adding an additional check that the network is non-nil, at the point where it crashed in #14647 ?
(Note I rebased and force-pushed your branch onto release-2.54 branch)
Agree, make sense. |
0d2c7e9
to
c23f097
Compare
Updated. |
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.
Code seems ok (though I am not a Docker expert).
Please amend the documentation in docs/configuration/configuration.md to describe the new behaviour. (or help me understand why this is unnecessary)
…r is reconnected to a new network Signed-off-by: [email protected] <[email protected]>
c23f097
to
89dee48
Compare
Done. |
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.
Thanks!
In the scenario where the container is connected to multiple networks, if the container is reconnected to a new network, the network mode will not change which will causes the issue of failing to match the first network.
Follows #10490.
Fixes #14647.