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.

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

add iptables option to the daemon #309

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

add iptables option to the daemon #309

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
e4b449e
add iptables option to the daemon
sherry-ummen Nov 25, 2020
90215b8
change to BoolTFlag
sherry-ummen Nov 25, 2020
34ddf9a
change to IPTablesOff
sherry-ummen Nov 25, 2020
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
6 changes: 6 additions & 0 deletions cmd/drone-docker/main.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,11 @@ func main() {
Usage: "docker daemon executes in debug mode",
EnvVar: "PLUGIN_DEBUG,DOCKER_LAUNCH_DEBUG",
},
cli.BoolTFlag{
Copy link
Member

Choose a reason for hiding this comment

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

I think this should be Bool instead of BoolT. The BoolT parameter defaults to true if unset, however, in this case I think we want to disable to false and only disable IP tables if the user explicitly sets this value to true

Name: "daemon.iptables",
Usage: "docker daemon enable addition of iptables rules",
Copy link
Member

Choose a reason for hiding this comment

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

also consider changing the description to make it clear that this flag disables iptable rules, instead of enabling

EnvVar: "PLUGIN_IPTABLES",
Copy link

Choose a reason for hiding this comment

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

change to PLUGIN_IPTABLES_OFF

Copy link
Author

Choose a reason for hiding this comment

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

done

},
cli.BoolFlag{
Name: "daemon.off",
Usage: "don't start the docker daemon",
Expand Down Expand Up @@ -285,6 +290,7 @@ func run(c *cli.Context) error {
Disabled: c.Bool("daemon.off"),
IPv6: c.Bool("daemon.ipv6"),
Debug: c.Bool("daemon.debug"),
IPTables: c.Bool("daemon.iptables"),
Copy link

@ashwilliams1 ashwilliams1 Nov 25, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

rename to IPTablesOff since they are on by default

Copy link
Author

Choose a reason for hiding this comment

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

done

Bip: c.String("daemon.bip"),
DNS: c.StringSlice("daemon.dns"),
DNSSearch: c.StringSlice("daemon.dns-search"),
Expand Down
4 changes: 4 additions & 0 deletions docker.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ type (
StoragePath string // Docker daemon storage path
Disabled bool // DOcker daemon is disabled (already running)
Debug bool // Docker daemon started in debug mode
IPTables bool // docker daemon enable/disable addition of iptables rules
Bip string // Docker daemon network bridge IP address
DNS []string // Docker daemon dns server
DNSSearch []string // Docker daemon dns search domain
Expand Down Expand Up @@ -370,6 +371,9 @@ func commandDaemon(daemon Daemon) *exec.Cmd {
if daemon.Experimental {
args = append(args, "--experimental")
}
if daemon.IPTables {
Copy link

Choose a reason for hiding this comment

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

need to verify the flag, but if iptables_off: true then we should disable iptables, which would otherwise be enabled by default

if daemon.IPTablesOff {
  args = append(args, "--iptables=false")
}

Copy link
Author

Choose a reason for hiding this comment

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

done

args = append(args, "--iptables")
}
return exec.Command(dockerdExe, args...)
}

Expand Down