Merge pull request #8743 from hashicorp/f-task_network_warning

Validate and document 0.12 mbits/network deprecations

CHANGELOG.md

@@ -47,6 +47,7 @@ BUG FIXES:
  * api: Fixed a bug where the event stream client didn't pass the index query parameters [[GH-9419](https://github.com/hashicorp/nomad/issues/9419)]
  * core: Fixed a bug where ACL handling prevented cross-namespace allocation listing [[GH-9278](https://github.com/hashicorp/nomad/issues/9278)]
  * core: Fixed a bug where AllocatedResources contained increasingly duplicated ports [[GH-9368](https://github.com/hashicorp/nomad/issues/9368)]
+ * core: Fixed a bug where group level network ports not usable by task resource network stanza [[GH-8780](https://github.com/hashicorp/nomad/issues/8780)]
  * core: Fixed a bug where scaling policy filtering would ignore type query if job query was present [[GH-9312](https://github.com/hashicorp/nomad/issues/9312)]
  * core: Fixed a bug where a request to scale a job would fail if the job was not in the default namespace. [[GH-9296](https://github.com/hashicorp/nomad/pull/9296)]
  * core: Fixed a bug where blocking queries would not include the query's maximum wait time when calculating whether it was safe to retry. [[GH-8921](https://github.com/hashicorp/nomad/issues/8921)]

GNUmakefile

@@ -218,12 +218,16 @@ check: ## Lint the source code
 	@cd ./api && if go list --test -f '{{ join .Deps "\n" }}' . | grep github.com/hashicorp/nomad/ | grep -v -e /vendor/ -e /nomad/api/ -e nomad/api.test; then echo "  /api package depends the ^^ above internal nomad packages.  Remove such dependency"; exit 1; fi
 
 	@echo "==> Checking Go mod.."
-	@GO111MODULE=on go mod tidy
+	@GO111MODULE=on $(MAKE) sync
 	@if (git status --porcelain | grep -Eq "go\.(mod|sum)"); then \
 		echo go.mod or go.sum needs updating; \
 		git --no-pager diff go.mod; \
 		git --no-pager diff go.sum; \
 		exit 1; fi
+	@if (git status --porcelain | grep -Eq "vendor/github.com/hashicorp/nomad/.*\.go"); then \
+		echo "nomad go submodules are out of sync, try 'make sync':"; \
+		git status -s | grep -E "vendor/github.com/hashicorp/nomad/.*\.go"; \
+		exit 1; fi
 
 	@echo "==> Check raft util msg type mapping are in-sync..."
 	@go generate ./helper/raftutil/

api/resources.go

@@ -30,9 +30,6 @@ func (r *Resources) Canonicalize() {
 	if r.MemoryMB == nil {
 		r.MemoryMB = defaultResources.MemoryMB
 	}
-	for _, n := range r.Networks {
-		n.Canonicalize()
-	}
 	for _, d := range r.Devices {
 		d.Canonicalize()
 	}
@@ -103,19 +100,31 @@ type NetworkResource struct {
 	Device        string     `hcl:"device,optional"`
 	CIDR          string     `hcl:"cidr,optional"`
 	IP            string     `hcl:"ip,optional"`
-	MBits         *int       `hcl:"mbits,optional"`
 	DNS           *DNSConfig `hcl:"dns,block"`
 	ReservedPorts []Port     `hcl:"reserved_ports,block"`
 	DynamicPorts  []Port     `hcl:"port,block"`
+
+	// COMPAT(0.13)
+	// XXX Deprecated. Please do not use. The field will be removed in Nomad
+	// 0.13 and is only being kept to allow any references to be removed before
+	// then.
+	MBits *int `hcl:"mbits,optional"`
 }
 
-func (n *NetworkResource) Canonicalize() {
-	// COMPAT(0.12) MBits is deprecated but this should not be removed
-	// until MBits is fully removed. Removing this *without* fully removing
-	// MBits would cause unnecessary job diffs and destructive updates.
-	if n.MBits == nil {
-		n.MBits = intToPtr(10)
+// COMPAT(0.13)
+// XXX Deprecated. Please do not use. The method will be removed in Nomad
+// 0.13 and is only being kept to allow any references to be removed before
+// then.
+func (n *NetworkResource) Megabits() int {
+	if n == nil || n.MBits == nil {
+		return 0
 	}
+	return *n.MBits
+}
+
+func (n *NetworkResource) Canonicalize() {
+	// COMPAT(0.13)
+	// Noop to maintain backwards compatibility
 }
 
 func (n *NetworkResource) HasPorts() bool {

command/agent/job_endpoint.go

@@ -1211,7 +1211,7 @@ func ApiNetworkResourceToStructs(in []*api.NetworkResource) []*structs.NetworkRe
 			Mode:  nw.Mode,
 			CIDR:  nw.CIDR,
 			IP:    nw.IP,
-			MBits: *nw.MBits,
+			MBits: nw.Megabits(),
 		}
 
 		if nw.DNS != nil {

command/agent/testingutils_test.go

@@ -37,6 +37,12 @@ func MockJob() *api.Job {
 					Delay:    helper.TimeToPtr(1 * time.Minute),
 					Mode:     helper.StringToPtr("delay"),
 				},
+				Networks: []*api.NetworkResource{
+					{
+						Mode:         "host",
+						DynamicPorts: []api.Port{{Label: "http"}, {Label: "admin"}},
+					},
+				},
 				Tasks: []*api.Task{
 					{
 						Name:   "web",
@@ -72,12 +78,6 @@ func MockJob() *api.Job {
 						Resources: &api.Resources{
 							CPU:      helper.IntToPtr(500),
 							MemoryMB: helper.IntToPtr(256),
-							Networks: []*api.NetworkResource{
-								{
-									MBits:        helper.IntToPtr(50),
-									DynamicPorts: []api.Port{{Label: "http"}, {Label: "admin"}},
-								},
-							},
 						},
 						Meta: map[string]string{
 							"foo": "bar",

drivers/docker/driver.go

@@ -1095,6 +1095,9 @@ func (d *Driver) createContainerConfig(task *drivers.TaskConfig, driverConfig *T
 
 	default:
 		if len(driverConfig.PortMap) > 0 {
+			if task.Resources.Ports != nil {
+				return c, fmt.Errorf("'port_map' cannot map group network ports, use 'ports' instead")
+			}
 			return c, fmt.Errorf("Trying to map ports but no network interface is available")
 		}
 	}

e2e/consul/input/canary_tags.nomad

@@ -9,6 +9,10 @@ job "consul_canary_test" {
   group "consul_canary_test" {
     count = 2
 
+    network {
+      port "db" {}
+    }
+
     task "consul_canary_test" {
       driver = "docker"
 
@@ -25,10 +29,6 @@ job "consul_canary_test" {
       resources {
         cpu    = 100
         memory = 100
-
-        network {
-          port "db" {}
-        }
       }
 
       service {

e2e/consul/input/consul_example.nomad

@@ -26,6 +26,10 @@ job "consul-example" {
   group "cache" {
     count = 3
 
+    network {
+      port "db" {}
+    }
+
     restart {
       attempts = 2
       interval = "30m"
@@ -45,18 +49,12 @@ job "consul-example" {
         command = "nc"
         args    = ["-ll", "-p", "1234", "-e", "/bin/cat"]
 
-        port_map {
-          db = 1234
-        }
+        ports = ["db"]
       }
 
       resources {
         cpu    = 100
         memory = 100
-
-        network {
-          port "db" {}
-        }
       }
 
       service {

e2e/metrics/input/fabio.nomad

@@ -8,6 +8,15 @@ job "fabio" {
   }
 
   group "fabio" {
+    network {
+      port "lb" {
+        static = 9999
+      }
+
+      port "ui" {
+        static = 9998
+      }
+    }
     task "fabio" {
       driver = "docker"
 
@@ -19,18 +28,6 @@ job "fabio" {
       resources {
         cpu    = 100
         memory = 64
-
-        network {
-          mbits = 20
-
-          port "lb" {
-            static = 9999
-          }
-
-          port "ui" {
-            static = 9998
-          }
-        }
       }
     }
   }

e2e/metrics/input/helloworld.nomad

@@ -9,6 +9,10 @@ job "helloworld" {
   group "hello" {
     count = 3
 
+    network {
+      port "web" {}
+    }
+
     task "hello" {
       driver = "raw_exec"
 
@@ -25,11 +29,6 @@ job "helloworld" {
       resources {
         cpu    = 500
         memory = 256
-
-        network {
-          mbits = 10
-          port "web" {}
-        }
       }
 
       service {

e2e/metrics/input/prometheus.nomad

@@ -21,6 +21,12 @@ job "prometheus" {
       size = 300
     }
 
+    network {
+      port "prometheus_ui" {
+        to = 9090
+      }
+    }
+
     task "prometheus" {
       template {
         change_mode = "noop"
@@ -61,16 +67,7 @@ EOH
           "local/prometheus.yml:/etc/prometheus/prometheus.yml",
         ]
 
-        port_map {
-          prometheus_ui = 9090
-        }
-      }
-
-      resources {
-        network {
-          mbits = 10
-          port "prometheus_ui" {}
-        }
+        ports = ["prometheus_ui"]
       }
 
       service {

e2e/metrics/input/simpleweb.nomad

@@ -8,25 +8,23 @@ job "simpleweb" {
   }
 
   group "simpleweb" {
+    network {
+      port "http" {
+        to = 8080
+      }
+    }
     task "simpleweb" {
       driver = "docker"
 
       config {
         image = "nginx:latest"
 
-        port_map {
-          http = 8080
-        }
+        ports = ["http"]
       }
 
       resources {
         cpu    = 256
         memory = 128
-
-        network {
-          mbits = 1
-          port "http" {}
-        }
       }
 
       // TODO(tgross): this isn't passing health checks

nomad/job_endpoint_test.go

@@ -183,6 +183,7 @@ func TestJobEndpoint_Register_Connect(t *testing.T) {
 
 	// Create the register request
 	job := mock.Job()
+	job.TaskGroups[0].Tasks[0].Services = nil
 	job.TaskGroups[0].Networks = structs.Networks{{
 		Mode: "bridge",
 	}}
@@ -453,6 +454,7 @@ func TestJobEndpoint_Register_ConnectExposeCheck(t *testing.T) {
 
 	// Setup the job we are going to register
 	job := mock.Job()
+	job.TaskGroups[0].Tasks[0].Services = nil
 	job.TaskGroups[0].Networks = structs.Networks{{
 		Mode: "bridge",
 		DynamicPorts: []structs.Port{{
@@ -571,6 +573,7 @@ func TestJobEndpoint_Register_ConnectWithSidecarTask(t *testing.T) {
 			Mode: "bridge",
 		},
 	}
+	job.TaskGroups[0].Tasks[0].Services = nil
 	job.TaskGroups[0].Services = []*structs.Service{
 		{
 			Name:      "backend",
@@ -665,11 +668,7 @@ func TestJobEndpoint_Register_Connect_AllowUnauthenticatedFalse(t *testing.T) {
 
 	// Create the register request
 	job := mock.Job()
-	job.TaskGroups[0].Networks = structs.Networks{
-		{
-			Mode: "bridge",
-		},
-	}
+	job.TaskGroups[0].Networks[0].Mode = "bridge"
 	job.TaskGroups[0].Services = []*structs.Service{
 		{
 			Name:      "service1", // matches consul.ExamplePolicyID1
@@ -5809,9 +5808,7 @@ func TestJobEndpoint_ValidateJob_ConsulConnect(t *testing.T) {
 
 		tg := j.TaskGroups[0]
 		tg.Services = tgServices
-		tg.Networks = structs.Networks{
-			{Mode: "bridge"},
-		}
+		tg.Networks[0].Mode = "bridge"
 
 		err := validateJob(j)
 		require.NoError(t, err)