fix: Make the reverse proxy connect and disconnect to and from the en…

…clave network idempotent (#2004) ## Description: This PR makes the reverse proxy network calls idempotent so the engine can be started when networking is already set up. This can happen when you restart the docker daemon. In addition, this PR appends the engine UUID to the reverse proxy container name to make it unique across runs. This is an approach similar to the engine naming scheme which is cleaner and safer. ## Is this change user facing? NO ## References (if applicable): #2005
kurtosis-tech · Jan 3, 2024 · 3cc68eb · 3cc68eb
1 parent 7de8faa
commit 3cc68eb
Show file tree

Hide file tree

Showing 11 changed files with 78 additions and 37 deletions.
diff --git a/...er-engine-lib/lib/backend_impls/docker/docker_kurtosis_backend/docker_kurtosis_backend.go b/...er-engine-lib/lib/backend_impls/docker/docker_kurtosis_backend/docker_kurtosis_backend.go
@@ -475,11 +475,12 @@ func (backend *DockerKurtosisBackend) DestroyLogsCollectorForEnclave(ctx context
 	return nil
 }
 
-func (backend *DockerKurtosisBackend) CreateReverseProxy(ctx context.Context) (*reverse_proxy.ReverseProxy, error) {
+func (backend *DockerKurtosisBackend) CreateReverseProxy(ctx context.Context, engineGuid engine.EngineGUID) (*reverse_proxy.ReverseProxy, error) {
 	reverseProxyContainer := traefik.NewTraefikReverseProxyContainer()
 
 	reverseProxy, _, err := reverse_proxy_functions.CreateReverseProxy(
 		ctx,
+		engineGuid,
 		reverseProxyContainer,
 		backend.dockerManager,
 		backend.objAttrsProvider,

diff --git a/...ne-lib/lib/backend_impls/docker/docker_kurtosis_backend/engine_functions/create_engine.go b/...ne-lib/lib/backend_impls/docker/docker_kurtosis_backend/engine_functions/create_engine.go
@@ -115,6 +115,7 @@ func CreateEngine(
 	reverseProxyContainer := traefik.NewTraefikReverseProxyContainer()
 	_, removeReverseProxyFunc, err := reverse_proxy_functions.CreateReverseProxy(
 		ctx,
+		engineGuid,
 		reverseProxyContainer,
 		dockerManager,
 		objAttrsProvider)

diff --git a/...kend_impls/docker/docker_kurtosis_backend/reverse_proxy_functions/create_reverse_proxy.go b/...kend_impls/docker/docker_kurtosis_backend/reverse_proxy_functions/create_reverse_proxy.go
@@ -2,10 +2,12 @@ package reverse_proxy_functions
 
 import (
 	"context"
+
 	"github.com/kurtosis-tech/kurtosis/container-engine-lib/lib/backend_impls/docker/docker_kurtosis_backend/shared_helpers"
 	"github.com/kurtosis-tech/kurtosis/container-engine-lib/lib/backend_impls/docker/docker_manager"
 	"github.com/kurtosis-tech/kurtosis/container-engine-lib/lib/backend_impls/docker/docker_manager/types"
 	"github.com/kurtosis-tech/kurtosis/container-engine-lib/lib/backend_impls/docker/object_attributes_provider"
+	"github.com/kurtosis-tech/kurtosis/container-engine-lib/lib/backend_interface/objects/engine"
 	"github.com/kurtosis-tech/kurtosis/container-engine-lib/lib/backend_interface/objects/reverse_proxy"
 	"github.com/kurtosis-tech/stacktrace"
 	"github.com/sirupsen/logrus"
@@ -18,6 +20,7 @@ const (
 // Create reverse proxy idempotently, if existing reverse proxy is found, then it is returned
 func CreateReverseProxy(
 	ctx context.Context,
+	engineGuid engine.EngineGUID,
 	reverseProxyContainer ReverseProxyContainer,
 	dockerManager *docker_manager.DockerManager,
 	objAttrsProvider object_attributes_provider.DockerObjectAttributesProvider,
@@ -59,6 +62,7 @@ func CreateReverseProxy(
 
 	containerId, containerLabels, removeReverseProxyContainerFunc, err := reverseProxyContainer.CreateAndStart(
 		ctx,
+		engineGuid,
 		defaultReverseProxyHttpPortNum,
 		defaultReverseProxyDashboardPortNum,
 		targetNetworkId,

diff --git a/...ackend/reverse_proxy_functions/implementations/traefik/traefik_reverse_proxy_container.go b/...ackend/reverse_proxy_functions/implementations/traefik/traefik_reverse_proxy_container.go
@@ -5,6 +5,7 @@ import (
 
 	"github.com/kurtosis-tech/kurtosis/container-engine-lib/lib/backend_impls/docker/docker_manager"
 	"github.com/kurtosis-tech/kurtosis/container-engine-lib/lib/backend_impls/docker/object_attributes_provider"
+	"github.com/kurtosis-tech/kurtosis/container-engine-lib/lib/backend_interface/objects/engine"
 	"github.com/kurtosis-tech/stacktrace"
 	"github.com/sirupsen/logrus"
 )
@@ -17,6 +18,7 @@ func NewTraefikReverseProxyContainer() *traefikReverseProxyContainer {
 
 func (traefikContainer *traefikReverseProxyContainer) CreateAndStart(
 	ctx context.Context,
+	engineGuid engine.EngineGUID,
 	httpPort uint16,
 	dashboardPort uint16,
 	targetNetworkId string,
@@ -25,7 +27,7 @@ func (traefikContainer *traefikReverseProxyContainer) CreateAndStart(
 ) (string, map[string]string, func(), error) {
 	traefikContainerConfigProviderObj := createTraefikContainerConfigProvider(httpPort, dashboardPort, targetNetworkId)
 
-	reverseProxyAttrs, err := objAttrsProvider.ForReverseProxy()
+	reverseProxyAttrs, err := objAttrsProvider.ForReverseProxy(engineGuid)
 	if err != nil {
 		return "", nil, nil, stacktrace.Propagate(err, "An error occurred getting the reverse proxy container attributes.")
 	}

diff --git a/...end_impls/docker/docker_kurtosis_backend/reverse_proxy_functions/network_reverse_proxy.go b/...end_impls/docker/docker_kurtosis_backend/reverse_proxy_functions/network_reverse_proxy.go
@@ -8,7 +8,6 @@ import (
 	"github.com/kurtosis-tech/kurtosis/container-engine-lib/lib/backend_impls/docker/object_attributes_provider/docker_label_key"
 	"github.com/kurtosis-tech/kurtosis/container-engine-lib/lib/backend_impls/docker/object_attributes_provider/label_value_consts"
 	"github.com/kurtosis-tech/stacktrace"
-	"github.com/sirupsen/logrus"
 )
 
 const (
@@ -20,31 +19,41 @@ var (
 )
 
 func ConnectReverseProxyToNetwork(ctx context.Context, dockerManager *docker_manager.DockerManager, networkId string) error {
-	_, maybeReverseProxyContainerId, err := getReverseProxyObjectAndContainerId(ctx, dockerManager)
+	maybeReverseProxyObject, maybeReverseProxyContainerId, err := getReverseProxyObjectAndContainerId(ctx, dockerManager)
 	if err != nil {
-		logrus.Warnf("Attempted to connect reverse proxy to a network but no reverse proxy container was found.")
-		return nil
+		return stacktrace.Propagate(err, "An error occurred while retrieving the reverse proxy object and container id")
+	}
+
+	if maybeReverseProxyObject == nil {
+		return stacktrace.Propagate(err, "An error occurred while connecting the reverse proxy to the enclave network '%v' because no reverse proxy was found", networkId)
 	}
 
-	if maybeReverseProxyContainerId == "" {
+	_, found := maybeReverseProxyObject.GetEnclaveNetworksIpAddress()[networkId]
+	if found {
+		// Do nothing if already connected
 		return nil
 	}
 
 	if err = dockerManager.ConnectContainerToNetwork(ctx, networkId, maybeReverseProxyContainerId, autoAssignIpAddressToReverseProxy, emptyAliasForReverseProxy); err != nil {
-		return stacktrace.Propagate(err, "An error occurred while connecting container '%v' to the enclave network '%v'", maybeReverseProxyContainerId, networkId)
+		return stacktrace.Propagate(err, "An error occurred while connecting the reverse proxy with container id '%v' to the enclave network '%v'", maybeReverseProxyContainerId, networkId)
 	}
 
 	return nil
 }
 
 func DisconnectReverseProxyFromNetwork(ctx context.Context, dockerManager *docker_manager.DockerManager, networkId string) error {
-	_, maybeReverseProxyContainerId, err := getReverseProxyObjectAndContainerId(ctx, dockerManager)
+	maybeReverseProxyObject, maybeReverseProxyContainerId, err := getReverseProxyObjectAndContainerId(ctx, dockerManager)
 	if err != nil {
-		logrus.Warnf("Attempted to disconnect reverse proxy from a network but no reverse proxy container was found.")
-		return nil
+		return stacktrace.Propagate(err, "An error occurred while retrieving the reverse proxy object and container id")
+	}
+
+	if maybeReverseProxyObject == nil {
+		return stacktrace.Propagate(err, "An error occurred while disconnecting the reverse proxy from the enclave network '%v' because no reverse proxy was found", networkId)
 	}
 
-	if maybeReverseProxyContainerId == "" {
+	_, found := maybeReverseProxyObject.GetEnclaveNetworksIpAddress()[networkId]
+	if !found {
+		// Do nothing if already disconnected
 		return nil
 	}
 

diff --git a/...d_impls/docker/docker_kurtosis_backend/reverse_proxy_functions/reverse_proxy_container.go b/...d_impls/docker/docker_kurtosis_backend/reverse_proxy_functions/reverse_proxy_container.go
@@ -2,13 +2,16 @@ package reverse_proxy_functions
 
 import (
 	"context"
+
 	"github.com/kurtosis-tech/kurtosis/container-engine-lib/lib/backend_impls/docker/docker_manager"
 	"github.com/kurtosis-tech/kurtosis/container-engine-lib/lib/backend_impls/docker/object_attributes_provider"
+	"github.com/kurtosis-tech/kurtosis/container-engine-lib/lib/backend_interface/objects/engine"
 )
 
 type ReverseProxyContainer interface {
 	CreateAndStart(
 		ctx context.Context,
+		engineGuid engine.EngineGUID,
 		httpPort uint16,
 		dashboardPort uint16,
 		targetNetworkId string,

diff --git a/...ine-lib/lib/backend_impls/docker/object_attributes_provider/object_attributes_provider.go b/...ine-lib/lib/backend_impls/docker/object_attributes_provider/object_attributes_provider.go
@@ -1,6 +1,8 @@
 package object_attributes_provider
 
 import (
+	"strings"
+
 	"github.com/kurtosis-tech/kurtosis/container-engine-lib/lib/backend_impls/docker/object_attributes_provider/docker_label_key"
 	"github.com/kurtosis-tech/kurtosis/container-engine-lib/lib/backend_impls/docker/object_attributes_provider/docker_label_value"
 	"github.com/kurtosis-tech/kurtosis/container-engine-lib/lib/backend_impls/docker/object_attributes_provider/docker_object_name"
@@ -10,14 +12,13 @@ import (
 	"github.com/kurtosis-tech/kurtosis/container-engine-lib/lib/backend_interface/objects/engine"
 	"github.com/kurtosis-tech/kurtosis/container-engine-lib/lib/backend_interface/objects/port_spec"
 	"github.com/kurtosis-tech/stacktrace"
-	"strings"
 )
 
 const (
 	engineServerNamePrefix = "kurtosis-engine"
 	logsAggregatorName     = "kurtosis-logs-aggregator"
 	logsStorageVolumeName  = "kurtosis-logs-storage"
-	reverseProxyName       = "kurtosis-reverse-proxy"
+	reverseProxyNamePrefix = "kurtosis-reverse-proxy"
 )
 
 type DockerObjectAttributesProvider interface {
@@ -29,7 +30,7 @@ type DockerObjectAttributesProvider interface {
 	ForEnclave(enclaveUuid enclave.EnclaveUUID) (DockerEnclaveObjectAttributesProvider, error)
 	ForLogsAggregator() (DockerObjectAttributes, error)
 	ForLogsStorageVolume() (DockerObjectAttributes, error)
-	ForReverseProxy() (DockerObjectAttributes, error)
+	ForReverseProxy(engineGuid engine.EngineGUID) (DockerObjectAttributes, error)
 }
 
 func GetDockerObjectAttributesProvider() DockerObjectAttributesProvider {
@@ -137,14 +138,33 @@ func (provider *dockerObjectAttributesProviderImpl) ForLogsStorageVolume() (Dock
 	return objectAttributes, nil
 }
 
-func (provider *dockerObjectAttributesProviderImpl) ForReverseProxy() (DockerObjectAttributes, error) {
-	name, err := docker_object_name.CreateNewDockerObjectName(reverseProxyName)
+func (provider *dockerObjectAttributesProviderImpl) ForReverseProxy(engineGuid engine.EngineGUID) (DockerObjectAttributes, error) {
+
+	nameStr := strings.Join(
+		[]string{
+			reverseProxyNamePrefix,
+			string(engineGuid),
+		},
+		objectNameElementSeparator,
+	)
+	name, err := docker_object_name.CreateNewDockerObjectName(nameStr)
+	if err != nil {
+		return nil, stacktrace.Propagate(err, "An error occurred creating a Docker object name object from string '%v'", nameStr)
+	}
+
+	idLabelValue, err := docker_label_value.CreateNewDockerLabelValue(string(engineGuid))
 	if err != nil {
-		return nil, stacktrace.Propagate(err, "An error occurred creating a Docker object name object from string '%v'", reverseProxyName)
+		return nil, stacktrace.Propagate(err, "An error occurred creating the reverse proxy GUID Docker label from string '%v'", engineGuid)
+	}
+	guidLabelValue, err := docker_label_value.CreateNewDockerLabelValue(string(engineGuid))
+	if err != nil {
+		return nil, stacktrace.Propagate(err, "An error occurred creating the reverse proxy GUID Docker label from string '%v'", engineGuid)
 	}
 
 	labels := map[*docker_label_key.DockerLabelKey]*docker_label_value.DockerLabelValue{
 		docker_label_key.ContainerTypeDockerLabelKey: label_value_consts.ReverseProxyTypeDockerLabelValue,
+		docker_label_key.IDDockerLabelKey:            idLabelValue,
+		docker_label_key.GUIDDockerLabelKey:          guidLabelValue,
 	}
 
 	objectAttributes, err := newDockerObjectAttributesImpl(name, labels)

diff --git a/...b/lib/backend_impls/kubernetes/kubernetes_kurtosis_backend/kubernetes_kurtosis_backend.go b/...b/lib/backend_impls/kubernetes/kubernetes_kurtosis_backend/kubernetes_kurtosis_backend.go
@@ -467,7 +467,7 @@ func (backend *KubernetesKurtosisBackend) GetReverseProxy(
 	return nil, stacktrace.NewError("Getting the reverse proxy isn't yet implemented on Kubernetes")
 }
 
-func (backend *KubernetesKurtosisBackend) CreateReverseProxy(ctx context.Context) (*reverse_proxy.ReverseProxy, error) {
+func (backend *KubernetesKurtosisBackend) CreateReverseProxy(ctx context.Context, engineGuid engine.EngineGUID) (*reverse_proxy.ReverseProxy, error) {
 	// TODO IMPLEMENT
 	return nil, stacktrace.NewError("Creating the reverse proxy isn't yet implemented on Kubernetes")
 }

diff --git a/...iner-engine-lib/lib/backend_impls/metrics_reporting/metrics_reporting_kurtosis_backend.go b/...iner-engine-lib/lib/backend_impls/metrics_reporting/metrics_reporting_kurtosis_backend.go
@@ -440,8 +440,8 @@ func (backend *MetricsReportingKurtosisBackend) DestroyLogsCollectorForEnclave(c
 	return nil
 }
 
-func (backend *MetricsReportingKurtosisBackend) CreateReverseProxy(ctx context.Context) (*reverse_proxy.ReverseProxy, error) {
-	return backend.underlying.CreateReverseProxy(ctx)
+func (backend *MetricsReportingKurtosisBackend) CreateReverseProxy(ctx context.Context, engineGuid engine.EngineGUID) (*reverse_proxy.ReverseProxy, error) {
+	return backend.underlying.CreateReverseProxy(ctx, engineGuid)
 }
 
 func (backend *MetricsReportingKurtosisBackend) GetReverseProxy(ctx context.Context) (*reverse_proxy.ReverseProxy, error) {

diff --git a/container-engine-lib/lib/backend_interface/kurtosis_backend.go b/container-engine-lib/lib/backend_interface/kurtosis_backend.go
@@ -339,7 +339,7 @@ type KurtosisBackend interface {
 	// Destroy the logs collector for enclave with UUID
 	DestroyLogsCollectorForEnclave(ctx context.Context, enclaveUuid enclave.EnclaveUUID) error
 
-	CreateReverseProxy(ctx context.Context) (*reverse_proxy.ReverseProxy, error)
+	CreateReverseProxy(ctx context.Context, engineGuid engine.EngineGUID) (*reverse_proxy.ReverseProxy, error)
 
 	// Returns nil if logs aggregator was not found
 	GetReverseProxy(ctx context.Context) (*reverse_proxy.ReverseProxy, error)

diff --git a/container-engine-lib/lib/backend_interface/mock_kurtosis_backend.go b/container-engine-lib/lib/backend_interface/mock_kurtosis_backend.go