Improve invalid port error message for services

Related to #3681

If a user specifies an invalid port *label* when using
address_mode=driver they'll get an error message about the label being
an invalid number which is very confusing.

I also added a bunch of testing around Service.AddressMode validation
since I was concerned by the linked issue that there were cases I was
missing. Unfortunately when address_mode=driver is used there's only so
much validation that can be done as structs/structs.go validation never
peeks into the driver config which would be needed to verify the port
labels/map.

command/agent/consul/client.go

@@ -1145,10 +1145,13 @@ func getAddress(addrMode, portLabel string, networks structs.Networks, driverNet
 		// If port isn't a label, try to parse it as a literal port number
 		port, err := strconv.Atoi(portLabel)
 		if err != nil {
-			return "", 0, fmt.Errorf("invalid port %q: %v", portLabel, err)
+			// Don't include Atoi error message as user likely
+			// never intended it to be a numeric and it creates a
+			// confusing error message
+			return "", 0, fmt.Errorf("invalid port label %q: port labels in driver address_mode must be numeric or in the driver's port map", portLabel)
 		}
 		if port <= 0 {
-			return "", 0, fmt.Errorf("invalid port: %q: port 0 is invalid", portLabel)
+			return "", 0, fmt.Errorf("invalid port: %q: port must be >0", portLabel)
 		}
 
 		return driverNet.IP, port, nil

command/agent/consul/unit_test.go

@@ -1468,6 +1468,7 @@ func TestGetAddress(t *testing.T) {
 		ExpectedPort int
 		ExpectedErr  string
 	}{
+		// Valid Configurations
 		{
 			Name:      "ExampleService",
 			Mode:      structs.AddressModeAuto,
@@ -1529,6 +1530,8 @@ func TestGetAddress(t *testing.T) {
 			ExpectedIP:   "10.1.2.3",
 			ExpectedPort: 7890,
 		},
+
+		// Invalid Configurations
 		{
 			Name:        "DriverWithoutNetwork",
 			Mode:        structs.AddressModeDriver,

nomad/structs/structs_test.go

@@ -1178,6 +1178,122 @@ func TestTask_Validate_Services(t *testing.T) {
 	}
 }
 
+func TestTask_Validate_Service_AddressMode_Ok(t *testing.T) {
+	ephemeralDisk := DefaultEphemeralDisk()
+	getTask := func(s *Service) *Task {
+		task := &Task{
+			Name:      "web",
+			Driver:    "docker",
+			Resources: DefaultResources(),
+			Services:  []*Service{s},
+			LogConfig: DefaultLogConfig(),
+		}
+		task.Resources.Networks = []*NetworkResource{
+			{
+				MBits: 10,
+				DynamicPorts: []Port{
+					{
+						Label: "http",
+						Value: 80,
+					},
+				},
+			},
+		}
+		return task
+	}
+
+	cases := []*Service{
+		{
+			// https://github.com/hashicorp/nomad/issues/3681#issuecomment-357274177
+			Name:        "DriverModeWithLabel",
+			PortLabel:   "http",
+			AddressMode: AddressModeDriver,
+		},
+		{
+			Name:        "DriverModeWithPort",
+			PortLabel:   "80",
+			AddressMode: AddressModeDriver,
+		},
+		{
+			Name:        "HostModeWithLabel",
+			PortLabel:   "http",
+			AddressMode: AddressModeHost,
+		},
+		{
+			Name:        "HostModeWithoutLabel",
+			AddressMode: AddressModeHost,
+		},
+		{
+			Name:        "DriverModeWithoutLabel",
+			AddressMode: AddressModeDriver,
+		},
+	}
+
+	for _, service := range cases {
+		task := getTask(service)
+		t.Run(service.Name, func(t *testing.T) {
+			if err := task.Validate(ephemeralDisk); err != nil {
+				t.Fatalf("unexpected err: %v", err)
+			}
+		})
+	}
+}
+
+func TestTask_Validate_Service_AddressMode_Bad(t *testing.T) {
+	ephemeralDisk := DefaultEphemeralDisk()
+	getTask := func(s *Service) *Task {
+		task := &Task{
+			Name:      "web",
+			Driver:    "docker",
+			Resources: DefaultResources(),
+			Services:  []*Service{s},
+			LogConfig: DefaultLogConfig(),
+		}
+		task.Resources.Networks = []*NetworkResource{
+			{
+				MBits: 10,
+				DynamicPorts: []Port{
+					{
+						Label: "http",
+						Value: 80,
+					},
+				},
+			},
+		}
+		return task
+	}
+
+	cases := []*Service{
+		{
+			// https://github.com/hashicorp/nomad/issues/3681#issuecomment-357274177
+			Name:        "DriverModeWithLabel",
+			PortLabel:   "asdf",
+			AddressMode: AddressModeDriver,
+		},
+		{
+			Name:        "HostModeWithLabel",
+			PortLabel:   "asdf",
+			AddressMode: AddressModeHost,
+		},
+		{
+			Name:        "HostModeWithPort",
+			PortLabel:   "80",
+			AddressMode: AddressModeHost,
+		},
+	}
+
+	for _, service := range cases {
+		task := getTask(service)
+		t.Run(service.Name, func(t *testing.T) {
+			err := task.Validate(ephemeralDisk)
+			if err == nil {
+				t.Fatalf("expected an error")
+			}
+			//t.Logf("err: %v", err)
+		})
+	}
+}
+
 func TestTask_Validate_Service_Check(t *testing.T) {
 
 	invalidCheck := ServiceCheck{