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

Proxy router restart triggered erroneously #1584

Closed
awh opened this issue Oct 23, 2015 · 8 comments
Closed

Proxy router restart triggered erroneously #1584

awh opened this issue Oct 23, 2015 · 8 comments
Assignees
Milestone

Comments

@awh
Copy link
Contributor

awh commented Oct 23, 2015

The weave script invokes the router container to take actions pertaining to fast datapath (creation/deletion of the datapath itself, and the addition of veths to the datapath) - since this has the same entrypoint as the router, the proxy thinks the router has been restarted by Docker and attempts to attach the ephemeral container to the weave network.

@awh awh added the bug label Oct 23, 2015
@awh awh self-assigned this Oct 23, 2015
@awh awh added this to the 1.3.0 milestone Oct 23, 2015
@awh
Copy link
Contributor Author

awh commented Oct 23, 2015

The plan of record for addressing this is to move the datapath manipulation functions into a separate library and incrementally pursue the migration discussed in the commentary of #307.

@rade
Copy link
Member

rade commented Oct 23, 2015

Is there any ill effect from this?

The plan of record for addressing this is to move the datapath manipulation functions into a separate library

Not quite. The plan is to take the dp functions and combine them with netcheck into a new executable (weaveutil? urgh)

@rade
Copy link
Member

rade commented Oct 23, 2015

combine them with netcheck

We could also include docker_tls_args here, the only other homegrown executable invoked by the script. However, I reckon for the time being weaveutil should be docker-free (note that docker_tls_args doesn't actually depend on any docker libraries, but it is docker-specific code).

@awh
Copy link
Contributor Author

awh commented Oct 23, 2015

Is there any ill effect from this?

I don't think so. The proxy calls weave attach-router, which is hardwired with a container name of weave - this is apparently idempotent.

@rade
Copy link
Member

rade commented Oct 23, 2015

hardwired with a container name of weave

d'oh. We should have used that as an additional discriminator in containerIsWeaveRouter. I suggest we do that for 1.2.1.

@awh
Copy link
Contributor Author

awh commented Oct 23, 2015

We should have used that as an additional discriminator

Is there a reason to not use it exclusively?

@awh
Copy link
Contributor Author

awh commented Oct 23, 2015

func containerIsWeaveRouter(container *docker.Container) bool {
    return container.Name == "/weave"
}

fixes the problem...

@rade
Copy link
Member

rade commented Oct 23, 2015

Is there a reason to not use it exclusively?

Yes, we want to take some reasonable steps to not mess with random containers called 'weave'. The weave script does that when launching, i.e. it moans differently about a running weave than a running container that happens to be called weave. The script uses the image name to tell the difference, but that is not so easily done in the proxy (the image name depends on the DOCKERHUB_USER), so we just check whether the entrypoint looks weaveish.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

2 participants