Skip to content

added the ability to include only container having label using INCLUDE_LABEL param #222

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.

Sign up for GitHub

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

Open
mohamed-el-habib wants to merge 3 commits into
base: master
Choose a base branch
from
Open

added the ability to include only container having label using INCLUDE_LABEL param #222

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
256154b
added INCLUDE_LABEL param
mohamed-el-habib Aug 10, 2016
77c5d84
remove comment and simplify check of the includeLabel param
mohamed-el-habib Aug 11, 2016
ff97e56
document includeLabel param
mohamed-el-habib Aug 11, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions router/pump.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,16 @@ func ignoreContainer(container *docker.Container) bool {
if value, ok := container.Config.Labels[excludeLabel]; ok {
return len(excludeLabel) > 0 && strings.ToLower(value) == "true"
}
includeLabel := getopt("INCLUDE_LABEL", "")
if includeLabel != "" {
if _, ok := container.Config.Labels[includeLabel]; ok {
Copy link
Copy Markdown
Member

@josegonzalez josegonzalez Aug 11, 2016

Choose a reason for hiding this comment

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

Is there a reason this check isn't similar to the exclude check where we check the actual value?

// include all containers that have the includeLabel
Copy link
Copy Markdown
Member

@josegonzalez josegonzalez Aug 11, 2016

Choose a reason for hiding this comment

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

Comments shouldn't be necessary, and if they are, the code needs to be made more clear.

return false
} else {
Copy link
Copy Markdown
Member

@josegonzalez josegonzalez Aug 11, 2016

Choose a reason for hiding this comment

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

We don't need the explicit else if the above will return false.

// ignore all containers that don't have the includeLabel
return true
}
}
return false
}

Expand Down