From 5d74d162019e95cf904c0dd4a2547039fe49af70 Mon Sep 17 00:00:00 2001 From: leoporoli Date: Mon, 18 Dec 2023 12:13:48 -0300 Subject: [PATCH] fix: adding remove logs aggregator container function when it already exists (#1974) ## Description: The remove aggregator function was missing when it already exists ## Is this change user facing? YES ## References (if applicable): One of our users was experiencing a segmentation violation error because this function was missing in the CreateEngine flow --- .../create_logs_aggregator.go | 14 ++++++++++++-- .../user_support_constants_test.go | 12 +++++++++++- 2 files changed, 23 insertions(+), 3 deletions(-) diff --git a/container-engine-lib/lib/backend_impls/docker/docker_kurtosis_backend/logs_aggregator_functions/create_logs_aggregator.go b/container-engine-lib/lib/backend_impls/docker/docker_kurtosis_backend/logs_aggregator_functions/create_logs_aggregator.go index bd667ebdfc..c6349f9b03 100644 --- a/container-engine-lib/lib/backend_impls/docker/docker_kurtosis_backend/logs_aggregator_functions/create_logs_aggregator.go +++ b/container-engine-lib/lib/backend_impls/docker/docker_kurtosis_backend/logs_aggregator_functions/create_logs_aggregator.go @@ -32,11 +32,21 @@ func CreateLogsAggregator( } if found { logrus.Debugf("Found existing logs aggregator; cannot start a new one.") - logsAggregatorObj, _, err := getLogsAggregatorObjectAndContainerId(ctx, dockerManager) + logsAggregatorObj, containerId, err := getLogsAggregatorObjectAndContainerId(ctx, dockerManager) if err != nil { return nil, nil, stacktrace.Propagate(err, "An error occurred getting existing logs aggregator.") } - return logsAggregatorObj, nil, nil + removeCtx := context.Background() + removeLogsAggregatorContainerFunc := func() { + if err := dockerManager.RemoveContainer(removeCtx, containerId); err != nil { + logrus.Errorf( + "Something failed while trying to remove the logs aggregator container with ID '%v'. Error was:\n%v", + containerId, + err) + logrus.Errorf("ACTION REQUIRED: You'll need to manually remove the logs aggregator server with Docker container ID '%v'!!!!!!", containerId) + } + } + return logsAggregatorObj, removeLogsAggregatorContainerFunc, nil } logsAggregatorNetwork, err := shared_helpers.GetEngineAndLogsComponentsNetwork(ctx, dockerManager) diff --git a/container-engine-lib/lib/user_support_constants/user_support_constants_test.go b/container-engine-lib/lib/user_support_constants/user_support_constants_test.go index f9f00f94c7..0ba8dcd589 100644 --- a/container-engine-lib/lib/user_support_constants/user_support_constants_test.go +++ b/container-engine-lib/lib/user_support_constants/user_support_constants_test.go @@ -7,11 +7,17 @@ package user_support_constants import ( "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" "net/http" "net/http/cookiejar" "testing" ) +const ( + safariUserAgent = "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/119.0.0.0 Safari/537.36" + userAgentHeaderKey = "User-Agent" +) + func TestValidUrls(t *testing.T) { for _, url := range urlsToValidateInTest { jar, err := cookiejar.New(nil) @@ -22,7 +28,11 @@ func TestValidUrls(t *testing.T) { client := &http.Client{ Jar: jar, } - resp, err := client.Get(url) + req, err := http.NewRequest(http.MethodGet, url, nil) + require.NoError(t, err, "Got an unexpected error while creating a new GET request with URL '%v'", url) + // Adding the User-Agent header because it's mandatory for sending a request to Twitter + req.Header.Set(userAgentHeaderKey, safariUserAgent) + resp, err := client.Do(req) assert.NoError(t, err, "Got an unexpected error checking url '%v'", url) assert.True(t, isValidReturnCode(resp.StatusCode), "URL '%v' returned unexpected status code: '%d'", url, resp.StatusCode) assert.NoError(t, err, "Got an unexpected error checking url '%v'", url)