Skip to content
This repository has been archived by the owner on Jun 20, 2024. It is now read-only.

make container aliveness check part of the ipam & dns API #1073

Merged
merged 1 commit into from
Jul 7, 2015

Conversation

rade
Copy link
Member

@rade rade commented Jul 4, 2015

...rather than conducting it implicitly when an ID "looks like" a container ID.

Closes #971.

...rather than conducting it implicitly when an ID "looks like" a
container ID.

Closes #971
@inercia
Copy link
Contributor

inercia commented Jul 5, 2015

What is the rationale for this? I think that we check the aliveness iff the container id looks like a container id, right?

@rade
Copy link
Member Author

rade commented Jul 6, 2015

What is the rationale for this?

See the discussion in #971. The purpose is simply to avoid the "looks like a container ID" semantics, which was an implicit part of the API, and instead make it explicitly the callers responsibility to request an ID is treated as a container ID and checked for aliveness.

@inercia
Copy link
Contributor

inercia commented Jul 6, 2015

Maybe "looks like a container ID" is not enough reason for checking for aliveness, but server should check that a valid ID has been provided anyway...

@rade
Copy link
Member Author

rade commented Jul 6, 2015

server should check that a valid ID has been provided anyway

What does "valid ID" mean? And would the purpose of such a check be?

@inercia
Copy link
Contributor

inercia commented Jul 6, 2015

Your code could happily check with Docker for aliveness of any container name (eg, weave:expose) as long as check-alive is true. I think that kind of checks should be caught earlier and save the query to Docker...

@rade
Copy link
Member Author

rade commented Jul 6, 2015

So you are saying that if a user makes a mistake, and asked to check the aliveness of something that isn't a container id, then we should catch that and suppress the check with docker? And do what? Add the record? Or not? It's exactly this kind of not-immediately obvious semantics that prompted @squaremo to file the issue.

@squaremo squaremo removed their assignment Jul 6, 2015
@squaremo
Copy link
Contributor

squaremo commented Jul 6, 2015

Travis seems to have failed for a spurious reason. Restarted that build.

@squaremo squaremo removed their assignment Jul 6, 2015
@squaremo
Copy link
Contributor

squaremo commented Jul 6, 2015

squaremo removed their assignment just now

@rade If I'm looking into a PR or issue, I will assign myself. If you would like me to look at something, please invite me to do so by @-mentioning me, or by email, or even -- gasp -- by talking to me.

inercia added a commit that referenced this pull request Jul 7, 2015
…_api

make container aliveness check part of the ipam & dns API
@inercia inercia merged commit 33921c2 into master Jul 7, 2015
@rade rade deleted the 971_check_container_alive_in_api branch July 8, 2015 15:00
@rade rade modified the milestones: current, 1.1.0 Jul 13, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants