From 7dbf0c4e3c435b430038d750fffc789cbd09c64b Mon Sep 17 00:00:00 2001
From: Michael Schurter <schmichael@hashicorp.com>
Date: Fri, 8 Dec 2017 16:20:20 -0800
Subject: [PATCH] Log missing mapped ports instead of failing

SDNs, overlays, etc often won't need to map ports.
---
 CHANGELOG.md              |  1 +
 client/driver/rkt.go      | 36 +++++++++++++++++++-----------------
 client/driver/rkt_test.go |  9 ++++++---
 3 files changed, 26 insertions(+), 20 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 68c17be6bc7..18fbc67ad0f 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -32,6 +32,7 @@ IMPROVEMENTS:
  * driver/docker: Adds support for `ulimit` and `sysctl` options [GH-3568]
  * driver/docker: Adds support for StopTimeout (set to the same value as
    kill_timeout [GH-3601]
+ * driver/rkt: Don't fail on unmapped ports [GH-3637]
  * driver/rkt: Add support for passing through user [GH-3612]
  * driver/qemu: Support graceful shutdowns on unix platforms [GH-3411]
  * template: Updated to consul template 0.19.4 [GH-3543]
diff --git a/client/driver/rkt.go b/client/driver/rkt.go
index 52c7c91f0a5..f9295b39b86 100644
--- a/client/driver/rkt.go
+++ b/client/driver/rkt.go
@@ -535,41 +535,43 @@ func (d *RktDriver) Start(ctx *ExecContext, task *structs.Task) (*StartResponse,
 	} else {
 		// TODO add support for more than one network
 		network := task.Resources.Networks[0]
+		unmappedPorts := make([]string, 0, len(network.ReservedPorts)+len(network.DynamicPorts))
 		for _, port := range network.ReservedPorts {
-			var containerPort string
-
 			mapped, ok := driverConfig.PortMap[port.Label]
 			if !ok {
-				// If the user doesn't have a mapped port using port_map, driver stops running container.
-				return nil, fmt.Errorf("port_map is not set. When you defined port in the resources, you need to configure port_map.")
+				unmappedPorts = append(unmappedPorts, port.Label)
+				continue
 			}
-			containerPort = mapped
 
 			hostPortStr := strconv.Itoa(port.Value)
 
-			d.logger.Printf("[DEBUG] driver.rkt: exposed port %s", containerPort)
+			d.logger.Printf("[DEBUG] driver.rkt: alloc %s task %s mapped reserved port %s:%s",
+				d.DriverContext.allocID, task.Name, mapped, hostPortStr)
+
 			// Add port option to rkt run arguments. rkt allows multiple port args
-			prepareArgs = append(prepareArgs, fmt.Sprintf("--port=%s:%s", containerPort, hostPortStr))
+			prepareArgs = append(prepareArgs, fmt.Sprintf("--port=%s:%s", mapped, hostPortStr))
 		}
 
 		for _, port := range network.DynamicPorts {
-			// By default we will map the allocated port 1:1 to the container
-			var containerPort string
-
-			if mapped, ok := driverConfig.PortMap[port.Label]; ok {
-				containerPort = mapped
-			} else {
-				// If the user doesn't have mapped a port using port_map, driver stops running container.
-				return nil, fmt.Errorf("port_map is not set. When you defined port in the resources, you need to configure port_map.")
+			mapped, ok := driverConfig.PortMap[port.Label]
+			if !ok {
+				unmappedPorts = append(unmappedPorts, port.Label)
+				continue
 			}
 
 			hostPortStr := strconv.Itoa(port.Value)
 
-			d.logger.Printf("[DEBUG] driver.rkt: exposed port %s", containerPort)
+			d.logger.Printf("[DEBUG] driver.rkt: alloc %s task %s mapped dynamic port %s:%s",
+				d.DriverContext.allocID, task.Name, mapped, hostPortStr)
+
 			// Add port option to rkt run arguments. rkt allows multiple port args
-			prepareArgs = append(prepareArgs, fmt.Sprintf("--port=%s:%s", containerPort, hostPortStr))
+			prepareArgs = append(prepareArgs, fmt.Sprintf("--port=%s:%s", mapped, hostPortStr))
 		}
 
+		if len(unmappedPorts) > 0 {
+			d.logger.Printf("[INFO] driver.rkt: alloc %s task %s did not map these ports: %s",
+				d.DriverContext.allocID, task.Name, strings.Join(unmappedPorts, ", "))
+		}
 	}
 
 	// If a user has been specified for the task, pass it through to the user
diff --git a/client/driver/rkt_test.go b/client/driver/rkt_test.go
index 3b4646dbfb3..08de654bf18 100644
--- a/client/driver/rkt_test.go
+++ b/client/driver/rkt_test.go
@@ -453,7 +453,7 @@ func TestRktTaskValidate(t *testing.T) {
 }
 
 // TODO: Port Mapping test should be ran with proper ACI image and test the port access.
-func TestRktDriver_PortsMapping(t *testing.T) {
+func TestRktDriver_PortMapping(t *testing.T) {
 	if !testutil.IsTravis() {
 		t.Parallel()
 	}
@@ -483,8 +483,11 @@ func TestRktDriver_PortsMapping(t *testing.T) {
 			CPU:      512,
 			Networks: []*structs.NetworkResource{
 				{
-					IP:            "127.0.0.1",
-					ReservedPorts: []structs.Port{{Label: "main", Value: 8080}},
+					IP: "127.0.0.1",
+					ReservedPorts: []structs.Port{
+						{Label: "main", Value: 8080},
+						{Label: "unmapped", Value: 9999},
+					},
 				},
 			},
 		},