Merge pull request #9946 from hashicorp/b-9477

structs: namespace port validation by host_network
hashicorp · Feb 11, 2021 · 2d4d468 · 2d4d468
2 parents 58a9480 + deb4261
commit 2d4d468
Show file tree

Hide file tree

Showing 3 changed files with 109 additions and 1 deletion.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -15,8 +15,10 @@ BUG FIXES:
  * consul/connect: Fixed a bug preventing more than one connect gateway per Nomad client [[GH-9849](https://github.com/hashicorp/nomad/pull/9849)]
  * drivers/docker: Fixed a bug preventing multiple ports to be mapped to the same container port [[GH-9951](https://github.com/hashicorp/nomad/issues/9951)]
  * driver/qemu: Fixed a bug where network namespaces were not supported for QEMU workloads [[GH-9861](https://github.com/hashicorp/nomad/pull/9861)]
+ * nomad/structs: Fixed a bug where static ports with the same value but different `host_network` were invalid [[GH-9946](https://github.com/hashicorp/nomad/issues/9946)]
  * scheduler: Fixed a bug where shared ports were not persisted during inplace updates for service jobs. [[GH-9830](https://github.com/hashicorp/nomad/issues/9830)]
  * scheduler: Fixed a bug where job statuses and summaries where duplicated and miscalculated when registering a job. [[GH-9768](https://github.com/hashicorp/nomad/issues/9768)]
+ * scheduler: Fixed a bug that caused the scheduler not to detect changes for `host_network` port field. [[GH-9973](https://github.com/hashicorp/nomad/issues/9973)]
  * scheduler (Enterprise): Fixed a bug where the deprecated network `mbits` field was being considered as part of quota enforcement. [[GH-9920](https://github.com/hashicorp/nomad/issues/9920)]
  * volumes: Fixed a bug where volume diffs were not displayed in the output of `nomad plan`. [[GH-9973](https://github.com/hashicorp/nomad/issues/9973)]
 

diff --git a/nomad/structs/structs.go b/nomad/structs/structs.go
@@ -6168,7 +6168,8 @@ func (tg *TaskGroup) Validate(j *Job) error {
 func (tg *TaskGroup) validateNetworks() error {
 	var mErr multierror.Error
 	portLabels := make(map[string]string)
-	staticPorts := make(map[int]string)
+	// host_network -> static port tracking
+	staticPortsIndex := make(map[string]map[int]string)
 
 	for _, net := range tg.Networks {
 		for _, port := range append(net.ReservedPorts, net.DynamicPorts...) {
@@ -6179,6 +6180,14 @@ func (tg *TaskGroup) validateNetworks() error {
 			}
 
 			if port.Value != 0 {
+				hostNetwork := port.HostNetwork
+				if hostNetwork == "" {
+					hostNetwork = "default"
+				}
+				staticPorts, ok := staticPortsIndex[hostNetwork]
+				if !ok {
+					staticPorts = make(map[int]string)
+				}
 				// static port
 				if other, ok := staticPorts[port.Value]; ok {
 					err := fmt.Errorf("Static port %d already reserved by %s", port.Value, other)
@@ -6188,6 +6197,7 @@ func (tg *TaskGroup) validateNetworks() error {
 					mErr.Errors = append(mErr.Errors, err)
 				} else {
 					staticPorts[port.Value] = fmt.Sprintf("taskgroup network:%s", port.Label)
+					staticPortsIndex[hostNetwork] = staticPorts
 				}
 			}
 
@@ -6213,6 +6223,14 @@ func (tg *TaskGroup) validateNetworks() error {
 				}
 
 				if port.Value != 0 {
+					hostNetwork := port.HostNetwork
+					if hostNetwork == "" {
+						hostNetwork = "default"
+					}
+					staticPorts, ok := staticPortsIndex[hostNetwork]
+					if !ok {
+						staticPorts = make(map[int]string)
+					}
 					if other, ok := staticPorts[port.Value]; ok {
 						err := fmt.Errorf("Static port %d already reserved by %s", port.Value, other)
 						mErr.Errors = append(mErr.Errors, err)
@@ -6221,6 +6239,7 @@ func (tg *TaskGroup) validateNetworks() error {
 						mErr.Errors = append(mErr.Errors, err)
 					} else {
 						staticPorts[port.Value] = fmt.Sprintf("%s:%s", task.Name, port.Label)
+						staticPortsIndex[hostNetwork] = staticPorts
 					}
 				}
 			}

diff --git a/nomad/structs/structs_test.go b/nomad/structs/structs_test.go
@@ -1298,6 +1298,93 @@ func TestTaskGroupNetwork_Validate(t *testing.T) {
 			},
 			ErrContains: "greater than",
 		},
+		{
+			TG: &TaskGroup{
+				Name: "group-same-static-port-different-host_network",
+				Networks: Networks{
+					&NetworkResource{
+						ReservedPorts: []Port{
+							{
+								Label:       "net1_http",
+								Value:       80,
+								HostNetwork: "net1",
+							},
+							{
+								Label:       "net2_http",
+								Value:       80,
+								HostNetwork: "net2",
+							},
+						},
+					},
+				},
+			},
+		},
+		{
+			TG: &TaskGroup{
+				Name: "mixing-group-task-ports",
+				Networks: Networks{
+					&NetworkResource{
+						ReservedPorts: []Port{
+							{
+								Label: "group_http",
+								Value: 80,
+							},
+						},
+					},
+				},
+				Tasks: []*Task{
+					&Task{
+						Name: "task1",
+						Resources: &Resources{
+							Networks: Networks{
+								&NetworkResource{
+									ReservedPorts: []Port{
+										{
+											Label: "task_http",
+											Value: 80,
+										},
+									},
+								},
+							},
+						},
+					},
+				},
+			},
+			ErrContains: "already reserved by",
+		},
+		{
+			TG: &TaskGroup{
+				Name: "mixing-group-task-ports-with-host_network",
+				Networks: Networks{
+					&NetworkResource{
+						ReservedPorts: []Port{
+							{
+								Label:       "group_http",
+								Value:       80,
+								HostNetwork: "net1",
+							},
+						},
+					},
+				},
+				Tasks: []*Task{
+					&Task{
+						Name: "task1",
+						Resources: &Resources{
+							Networks: Networks{
+								&NetworkResource{
+									ReservedPorts: []Port{
+										{
+											Label: "task_http",
+											Value: 80,
+										},
+									},
+								},
+							},
+						},
+					},
+				},
+			},
+		},
 	}
 
 	for i := range cases {