Resolve review comments

Signed-off-by: Matthew Heon <matthew.heon@pm.me>
This commit is contained in:
Matthew Heon
2019-03-26 13:55:19 -04:00
parent 86f03e0e52
commit 0cd92eae65
7 changed files with 28 additions and 30 deletions

View File

@ -357,10 +357,8 @@ func ParseCreateOpts(ctx context.Context, c *cliconfig.PodmanCommand, runtime *l
return nil, errors.Errorf("--cpu-quota and --cpus cannot be set together") return nil, errors.Errorf("--cpu-quota and --cpus cannot be set together")
} }
if c.Flag("no-hosts").Changed && c.Flag("add-host").Changed { if c.Bool("no-hosts") && c.Flag("add-host").Changed {
if c.Bool("no-hosts") { return nil, errors.Errorf("--no-hosts and --add-host cannot be set together")
return nil, errors.Errorf("--no-hosts and --add-host cannot be set together")
}
} }
// EXPOSED PORTS // EXPOSED PORTS

View File

@ -204,7 +204,7 @@ configuration passed to the container. Typically this is necessary when the
host DNS configuration is invalid for the container (e.g., 127.0.0.1). When this host DNS configuration is invalid for the container (e.g., 127.0.0.1). When this
is the case the **--dns** flags is necessary for every run. is the case the **--dns** flags is necessary for every run.
The special sigil **none** can be specified to disable creation of **/etc/resolv.conf** in the container by Podman. The special value **none** can be specified to disable creation of **/etc/resolv.conf** in the container by Podman.
The **/etc/resolv.conf** file in the image will be used without changes. The **/etc/resolv.conf** file in the image will be used without changes.
**--dns-option**=[] **--dns-option**=[]
@ -465,7 +465,7 @@ Not implemented
Do not create /etc/hosts for the container. Do not create /etc/hosts for the container.
By default, Podman will manage /etc/hosts, adding the container's own IP address and any hosts from **--add-host**. By default, Podman will manage /etc/hosts, adding the container's own IP address and any hosts from **--add-host**.
**--no-hosts** disables this, and the image's **/etc/host** will be preserved unmodified. **--no-hosts** disables this, and the image's **/etc/host** will be preserved unmodified.
This conflicts with **--add-host**. This option conflicts with **--add-host**.
**--oom-kill-disable**=*true*|*false* **--oom-kill-disable**=*true*|*false*

View File

@ -210,7 +210,7 @@ configuration passed to the container. Typically this is necessary when the
host DNS configuration is invalid for the container (e.g., 127.0.0.1). When this host DNS configuration is invalid for the container (e.g., 127.0.0.1). When this
is the case the **--dns** flags is necessary for every run. is the case the **--dns** flags is necessary for every run.
The special sigil **none** can be specified to disable creation of **/etc/resolv.conf** in the container by Podman. The special value **none** can be specified to disable creation of **/etc/resolv.conf** in the container by Podman.
The **/etc/resolv.conf** file in the image will be used without changes. The **/etc/resolv.conf** file in the image will be used without changes.
**--dns-option**=[] **--dns-option**=[]
@ -449,7 +449,7 @@ Not implemented
Do not create /etc/hosts for the container. Do not create /etc/hosts for the container.
By default, Podman will manage /etc/hosts, adding the container's own IP address and any hosts from **--add-host**. By default, Podman will manage /etc/hosts, adding the container's own IP address and any hosts from **--add-host**.
**--no-hosts** disables this, and the image's **/etc/host** will be preserved unmodified. **--no-hosts** disables this, and the image's **/etc/host** will be preserved unmodified.
This conflicts with **--add-host**. This option conflicts with **--add-host**.
**--oom-kill-disable**=*true*|*false* **--oom-kill-disable**=*true*|*false*

View File

@ -293,10 +293,10 @@ type ContainerConfig struct {
// namespace // namespace
// These are not used unless CreateNetNS is true // These are not used unless CreateNetNS is true
PortMappings []ocicni.PortMapping `json:"portMappings,omitempty"` PortMappings []ocicni.PortMapping `json:"portMappings,omitempty"`
// NoCreateResolvConf indicates that resolv.conf should not be // UseImageResolvConf indicates that resolv.conf should not be
// bind-mounted inside the container. // bind-mounted inside the container.
// Conflicts with DNSServer, DNSSearch, DNSOption. // Conflicts with DNSServer, DNSSearch, DNSOption.
NoCreateResolvConf bool UseImageResolvConf bool
// DNS servers to use in container resolv.conf // DNS servers to use in container resolv.conf
// Will override servers in host resolv if set // Will override servers in host resolv if set
DNSServer []net.IP `json:"dnsServer,omitempty"` DNSServer []net.IP `json:"dnsServer,omitempty"`
@ -306,10 +306,10 @@ type ContainerConfig struct {
// DNS options to be set in container resolv.conf // DNS options to be set in container resolv.conf
// With override options in host resolv if set // With override options in host resolv if set
DNSOption []string `json:"dnsOption,omitempty"` DNSOption []string `json:"dnsOption,omitempty"`
// NoCreateHosts indicates that /etc/hosts should not be // UseImageHosts indicates that /etc/hosts should not be
// bind-mounted inside the container. // bind-mounted inside the container.
// Conflicts with HostAdd. // Conflicts with HostAdd.
NoCreateHosts bool UseImageHosts bool
// Hosts to add in container // Hosts to add in container
// Will be appended to host's host file // Will be appended to host's host file
HostAdd []string `json:"hostsAdd,omitempty"` HostAdd []string `json:"hostsAdd,omitempty"`

View File

@ -703,7 +703,7 @@ func (c *Container) makeBindMounts() error {
} }
} }
if c.config.NetNsCtr != "" && (!c.config.NoCreateHosts || !c.config.NoCreateResolvConf) { if c.config.NetNsCtr != "" && (!c.config.UseImageResolvConf || !c.config.UseImageHosts) {
// We share a net namespace. // We share a net namespace.
// We want /etc/resolv.conf and /etc/hosts from the // We want /etc/resolv.conf and /etc/hosts from the
// other container. Unless we're not creating both of // other container. Unless we're not creating both of
@ -719,7 +719,7 @@ func (c *Container) makeBindMounts() error {
return errors.Wrapf(err, "error fetching bind mounts from dependency %s of container %s", depCtr.ID(), c.ID()) return errors.Wrapf(err, "error fetching bind mounts from dependency %s of container %s", depCtr.ID(), c.ID())
} }
if !c.config.NoCreateResolvConf { if !c.config.UseImageResolvConf {
// The other container may not have a resolv.conf or /etc/hosts // The other container may not have a resolv.conf or /etc/hosts
// If it doesn't, don't copy them // If it doesn't, don't copy them
resolvPath, exists := bindMounts["/etc/resolv.conf"] resolvPath, exists := bindMounts["/etc/resolv.conf"]
@ -728,7 +728,7 @@ func (c *Container) makeBindMounts() error {
} }
} }
if !c.config.NoCreateHosts { if !c.config.UseImageHosts {
// check if dependency container has an /etc/hosts file // check if dependency container has an /etc/hosts file
hostsPath, exists := bindMounts["/etc/hosts"] hostsPath, exists := bindMounts["/etc/hosts"]
if !exists { if !exists {
@ -751,7 +751,7 @@ func (c *Container) makeBindMounts() error {
c.state.BindMounts["/etc/hosts"] = hostsPath c.state.BindMounts["/etc/hosts"] = hostsPath
} }
} else { } else {
if !c.config.NoCreateResolvConf { if !c.config.UseImageResolvConf {
newResolv, err := c.generateResolvConf() newResolv, err := c.generateResolvConf()
if err != nil { if err != nil {
return errors.Wrapf(err, "error creating resolv.conf for container %s", c.ID()) return errors.Wrapf(err, "error creating resolv.conf for container %s", c.ID())
@ -759,7 +759,7 @@ func (c *Container) makeBindMounts() error {
c.state.BindMounts["/etc/resolv.conf"] = newResolv c.state.BindMounts["/etc/resolv.conf"] = newResolv
} }
if !c.config.NoCreateHosts { if !c.config.UseImageHosts {
newHosts, err := c.generateHosts("/etc/hosts") newHosts, err := c.generateHosts("/etc/hosts")
if err != nil { if err != nil {
return errors.Wrapf(err, "error creating hosts file for container %s", c.ID()) return errors.Wrapf(err, "error creating hosts file for container %s", c.ID())

View File

@ -997,7 +997,7 @@ func WithDNSSearch(searchDomains []string) CtrCreateOption {
if ctr.valid { if ctr.valid {
return ErrCtrFinalized return ErrCtrFinalized
} }
if ctr.config.NoCreateResolvConf { if ctr.config.UseImageResolvConf {
return errors.Wrapf(ErrInvalidArg, "cannot add DNS search domains if container will not create /etc/resolv.conf") return errors.Wrapf(ErrInvalidArg, "cannot add DNS search domains if container will not create /etc/resolv.conf")
} }
ctr.config.DNSSearch = searchDomains ctr.config.DNSSearch = searchDomains
@ -1011,7 +1011,7 @@ func WithDNS(dnsServers []string) CtrCreateOption {
if ctr.valid { if ctr.valid {
return ErrCtrFinalized return ErrCtrFinalized
} }
if ctr.config.NoCreateResolvConf { if ctr.config.UseImageResolvConf {
return errors.Wrapf(ErrInvalidArg, "cannot add DNS servers if container will not create /etc/resolv.conf") return errors.Wrapf(ErrInvalidArg, "cannot add DNS servers if container will not create /etc/resolv.conf")
} }
var dns []net.IP var dns []net.IP
@ -1033,7 +1033,7 @@ func WithDNSOption(dnsOptions []string) CtrCreateOption {
if ctr.valid { if ctr.valid {
return ErrCtrFinalized return ErrCtrFinalized
} }
if ctr.config.NoCreateResolvConf { if ctr.config.UseImageResolvConf {
return errors.Wrapf(ErrInvalidArg, "cannot add DNS options if container will not create /etc/resolv.conf") return errors.Wrapf(ErrInvalidArg, "cannot add DNS options if container will not create /etc/resolv.conf")
} }
ctr.config.DNSOption = dnsOptions ctr.config.DNSOption = dnsOptions
@ -1048,7 +1048,7 @@ func WithHosts(hosts []string) CtrCreateOption {
return ErrCtrFinalized return ErrCtrFinalized
} }
if ctr.config.NoCreateHosts { if ctr.config.UseImageHosts {
return errors.Wrapf(ErrInvalidArg, "cannot add hosts if container will not create /etc/hosts") return errors.Wrapf(ErrInvalidArg, "cannot add hosts if container will not create /etc/hosts")
} }
@ -1198,9 +1198,9 @@ func WithCtrNamespace(ns string) CtrCreateOption {
} }
} }
// WithNoCreateResolvConf tells the container not to bind-mount resolv.conf in. // WithUseImageResolvConf tells the container not to bind-mount resolv.conf in.
// This conflicts with other DNS-related options. // This conflicts with other DNS-related options.
func WithNoCreateResolvConf() CtrCreateOption { func WithUseImageResolvConf() CtrCreateOption {
return func(ctr *Container) error { return func(ctr *Container) error {
if ctr.valid { if ctr.valid {
return ErrCtrFinalized return ErrCtrFinalized
@ -1212,15 +1212,15 @@ func WithNoCreateResolvConf() CtrCreateOption {
return errors.Wrapf(ErrInvalidArg, "not creating resolv.conf conflicts with DNS options") return errors.Wrapf(ErrInvalidArg, "not creating resolv.conf conflicts with DNS options")
} }
ctr.config.NoCreateResolvConf = true ctr.config.UseImageResolvConf = true
return nil return nil
} }
} }
// WithNoCreateHosts tells the container not to bind-mount /etc/hosts in. // WithUseImageHosts tells the container not to bind-mount /etc/hosts in.
// This conflicts with WithHosts(). // This conflicts with WithHosts().
func WithNoCreateHosts() CtrCreateOption { func WithUseImageHosts() CtrCreateOption {
return func(ctr *Container) error { return func(ctr *Container) error {
if ctr.valid { if ctr.valid {
return ErrCtrFinalized return ErrCtrFinalized
@ -1230,7 +1230,7 @@ func WithNoCreateHosts() CtrCreateOption {
return errors.Wrapf(ErrInvalidArg, "not creating /etc/hosts conflicts with adding to the hosts file") return errors.Wrapf(ErrInvalidArg, "not creating /etc/hosts conflicts with adding to the hosts file")
} }
ctr.config.NoCreateHosts = true ctr.config.UseImageHosts = true
return nil return nil
} }

View File

@ -506,8 +506,8 @@ func (c *CreateConfig) GetContainerCreateOptions(runtime *libpod.Runtime, pod *l
options = append(options, libpod.WithDNSSearch(c.DNSSearch)) options = append(options, libpod.WithDNSSearch(c.DNSSearch))
} }
if len(c.DNSServers) > 0 { if len(c.DNSServers) > 0 {
if len(c.DNSServers) == 1 && c.DNSServers[0] == "none" { if len(c.DNSServers) == 1 && strings.ToLower(c.DNSServers[0]) == "none" {
options = append(options, libpod.WithNoCreateResolvConf()) options = append(options, libpod.WithUseImageResolvConf())
} else { } else {
options = append(options, libpod.WithDNS(c.DNSServers)) options = append(options, libpod.WithDNS(c.DNSServers))
} }
@ -516,7 +516,7 @@ func (c *CreateConfig) GetContainerCreateOptions(runtime *libpod.Runtime, pod *l
options = append(options, libpod.WithDNSOption(c.DNSOpt)) options = append(options, libpod.WithDNSOption(c.DNSOpt))
} }
if c.NoHosts { if c.NoHosts {
options = append(options, libpod.WithNoCreateHosts()) options = append(options, libpod.WithUseImageHosts())
} }
if len(c.HostAdd) > 0 && !c.NoHosts { if len(c.HostAdd) > 0 && !c.NoHosts {
options = append(options, libpod.WithHosts(c.HostAdd)) options = append(options, libpod.WithHosts(c.HostAdd))