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_daemon] [ecs] introspection resilience #2745

Merged
merged 3 commits into from
Sep 6, 2016
Merged

Conversation

truthbk
Copy link
Member

@truthbk truthbk commented Aug 9, 2016

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

What does this PR do?

ECS agent container no longer has the IP address or ports available when we inspect its container to infer the introspection point. The docker gateway, typically at 172.17.0.1 is fine for constructing the url to query the API. We can set it as a parameter in the config file, or we can extract it from the agent's own information.

Also, although rare, if the agent is not run dockerized, we can grab it @localhost

Motivation

Our ECS task reporting broke due to our inability to query the endpoint.

Testing Guidelines

An overview on testing
is available in our contribution guidelines.

Additional Notes

Anything else we should know when reviewing?

@truthbk truthbk changed the title Jaime/ecs introspect ECS introspection resilience Aug 9, 2016
@truthbk
Copy link
Member Author

truthbk commented Aug 9, 2016

Still untested - just verified manually. VERIFIED working - conatiner's default gateway works just fine because the ecs agent exposes its port.

@truthbk truthbk force-pushed the jaime/ecs_introspect branch from 9ac94d8 to d44e084 Compare August 10, 2016 21:37
@truthbk truthbk changed the title ECS introspection resilience [docker_daemon] [ecs] introspection resilience Aug 10, 2016
@truthbk truthbk added this to the 5.9.0 milestone Aug 10, 2016
@truthbk truthbk force-pushed the jaime/ecs_introspect branch 2 times, most recently from 6a5ee76 to 5f1c6f3 Compare August 10, 2016 22:41
for line in f.readlines():
fields = line.strip().split()
if fields[1] == '00000000':
return socket.inet_ntoa(struct.pack('<L', int(fields[2], 16)))
Copy link
Member Author

@truthbk truthbk Aug 12, 2016

Choose a reason for hiding this comment

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

We might need some fallback for ipv6 only systems... Docker doesn't support ipv6-only, so we're good (for now).

[docker_deamon] [ecs] add the ability to add the ecs (docker) gateway.

[docker_daemon] [ecs] no need to raise, implicit in json() call.

[docker_deamon] [ecs] get docker gw from /proc.
@truthbk truthbk force-pushed the jaime/ecs_introspect branch from 5f1c6f3 to 0a3e954 Compare August 31, 2016 22:25
@truthbk truthbk force-pushed the jaime/ecs_introspect branch from 0a3e954 to 45cf3a8 Compare August 31, 2016 22:28
port = ECS_INTROSPECT_DEFAULT_PORT
elif not ip:
ip = "localhost"
port = ECS_INTROSPECT_DEFAULT_PORT
Copy link
Member

Choose a reason for hiding this comment

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

You could refactor the conditions to look like:

if not ip:
    port = ECS_INTROSPECT_DEFAULT_PORT
    if DockerUtil.is_dockerized() and self.docker_gateway:
        ip = self.docker_gateway
    else:
        ip = 'localhost'

which is 1 line shorter \o/ (kidding aside I think it's also a bit clearer)

@hkaj
Copy link
Member

hkaj commented Sep 5, 2016

2 minor comments, feel free to merge once they're addressed 👍

@truthbk truthbk force-pushed the jaime/ecs_introspect branch from c59ac6f to 9b60949 Compare September 6, 2016 14:36
@truthbk truthbk merged commit 58afab3 into master Sep 6, 2016
@kuvia
Copy link

kuvia commented Sep 8, 2016

I've pulled the image docker-dd-agent:latest but this change is not in there yet. When can I expect an updated image in the docker hub?
Thanks!

@kuvia
Copy link

kuvia commented Sep 9, 2016

I've tried the nightly build and I get an exception in refresh_ecs_tags
2016-09-09 11:16:57 UTC | CRITICAL | dd.collector | utils.dockerutil(dockerutil.py:167) | Unable to find docker host hostname. Trying default route 2016-09-09 11:17:00 UTC | ERROR | dd.collector | checks.docker_daemon(__init__.py:787) | Check 'docker_daemon' instance #0 failed Traceback (most recent call last): File "/opt/datadog-agent/agent/checks/__init__.py", line 770, in run self.check(copy.deepcopy(instance)) File "/opt/datadog-agent/agent/checks.d/docker_daemon.py", line 245, in check self.refresh_ecs_tags() File "/opt/datadog-agent/agent/checks.d/docker_daemon.py", line 451, in refresh_ecs_tags if DockerUtil.is_dockerized() and self.docker_gateway(): TypeError: 'str' object is not callable

@hkaj
Copy link
Member

hkaj commented Sep 9, 2016

Hi @himawri
That's a known issue, it's been addressed by #2825 which will go out in the next nightly.

@truthbk truthbk deleted the jaime/ecs_introspect branch August 2, 2017 10:21
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