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.

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

feature: support dns related flags when creating container through pouch cli #2380

Merged
merged 1 commit into from
Nov 5, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
5 changes: 5 additions & 0 deletions cli/common_flags.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,11 @@ func addCommonFlags(flagSet *pflag.FlagSet) *container {
// device related options
flagSet.StringSliceVarP(&c.devices, "device", "", nil, "Add a host device to the container")

// dns
flagSet.StringArrayVar(&c.dns, "dns", nil, "Set DNS servers")
flagSet.StringSliceVar(&c.dnsOptions, "dns-option", nil, "Set DNS options")
flagSet.StringArrayVar(&c.dnsSearch, "dns-search", nil, "Set DNS search domains")

flagSet.BoolVar(&c.enableLxcfs, "enableLxcfs", false, "Enable lxcfs for the container, only effective when enable-lxcfs switched on in Pouchd")
flagSet.StringVar(&c.entrypoint, "entrypoint", "", "Overwrite the default ENTRYPOINT of the image")
flagSet.StringArrayVarP(&c.env, "env", "e", nil, "Set environment variables for container")
Expand Down
9 changes: 8 additions & 1 deletion cli/container.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import (
"github.com/alibaba/pouch/apis/opts/config"
"github.com/alibaba/pouch/apis/types"

strfmt "github.com/go-openapi/strfmt"
"github.com/go-openapi/strfmt"
)

type container struct {
Expand Down Expand Up @@ -49,6 +49,10 @@ type container struct {
scheLatSwitch int64
oomKillDisable bool

dns []string
dnsOptions []string
dnsSearch []string

devices []string
enableLxcfs bool
privileged bool
Expand Down Expand Up @@ -233,6 +237,9 @@ func (c *container) config() (*types.ContainerCreateConfig, error) {
Ulimits: c.ulimit.Value(),
PidsLimit: c.pidsLimit,
},
DNS: c.dns,
DNSOptions: c.dnsOptions,
DNSSearch: c.dnsSearch,
EnableLxcfs: c.enableLxcfs,
Privileged: c.privileged,
RestartPolicy: restartPolicy,
Expand Down
73 changes: 71 additions & 2 deletions test/cli_run_dns_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ func (suite *PouchRunDNSSuite) TearDownTest(c *check.C) {
}

// TestRunWithUserDefinedNetwork tests enabling libnetwork resolver if user-defined network.
func (suite *PouchRunSuite) TestRunWithUserDefinedNetwork(c *check.C) {
func (suite *PouchRunDNSSuite) TestRunWithUserDefinedNetwork(c *check.C) {
cname := "TestRunWithUserDefinedNetwork"

// Create a user-defined network
Expand All @@ -52,7 +52,7 @@ func (suite *PouchRunSuite) TestRunWithUserDefinedNetwork(c *check.C) {
}

// TestRunWithBridgeNetwork tests disabling libnetwork resolver if not user-defined network.
func (suite *PouchRunSuite) TestRunWithBridgeNetwork(c *check.C) {
func (suite *PouchRunDNSSuite) TestRunWithBridgeNetwork(c *check.C) {
cname := "TestRunWithBridgeNetwork"

// Use bridge network if not set --net.
Expand All @@ -66,3 +66,72 @@ func (suite *PouchRunSuite) TestRunWithBridgeNetwork(c *check.C) {

c.Assert(res.Stdout(), check.Equals, hostRes.Stdout())
}

// TestRunWithDNSFlags tests DNS related flags.
func (suite *PouchRunDNSSuite) TestRunWithDNSFlags(c *check.C) {
cname := "TestRunWithDNSFlags"

res := command.PouchRun("run", "--name", cname,
Copy link
Contributor

Choose a reason for hiding this comment

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

since res not used, how about command.PouchRun(xx).(c, icmd.Success)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nope. res is used in line 84.

Copy link
Contributor

Choose a reason for hiding this comment

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

OK, got it , my mistake

"--dns", "1.2.3.4",
"--dns-option", "timeout:3",
"--dns-search", "example.com",
busyboxImage,
"cat", "/etc/resolv.conf")
defer DelContainerForceMultyTime(c, cname)
res.Assert(c, icmd.Success)

// test if the value is correct in container
out := strings.Trim(res.Stdout(), "\n")
c.Assert(strings.Contains(out, "nameserver 1.2.3.4"), check.Equals, true)
c.Assert(strings.Contains(out, "options timeout:3"), check.Equals, true)
c.Assert(strings.Contains(out, "search example.com"), check.Equals, true)

// test if the value is in inspect result
dns, err := inspectFilter(cname, ".HostConfig.DNS")
c.Assert(err, check.IsNil)
c.Assert(dns, check.Equals, "[1.2.3.4]")

dnsOptions, err := inspectFilter(cname, ".HostConfig.DNSOptions")
c.Assert(err, check.IsNil)
c.Assert(dnsOptions, check.Equals, "[timeout:3]")

dnsSearch, err := inspectFilter(cname, ".HostConfig.DNSSearch")
c.Assert(err, check.IsNil)
c.Assert(dnsSearch, check.Equals, "[example.com]")
}

// TestRunWithDNSRepeatFlags tests repeated DNS related flags.
func (suite *PouchRunDNSSuite) TestRunWithDNSRepeatFlags(c *check.C) {
cname := "TestRunWithDNSRepeatFlags"

res := command.PouchRun("run", "--name", cname,
"--dns", "1.2.3.4",
"--dns", "2.3.4.5",
"--dns-option", "timeout:3",
"--dns-option", "ndots:9",
"--dns-search", "mydomain",
"--dns-search", "mydomain2",
busyboxImage,
"cat", "/etc/resolv.conf")
defer DelContainerForceMultyTime(c, cname)
res.Assert(c, icmd.Success)
Copy link
Contributor

Choose a reason for hiding this comment

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

It's a good chioce to put command.PouchRun and Assert into one line? Since the res var didn't use after this.

Copy link
Contributor

Choose a reason for hiding this comment

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

I saw the behavior was divergent in the cli test.

Copy link
Contributor

Choose a reason for hiding this comment

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

@ZYecho Fixed.

LGTM, but waiting for the CI pass.


// test if the value is correct in container
out := strings.Trim(res.Stdout(), "\n")
c.Assert(strings.Contains(out, "nameserver 1.2.3.4\nnameserver 2.3.4.5"), check.Equals, true)
c.Assert(strings.Contains(out, "options timeout:3 ndots:9"), check.Equals, true)
c.Assert(strings.Contains(out, "search mydomain mydomain2"), check.Equals, true)

// test if the value is in inspect result
dns, err := inspectFilter(cname, ".HostConfig.DNS")
c.Assert(err, check.IsNil)
c.Assert(dns, check.Equals, "[1.2.3.4 2.3.4.5]")

dnsOptions, err := inspectFilter(cname, ".HostConfig.DNSOptions")
c.Assert(err, check.IsNil)
c.Assert(dnsOptions, check.Equals, "[timeout:3 ndots:9]")

dnsSearch, err := inspectFilter(cname, ".HostConfig.DNSSearch")
c.Assert(err, check.IsNil)
c.Assert(dnsSearch, check.Equals, "[mydomain mydomain2]")
}