From aa49df4fe4d65da8a52fe15df346f50bf6f9b5c0 Mon Sep 17 00:00:00 2001
From: Jose Diaz-Gonzalez <email@josediazgonzalez.com>
Date: Sun, 6 Nov 2022 18:30:05 -0500
Subject: [PATCH] fix: do not attempt to attach a network to a container that
 already has it

This avoids issues when scaling unrelated process types or if the user has specified the same network at different phases.

Closes #5352
---
 plugins/network/functions.go | 33 +++++++++++++++++++++++++
 tests/unit/network.bats      | 47 ++++++++++++++++++++++++++++++++++++
 2 files changed, 80 insertions(+)

diff --git a/plugins/network/functions.go b/plugins/network/functions.go
index 28badd79c..d1fd35830 100644
--- a/plugins/network/functions.go
+++ b/plugins/network/functions.go
@@ -13,6 +13,10 @@ import (
 
 // attachAppToNetwork attaches a container to a network
 func attachAppToNetwork(containerID string, networkName string, appName string, phase string, processType string) error {
+	if isContainerInNetwork(containerID, networkName) {
+		return nil
+	}
+
 	cmdParts := []string{
 		common.DockerBin(),
 		"network",
@@ -54,6 +58,35 @@ func attachAppToNetwork(containerID string, networkName string, appName string,
 	return nil
 }
 
+// isContainerInNetwork returns true if the container is already attached to the specified network
+func isContainerInNetwork(containerID string, networkName string) bool {
+	b, err := sh.Command(
+		common.DockerBin(),
+		"container",
+		"inspect",
+		"--format",
+		"{{range $net, $v := .NetworkSettings.Networks}}{{println $net}}{{end}}",
+		containerID,
+	).Output()
+	if err != nil {
+		common.LogVerboseQuiet(fmt.Sprintf("Error checking container networking status:%v", err.Error()))
+		return false
+	}
+
+	for _, line := range strings.Split(strings.TrimSpace(string(b[:])), "\n") {
+		network := strings.TrimSpace(line)
+		if network == "" {
+			continue
+		}
+
+		if network == networkName {
+			return true
+		}
+	}
+
+	return false
+}
+
 // isConflictingPropertyValue returns true if the other attach property has a conflicting value
 func isConflictingPropertyValue(appName string, property string, value string) bool {
 	if value == "" {
diff --git a/tests/unit/network.bats b/tests/unit/network.bats
index c8d503bb8..9a5abb6bd 100644
--- a/tests/unit/network.bats
+++ b/tests/unit/network.bats
@@ -242,6 +242,53 @@ teardown() {
   assert_success
 }
 
+@test "(network) dont re-attach to network" {
+  run /bin/bash -c "dokku network:create deploy-network"
+  echo "output: $output"
+  echo "status: $status"
+  assert_success
+
+  run /bin/bash -c "dokku network:set $TEST_APP attach-post-deploy deploy-network"
+  echo "output: $output"
+  echo "status: $status"
+  assert_success
+
+  run deploy_app dockerfile-procfile
+  echo "output: $output"
+  echo "status: $status"
+  assert_success
+
+  run /bin/bash -c "dokku ps:scale $TEST_APP worker=1"
+  echo "output: $output"
+  echo "status: $status"
+  assert_success
+
+  run /bin/bash -c "dokku ps:scale $TEST_APP web=3"
+  echo "output: $output"
+  echo "status: $status"
+  assert_success
+
+  run /bin/bash -c "docker container inspect $TEST_APP.web.1"
+  echo "output: $output"
+  echo "status: $status"
+  assert_success
+
+  run /bin/bash -c "docker container inspect $TEST_APP.web.2"
+  echo "output: $output"
+  echo "status: $status"
+  assert_success
+
+  run /bin/bash -c "docker container inspect $TEST_APP.web.3"
+  echo "output: $output"
+  echo "status: $status"
+  assert_success
+
+  run /bin/bash -c "docker container inspect $TEST_APP.worker.1"
+  echo "output: $output"
+  echo "status: $status"
+  assert_success
+}
+
 @test "(network) handle single-udp app" {
   run /bin/bash -c "dokku docker-options:add $TEST_APP -p 1194:1194"
   echo "output: $output"