-
Notifications
You must be signed in to change notification settings - Fork 126
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
Multi-interlock config #123
base: master
Are you sure you want to change the base?
Conversation
@@ -461,10 +477,8 @@ func (l *LoadBalancer) isExposedContainer(id string) bool { | |||
return false | |||
} | |||
|
|||
log().Debugf("checking container labels: id=%s", id) | |||
// ignore proxy containers | |||
if _, ok := c.Config.Labels[ext.InterlockExtNameLabel]; ok { |
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.
When the proxy is ignored:
- Proxy has containers in its config
- Kill proxy
- Run proxy
- Containers are not written to new proxy config
When the proxy isn't ignored:
- Proxy has containers in its config
- Kill proxy
- Run proxy
- Containers are written to new proxy config
@ehazlett Is there anything else I can do to help get this merged? |
@ehazlett Does anything else need to be done here? |
The review looks good. I tried to get this to work with the example in You can see the part of the log here where it is ignoring it (this is the
|
Also I don't like how Interlock uses the label for its service name. This should be in the |
Actually Interlock does need to have the service name in the For example
I documented the I chose to also have Interlock require the label because it makes it very simple to determine all of the containers that make up a functioning service. It makes it simple for Interlock itself and also for anyone listing containers with a |
@ehazlett Is this possible in newer versions of interlock or does this need to be merged? |
Closes #89