Cleanup network option parsing

We were not handling the parsing of --ip.  This pr adds validation
checks and now will support the flag.

Move validation to the actual parsing of the network flags.

We should only parse the dns flags if the user changed them. We don't
want to pass default options if set in containers.conf to the server.
Potential for duplicating defaults.

Add support for --dns-opt flag passing

Begin handling of --network flag, although we don't have a way right now
to translate a string into a specgen.Namespace.

Signed-off-by: Daniel J Walsh <dwalsh@redhat.com>

cmd/podman/common/createparse.go

@@ -1,7 +1,6 @@
 package common
 
 import (
-	"github.com/containers/libpod/cmd/podman/parse"
 	"github.com/containers/libpod/pkg/util"
 	"github.com/pkg/errors"
 )
@@ -17,27 +16,7 @@ func (c *ContainerCLIOpts) validate() error {
 	if _, err := util.ValidatePullType(c.Pull); err != nil {
 		return err
 	}
-	// Verify the additional hosts are in correct format
-	for _, host := range c.Net.AddHosts {
-		if _, err := parse.ValidateExtraHost(host); err != nil {
-			return err
-		}
-	}
 
-	if dnsSearches := c.Net.DNSSearch; len(dnsSearches) > 0 {
-		// Validate domains are good
-		for _, dom := range dnsSearches {
-			if dom == "." {
-				if len(dnsSearches) > 1 {
-					return errors.Errorf("cannot pass additional search domains when also specifying '.'")
-				}
-				continue
-			}
-			if _, err := parse.ValidateDomain(dom); err != nil {
-				return err
-			}
-		}
-	}
 	var imageVolType = map[string]string{
 		"bind":   "",
 		"tmpfs":  "",

cmd/podman/common/netflags.go

@@ -3,7 +3,9 @@ package common
 import (
 	"net"
 
+	"github.com/containers/libpod/cmd/podman/parse"
 	"github.com/containers/libpod/pkg/domain/entities"
+	"github.com/pkg/errors"
 	"github.com/spf13/cobra"
 	"github.com/spf13/pflag"
 )
@@ -58,20 +60,60 @@ func NetFlagsToNetOptions(cmd *cobra.Command) (*entities.NetOptions, error) {
 	if err != nil {
 		return nil, err
 	}
-	servers, err := cmd.Flags().GetStringSlice("dns")
-	if err != nil {
-		return nil, err
-	}
-	for _, d := range servers {
-		if d == "none" {
-			opts.DNSHost = true
-			break
+	// Verify the additional hosts are in correct format
+	for _, host := range opts.AddHosts {
+		if _, err := parse.ValidateExtraHost(host); err != nil {
+			return nil, err
 		}
-		opts.DNSServers = append(opts.DNSServers, net.ParseIP(d))
 	}
-	opts.DNSSearch, err = cmd.Flags().GetStringSlice("dns-search")
-	if err != nil {
-		return nil, err
+
+	if cmd.Flags().Changed("dns") {
+		servers, err := cmd.Flags().GetStringSlice("dns")
+		if err != nil {
+			return nil, err
+		}
+		for _, d := range servers {
+			if d == "none" {
+				opts.UseImageResolvConf = true
+				if len(servers) > 1 {
+					return nil, errors.Errorf("%s is not allowed to be specified with other DNS ip addresses", d)
+				}
+				break
+			}
+			dns := net.ParseIP(d)
+			if dns == nil {
+				return nil, errors.Errorf("%s is not an ip address", d)
+			}
+			opts.DNSServers = append(opts.DNSServers, dns)
+		}
+	}
+
+	if cmd.Flags().Changed("dns-opt") {
+		options, err := cmd.Flags().GetStringSlice("dns-opt")
+		if err != nil {
+			return nil, err
+		}
+		opts.DNSOptions = options
+	}
+
+	if cmd.Flags().Changed("dns-search") {
+		dnsSearches, err := cmd.Flags().GetStringSlice("dns-search")
+		if err != nil {
+			return nil, err
+		}
+		// Validate domains are good
+		for _, dom := range dnsSearches {
+			if dom == "." {
+				if len(dnsSearches) > 1 {
+					return nil, errors.Errorf("cannot pass additional search domains when also specifying '.'")
+				}
+				continue
+			}
+			if _, err := parse.ValidateDomain(dom); err != nil {
+				return nil, err
+			}
+		}
+		opts.DNSSearch = dnsSearches
 	}
 
 	m, err := cmd.Flags().GetString("mac-address")
@@ -85,6 +127,7 @@ func NetFlagsToNetOptions(cmd *cobra.Command) (*entities.NetOptions, error) {
 		}
 		opts.StaticMAC = &mac
 	}
+
 	inputPorts, err := cmd.Flags().GetStringSlice("publish")
 	if err != nil {
 		return nil, err
@@ -95,6 +138,31 @@ func NetFlagsToNetOptions(cmd *cobra.Command) (*entities.NetOptions, error) {
 			return nil, err
 		}
 	}
+
+	ip, err := cmd.Flags().GetString("ip")
+	if err != nil {
+		return nil, err
+	}
+	if ip != "" {
+		staticIP := net.ParseIP(ip)
+		if staticIP == nil {
+			return nil, errors.Errorf("%s is not an ip address", ip)
+		}
+		opts.StaticIP = &staticIP
+	}
+
 	opts.NoHosts, err = cmd.Flags().GetBool("no-hosts")
+
+	if cmd.Flags().Changed("network") {
+		network, err := cmd.Flags().GetString("network")
+		if err != nil {
+			return nil, err
+		}
+
+		return nil, errors.Errorf("network %s is not yet supported", network)
+		// TODO How do I convert a string network to a Specgen.Namespace?
+		//		opts.Network = specgen.Namespace{NSMode: network}
+	}
+
 	return &opts, err
 }

cmd/podman/common/specgen.go

@@ -283,9 +283,12 @@ func FillOutSpecGen(s *specgen.SpecGenerator, c *ContainerCLIOpts, args []string
 	}
 	s.ShmSize = &shmSize
 	s.HostAdd = c.Net.AddHosts
-	s.DNSServer = c.Net.DNSServers
+	s.UseImageResolvConf = c.Net.UseImageResolvConf
+	s.DNSServers = c.Net.DNSServers
 	s.DNSSearch = c.Net.DNSSearch
-	s.DNSOption = c.Net.DNSOptions
+	s.DNSOptions = c.Net.DNSOptions
+	s.StaticIP = c.Net.StaticIP
+	s.StaticMAC = c.Net.StaticMAC
 
 	// deferred, must be added on libpod side
 	//var ImageVolumes map[string]struct{}

pkg/domain/entities/pods.go

@@ -134,7 +134,7 @@ func (p PodCreateOptions) ToPodSpecGen(s *specgen.PodSpecGenerator) {
 	s.StaticMAC = p.Net.StaticMAC
 	s.PortMappings = p.Net.PublishPorts
 	s.CNINetworks = p.Net.CNINetworks
-	if p.Net.DNSHost {
+	if p.Net.UseImageResolvConf {
 		s.NoManageResolvConf = true
 	}
 	s.DNSServer = p.Net.DNSServers

pkg/domain/entities/types.go

@@ -32,17 +32,17 @@ type VolumeDeleteReport struct{ Report }
 // NetOptions reflect the shared network options between
 // pods and containers
 type NetOptions struct {
-	AddHosts     []string
-	CNINetworks  []string
-	DNSHost      bool
-	DNSOptions   []string
-	DNSSearch    []string
-	DNSServers   []net.IP
-	Network      specgen.Namespace
-	NoHosts      bool
-	PublishPorts []ocicni.PortMapping
-	StaticIP     *net.IP
-	StaticMAC    *net.HardwareAddr
+	AddHosts           []string
+	CNINetworks        []string
+	UseImageResolvConf bool
+	DNSOptions         []string
+	DNSSearch          []string
+	DNSServers         []net.IP
+	Network            specgen.Namespace
+	NoHosts            bool
+	PublishPorts       []ocicni.PortMapping
+	StaticIP           *net.IP
+	StaticMAC          *net.HardwareAddr
 }
 
 // All CLI inspect commands and inspect sub-commands use the same options

pkg/specgen/container_validate.go

@@ -91,13 +91,13 @@ func (s *SpecGenerator) Validate() error {
 	}
 	// useimageresolveconf conflicts with dnsserver, dnssearch, dnsoption
 	if s.UseImageResolvConf {
-		if len(s.DNSServer) > 0 {
+		if len(s.DNSServers) > 0 {
 			return exclusiveOptions("UseImageResolvConf", "DNSServer")
 		}
 		if len(s.DNSSearch) > 0 {
 			return exclusiveOptions("UseImageResolvConf", "DNSSearch")
 		}
-		if len(s.DNSOption) > 0 {
+		if len(s.DNSOptions) > 0 {
 			return exclusiveOptions("UseImageResolvConf", "DNSOption")
 		}
 	}

pkg/specgen/generate/namespaces.go

@@ -147,20 +147,19 @@ func GenerateNamespaceContainerOpts(s *specgen.SpecGenerator, rt *libpod.Runtime
 	if len(s.DNSSearch) > 0 {
 		options = append(options, libpod.WithDNSSearch(s.DNSSearch))
 	}
-	if len(s.DNSServer) > 0 {
-		// TODO I'm not sure how we are going to handle this given the input
-		if len(s.DNSServer) == 1 { //&& strings.ToLower(s.DNSServer[0].) == "none" {
-			options = append(options, libpod.WithUseImageResolvConf())
-		} else {
-			var dnsServers []string
-			for _, d := range s.DNSServer {
-				dnsServers = append(dnsServers, d.String())
-			}
-			options = append(options, libpod.WithDNS(dnsServers))
+
+	if s.UseImageResolvConf {
+		options = append(options, libpod.WithUseImageResolvConf())
+	} else {
+		var dnsServers []string
+		for _, d := range s.DNSServers {
+			dnsServers = append(dnsServers, d.String())
 		}
+		options = append(options, libpod.WithDNS(dnsServers))
 	}
-	if len(s.DNSOption) > 0 {
-		options = append(options, libpod.WithDNSOption(s.DNSOption))
+
+	if len(s.DNSOptions) > 0 {
+		options = append(options, libpod.WithDNSOption(s.DNSOptions))
 	}
 	if s.StaticIP != nil {
 		options = append(options, libpod.WithStaticIP(*s.StaticIP))

pkg/specgen/specgen.go

@@ -319,24 +319,24 @@ type ContainerNetworkConfig struct {
 	// by Podman, but instead sourced from the image.
 	// Conflicts with DNSServer, DNSSearch, DNSOption.
 	UseImageResolvConf bool `json:"use_image_resolve_conf,omitempty"`
-	// DNSServer is a set of DNS servers that will be used in the
+	// DNSServers is a set of DNS servers that will be used in the
 	// container's resolv.conf, replacing the host's DNS Servers which are
 	// used by default.
 	// Conflicts with UseImageResolvConf.
 	// Optional.
-	DNSServer []net.IP `json:"dns_server,omitempty"`
+	DNSServers []net.IP `json:"dns_server,omitempty"`
 	// DNSSearch is a set of DNS search domains that will be used in the
 	// container's resolv.conf, replacing the host's DNS search domains
 	// which are used by default.
 	// Conflicts with UseImageResolvConf.
 	// Optional.
 	DNSSearch []string `json:"dns_search,omitempty"`
-	// DNSOption is a set of DNS options that will be used in the
+	// DNSOptions is a set of DNS options that will be used in the
 	// container's resolv.conf, replacing the host's DNS options which are
 	// used by default.
 	// Conflicts with UseImageResolvConf.
 	// Optional.
-	DNSOption []string `json:"dns_option,omitempty"`
+	DNSOptions []string `json:"dns_option,omitempty"`
 	// UseImageHosts indicates that /etc/hosts should not be managed by
 	// Podman, and instead sourced from the image.
 	// Conflicts with HostAdd.