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

[docker/hostname] improve get_hostname for containers #3116

Merged
merged 1 commit into from
Mar 1, 2017

Conversation

hkaj
Copy link
Member

@hkaj hkaj commented Jan 9, 2017

Note: Please remember to review the Datadog Contribution Guidelines
if you have not yet done so.

What does this PR do?

When the agent is containerized it makes sense to use
docker info's Name as the hostname even if it doesn't resolve.
Because otherwise the agent falls back to the container short ID
which is not really helpful.

This PR drops the resolvability check for the name.

Motivation

Fixing DataDog/docker-dd-agent#139

Testing Guidelines

An overview on testing
is available in our contribution guidelines.

@hkaj hkaj requested a review from remh January 9, 2017 18:29
@hkaj hkaj added this to the 5.11.0 milestone Jan 9, 2017
@masci masci modified the milestones: 5.11.0, 5.12.0 Jan 24, 2017
@degemer
Copy link
Member

degemer commented Jan 30, 2017

We first need to make sure that the issue this code was fixing - #2753 - cannot happen anymore.

@hkaj
Copy link
Member Author

hkaj commented Feb 7, 2017

I believe this was fixing an issue where the agent couldn't reach the local kubelet due to the hostname not being routable. #3142 introduces new methods to reach kubelet so it should be fine. Let's just wait on it to be merged.

@hkaj hkaj force-pushed the haissam/improve-get_hostname-for-docker branch from 17ce742 to dd4d769 Compare February 7, 2017 17:28

if self.hostname is not None:
# Use cache
return self.hostname
if not should_resolve or is_resolvable(self.hostname):
Copy link
Member

Choose a reason for hiding this comment

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

This line can raise an uncaught error if the hostname is not resolvable.

@hkaj hkaj force-pushed the haissam/improve-get_hostname-for-docker branch 2 times, most recently from 0558fbf to 6516ce4 Compare February 10, 2017 11:01
When the agent is containerized it makes sense to use
docker info's Name as the hostname even if it doesn't resolve.
Because otherwise the agent fallsback to the container short ID
which is not really helpful.
@hkaj hkaj force-pushed the haissam/improve-get_hostname-for-docker branch from 6516ce4 to 579893d Compare March 1, 2017 13:12
@hkaj hkaj merged commit 563f632 into master Mar 1, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants