Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

structs: namespace port validation by host_network #9946

Merged
merged 4 commits into from
Feb 11, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,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)]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was this line intended to be removed?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm no that might have came from a rebase. Fixing up!


Expand Down
21 changes: 20 additions & 1 deletion nomad/structs/structs.go
Original file line number Diff line number Diff line change
Expand Up @@ -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...) {
Expand All @@ -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)
Expand All @@ -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
}
}

Expand All @@ -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)
Expand All @@ -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
}
}
}
Expand Down
87 changes: 87 additions & 0 deletions nomad/structs/structs_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down