From 151930fe1ede7cbfea3c1b17cddbda4855960b3f Mon Sep 17 00:00:00 2001 From: Tiago Queiroz Date: Tue, 11 Jun 2024 13:25:09 -0400 Subject: [PATCH 01/10] Ensure socket path when STATE_PATH is set is < 104 characters long --- internal/pkg/agent/cmd/container.go | 9 ++ testing/integration/container_cmd_test.go | 134 +++++++++++++++++----- 2 files changed, 113 insertions(+), 30 deletions(-) diff --git a/internal/pkg/agent/cmd/container.go b/internal/pkg/agent/cmd/container.go index 8ceea38c14e..a00cc2f58ee 100644 --- a/internal/pkg/agent/cmd/container.go +++ b/internal/pkg/agent/cmd/container.go @@ -34,6 +34,7 @@ import ( "github.com/elastic/elastic-agent/pkg/component" "github.com/elastic/elastic-agent/pkg/core/logger" "github.com/elastic/elastic-agent/pkg/core/process" + "github.com/elastic/elastic-agent/pkg/utils" "github.com/elastic/elastic-agent/version" ) @@ -774,6 +775,14 @@ func setPaths(statePath, configPath, logsPath, socketPath string, writePaths boo if statePath == "" { statePath = defaultStateDirectory } + + // A Unix socket path needs to be < 104 characters long, so we ensure + // the statePath when concatenated with the socked name is going to + // be smaller than 104 characters. + if len(statePath) > 75 { + statePath = utils.SocketFallbackDirectory + } + topPath := filepath.Join(statePath, "data") configPath = envWithDefault(configPath, "CONFIG_PATH") if configPath == "" { diff --git a/testing/integration/container_cmd_test.go b/testing/integration/container_cmd_test.go index c9c0b3eccbe..748a3874e12 100644 --- a/testing/integration/container_cmd_test.go +++ b/testing/integration/container_cmd_test.go @@ -10,6 +10,8 @@ import ( "context" "fmt" "os" + "os/exec" + "path/filepath" "strings" "testing" "time" @@ -18,29 +20,12 @@ import ( "github.com/stretchr/testify/require" "github.com/elastic/elastic-agent-libs/kibana" + atesting "github.com/elastic/elastic-agent/pkg/testing" "github.com/elastic/elastic-agent/pkg/testing/define" "github.com/elastic/elastic-agent/pkg/testing/tools/fleettools" ) -func TestContainerCMD(t *testing.T) { - info := define.Require(t, define.Requirements{ - Stack: &define.Stack{}, - Local: false, - Sudo: true, - OS: []define.OS{ - {Type: define.Linux}, - }, - // This test runs the command we use when executing inside a container - // which leaves files under /usr/share/elastic-agent. Run it isolated - // to avoid interfering with other tests and better simulate a container - // environment we run it in isolation - Group: "container", - }) - ctx := context.Background() - - agentFixture, err := define.NewFixtureFromLocalBuild(t, define.Version()) - require.NoError(t, err) - +func createPolicy(t *testing.T, ctx context.Context, agentFixture *atesting.Fixture, info *define.Info) string { createPolicyReq := kibana.AgentPolicy{ Name: fmt.Sprintf("test-policy-enroll-%s", uuid.New().String()), Namespace: info.Namespace, @@ -74,13 +59,10 @@ func TestContainerCMD(t *testing.T) { t.Fatalf("unable to create enrolment API key: %s", err) } - fleetURL, err := fleettools.DefaultURL(ctx, info.KibanaClient) - if err != nil { - t.Fatalf("could not get Fleet URL: %s", err) - } + return enrollmentToken.APIKey +} - ctx, cancel := context.WithTimeout(ctx, 1*time.Minute) - defer cancel() +func prepareContainerCMD(t *testing.T, ctx context.Context, agentFixture *atesting.Fixture, info *define.Info, env []string) *exec.Cmd { cmd, err := agentFixture.PrepareAgentCommand(ctx, []string{"container"}) if err != nil { t.Fatalf("could not prepare agent command: %s", err) @@ -112,22 +94,114 @@ func TestContainerCMD(t *testing.T) { agentOutput := strings.Builder{} cmd.Stderr = &agentOutput cmd.Stdout = &agentOutput - cmd.Env = append(os.Environ(), + cmd.Env = append(os.Environ(), env...) + return cmd +} + +func TestContainerCMD(t *testing.T) { + info := define.Require(t, define.Requirements{ + Stack: &define.Stack{}, + Local: false, + Sudo: true, + OS: []define.OS{ + {Type: define.Linux}, + }, + Group: define.Default, + }) + + ctx, cancel := context.WithTimeout(context.Background(), 1*time.Minute) + defer cancel() + + agentFixture, err := define.NewFixtureFromLocalBuild(t, define.Version()) + require.NoError(t, err) + + fleetURL, err := fleettools.DefaultURL(ctx, info.KibanaClient) + if err != nil { + t.Fatalf("could not get Fleet URL: %s", err) + } + + enrollmentToken := createPolicy(t, ctx, agentFixture, info) + env := []string{ "FLEET_ENROLL=1", - "FLEET_URL="+fleetURL, - "FLEET_ENROLLMENT_TOKEN="+enrollmentToken.APIKey, + "FLEET_URL=" + fleetURL, + "FLEET_ENROLLMENT_TOKEN=" + enrollmentToken, // As the agent isn't built for a container, it's upgradable, triggering // the start of the upgrade watcher. If `STATE_PATH` isn't set, the // upgrade watcher will commence from a different path within the // container, distinct from the current execution path. - "STATE_PATH="+agentFixture.WorkDir(), + "STATE_PATH=" + agentFixture.WorkDir(), + } + + cmd := prepareContainerCMD(t, ctx, agentFixture, info, env) + t.Logf(">> running binary with: %v", cmd.Args) + if err := cmd.Start(); err != nil { + t.Fatalf("error running container cmd: %s", err) + } + + agentOutput := cmd.Stderr.(*strings.Builder) + + require.Eventuallyf(t, func() bool { + // This will return errors until it connects to the agent, + // they're mostly noise because until the agent starts running + // we will get connection errors. If the test fails + // the agent logs will be present in the error message + // which should help to explain why the agent was not + // healthy. + err = agentFixture.IsHealthy(ctx) + return err == nil + }, + 5*time.Minute, time.Second, + "Elastic-Agent did not report healthy. Agent status error: \"%v\", Agent logs\n%s", + err, agentOutput, ) +} +func TestContainerCMDWithAVeryLongStatePath(t *testing.T) { + info := define.Require(t, define.Requirements{ + Stack: &define.Stack{}, + Local: false, + Sudo: true, + OS: []define.OS{ + {Type: define.Linux}, + }, + Group: define.Default, + }) + + ctx, cancel := context.WithTimeout(context.Background(), 1*time.Minute) + defer cancel() + + agentFixture, err := define.NewFixtureFromLocalBuild(t, define.Version()) + require.NoError(t, err) + + fleetURL, err := fleettools.DefaultURL(ctx, info.KibanaClient) + if err != nil { + t.Fatalf("could not get Fleet URL: %s", err) + } + + // We need a statePath that will make the unix socket path longer than 105 characters + // so we join the workdir and a 120 characters long string. + statePath := filepath.Join(agentFixture.WorkDir(), "de9a2a338c4fe10a466ee9fae57ce0c8a5b010dfcd6bd3f41d2c569ef5ed873193fd7d1966a070174f47f93ee667f921616c2d6d29efb6dbcc2b8b33") + + enrollmentToken := createPolicy(t, ctx, agentFixture, info) + env := []string{ + "FLEET_ENROLL=1", + "FLEET_URL=" + fleetURL, + "FLEET_ENROLLMENT_TOKEN=" + enrollmentToken, + // As the agent isn't built for a container, it's upgradable, triggering + // the start of the upgrade watcher. If `STATE_PATH` isn't set, the + // upgrade watcher will commence from a different path within the + // container, distinct from the current execution path. + "STATE_PATH=" + statePath, + } + + cmd := prepareContainerCMD(t, ctx, agentFixture, info, env) t.Logf(">> running binary with: %v", cmd.Args) if err := cmd.Start(); err != nil { t.Fatalf("error running container cmd: %s", err) } + agentOutput := cmd.Stderr.(*strings.Builder) + require.Eventuallyf(t, func() bool { // This will return errors until it connects to the agent, // they're mostly noise because until the agent starts running @@ -140,6 +214,6 @@ func TestContainerCMD(t *testing.T) { }, 5*time.Minute, time.Second, "Elastic-Agent did not report healthy. Agent status error: \"%v\", Agent logs\n%s", - err, &agentOutput, + err, agentOutput, ) } From 873293ffa2f144a59037ad1417856ec254831aad Mon Sep 17 00:00:00 2001 From: Tiago Queiroz Date: Tue, 11 Jun 2024 15:57:59 -0400 Subject: [PATCH 02/10] Fix Windows and use "container" test group This commit fixes the implementation on Windows and puts the container integration tests in the container group again. The test for long state paths will create files outside of the test directory. It also cleans up the default folder used as state path. --- internal/pkg/agent/cmd/container.go | 2 +- testing/integration/container_cmd_test.go | 13 +++++++++++-- 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/internal/pkg/agent/cmd/container.go b/internal/pkg/agent/cmd/container.go index a00cc2f58ee..9e51cd93489 100644 --- a/internal/pkg/agent/cmd/container.go +++ b/internal/pkg/agent/cmd/container.go @@ -780,7 +780,7 @@ func setPaths(statePath, configPath, logsPath, socketPath string, writePaths boo // the statePath when concatenated with the socked name is going to // be smaller than 104 characters. if len(statePath) > 75 { - statePath = utils.SocketFallbackDirectory + statePath = utils.SocketURLWithFallback(statePath, paths.TempDir()) } topPath := filepath.Join(statePath, "data") diff --git a/testing/integration/container_cmd_test.go b/testing/integration/container_cmd_test.go index 748a3874e12..6d321c038f5 100644 --- a/testing/integration/container_cmd_test.go +++ b/testing/integration/container_cmd_test.go @@ -106,7 +106,7 @@ func TestContainerCMD(t *testing.T) { OS: []define.OS{ {Type: define.Linux}, }, - Group: define.Default, + Group: "container", }) ctx, cancel := context.WithTimeout(context.Background(), 1*time.Minute) @@ -164,7 +164,7 @@ func TestContainerCMDWithAVeryLongStatePath(t *testing.T) { OS: []define.OS{ {Type: define.Linux}, }, - Group: define.Default, + Group: "container", }) ctx, cancel := context.WithTimeout(context.Background(), 1*time.Minute) @@ -182,6 +182,15 @@ func TestContainerCMDWithAVeryLongStatePath(t *testing.T) { // so we join the workdir and a 120 characters long string. statePath := filepath.Join(agentFixture.WorkDir(), "de9a2a338c4fe10a466ee9fae57ce0c8a5b010dfcd6bd3f41d2c569ef5ed873193fd7d1966a070174f47f93ee667f921616c2d6d29efb6dbcc2b8b33") + // We know it will use the OS temp folder for the state path, so we try + // to clean it up at the end of the test. + t.Cleanup(func() { + defaultStatePath := "/tmp/elastic-agent" + if err := os.RemoveAll(defaultStatePath); err != nil { + t.Errorf("could not remove config path '%s': %s", defaultStatePath, err) + } + }) + enrollmentToken := createPolicy(t, ctx, agentFixture, info) env := []string{ "FLEET_ENROLL=1", From 845fff352119a454fceac7ceae15dad2f4b554c4 Mon Sep 17 00:00:00 2001 From: Tiago Queiroz Date: Wed, 12 Jun 2024 16:53:28 -0400 Subject: [PATCH 03/10] Fix corner cases and improve tests --- internal/pkg/agent/cmd/container.go | 24 +++-- testing/integration/container_cmd_test.go | 116 +++++++++++++--------- 2 files changed, 83 insertions(+), 57 deletions(-) diff --git a/internal/pkg/agent/cmd/container.go b/internal/pkg/agent/cmd/container.go index 9e51cd93489..8b553152975 100644 --- a/internal/pkg/agent/cmd/container.go +++ b/internal/pkg/agent/cmd/container.go @@ -34,7 +34,6 @@ import ( "github.com/elastic/elastic-agent/pkg/component" "github.com/elastic/elastic-agent/pkg/core/logger" "github.com/elastic/elastic-agent/pkg/core/process" - "github.com/elastic/elastic-agent/pkg/utils" "github.com/elastic/elastic-agent/version" ) @@ -772,17 +771,10 @@ func logToStderr(cfg *configuration.Configuration) { func setPaths(statePath, configPath, logsPath, socketPath string, writePaths bool) error { statePath = envWithDefault(statePath, "STATE_PATH") - if statePath == "" { + if isStatePathTooLong(statePath) || statePath == "" { statePath = defaultStateDirectory } - // A Unix socket path needs to be < 104 characters long, so we ensure - // the statePath when concatenated with the socked name is going to - // be smaller than 104 characters. - if len(statePath) > 75 { - statePath = utils.SocketURLWithFallback(statePath, paths.TempDir()) - } - topPath := filepath.Join(statePath, "data") configPath = envWithDefault(configPath, "CONFIG_PATH") if configPath == "" { @@ -971,3 +963,17 @@ func isContainer(detail component.PlatformDetail) component.PlatformDetail { detail.OS = component.Container return detail } + +// isStatePathTooLong returns true if the final Unix socket path +// will be too long and needs to be shortened. +// +// Source: https://www.man7.org/linux/man-pages/man7/unix.7.html +func isStatePathTooLong(statePath string) bool { + // This replicates the logic from `setPaths` + socketPath := filepath.Join(statePath, "data", paths.ControlSocketName) + if len(socketPath) > 107 { + return true + } + + return false +} diff --git a/testing/integration/container_cmd_test.go b/testing/integration/container_cmd_test.go index 6d321c038f5..6b974d20139 100644 --- a/testing/integration/container_cmd_test.go +++ b/testing/integration/container_cmd_test.go @@ -167,62 +167,82 @@ func TestContainerCMDWithAVeryLongStatePath(t *testing.T) { Group: "container", }) - ctx, cancel := context.WithTimeout(context.Background(), 1*time.Minute) + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Minute) defer cancel() - agentFixture, err := define.NewFixtureFromLocalBuild(t, define.Version()) - require.NoError(t, err) - fleetURL, err := fleettools.DefaultURL(ctx, info.KibanaClient) if err != nil { t.Fatalf("could not get Fleet URL: %s", err) } - // We need a statePath that will make the unix socket path longer than 105 characters - // so we join the workdir and a 120 characters long string. - statePath := filepath.Join(agentFixture.WorkDir(), "de9a2a338c4fe10a466ee9fae57ce0c8a5b010dfcd6bd3f41d2c569ef5ed873193fd7d1966a070174f47f93ee667f921616c2d6d29efb6dbcc2b8b33") - - // We know it will use the OS temp folder for the state path, so we try - // to clean it up at the end of the test. - t.Cleanup(func() { - defaultStatePath := "/tmp/elastic-agent" - if err := os.RemoveAll(defaultStatePath); err != nil { - t.Errorf("could not remove config path '%s': %s", defaultStatePath, err) - } - }) - - enrollmentToken := createPolicy(t, ctx, agentFixture, info) - env := []string{ - "FLEET_ENROLL=1", - "FLEET_URL=" + fleetURL, - "FLEET_ENROLLMENT_TOKEN=" + enrollmentToken, - // As the agent isn't built for a container, it's upgradable, triggering - // the start of the upgrade watcher. If `STATE_PATH` isn't set, the - // upgrade watcher will commence from a different path within the - // container, distinct from the current execution path. - "STATE_PATH=" + statePath, - } - - cmd := prepareContainerCMD(t, ctx, agentFixture, info, env) - t.Logf(">> running binary with: %v", cmd.Args) - if err := cmd.Start(); err != nil { - t.Fatalf("error running container cmd: %s", err) + testCases := map[string]struct { + statePath string + expectedStatePath string + expectError bool + }{ + "small path": { // Use the set path + statePath: filepath.Join(os.TempDir(), "foo", "bar"), + expectedStatePath: filepath.Join(os.TempDir(), "foo", "bar"), + }, + "no path set": { // Use the default path + statePath: "", + expectedStatePath: "/usr/share/elastic-agent/state", + }, + "107 characters path": { // Longest path that still works for creating a unix socket + statePath: "/tmp/tttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttt", + expectedStatePath: "/tmp/tttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttt", + }, + "long path": { // This will falback to the default path + statePath: filepath.Join(os.TempDir(), "ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff"), + expectedStatePath: "/usr/share/elastic-agent/state", + }, } - agentOutput := cmd.Stderr.(*strings.Builder) + for name, tc := range testCases { + t.Run(name, func(t *testing.T) { + agentFixture, err := define.NewFixtureFromLocalBuild(t, define.Version()) + require.NoError(t, err) + + enrollmentToken := createPolicy(t, ctx, agentFixture, info) + env := []string{ + "FLEET_ENROLL=1", + "FLEET_URL=" + fleetURL, + "FLEET_ENROLLMENT_TOKEN=" + enrollmentToken, + "STATE_PATH=" + tc.statePath, + } - require.Eventuallyf(t, func() bool { - // This will return errors until it connects to the agent, - // they're mostly noise because until the agent starts running - // we will get connection errors. If the test fails - // the agent logs will be present in the error message - // which should help to explain why the agent was not - // healthy. - err = agentFixture.IsHealthy(ctx) - return err == nil - }, - 5*time.Minute, time.Second, - "Elastic-Agent did not report healthy. Agent status error: \"%v\", Agent logs\n%s", - err, agentOutput, - ) + cmd := prepareContainerCMD(t, ctx, agentFixture, info, env) + t.Logf(">> running binary with: %v", cmd.Args) + if err := cmd.Start(); err != nil { + t.Fatalf("error running container cmd: %s", err) + } + agentOutput := cmd.Stderr.(*strings.Builder) + + require.Eventuallyf(t, func() bool { + // This will return errors until it connects to the agent, + // they're mostly noise because until the agent starts running + // we will get connection errors. If the test fails + // the agent logs will be present in the error message + // which should help to explain why the agent was not + // healthy. + err = agentFixture.IsHealthy(ctx) + return err == nil + }, + 1*time.Minute, time.Second, + "Elastic-Agent did not report healthy. Agent status error: \"%v\", Agent logs\n%s", + err, agentOutput, + ) + + t.Cleanup(func() { + _ = os.RemoveAll(tc.expectedStatePath) + }) + + // Now that the Elastic-Agent is healthy, check that the control socket path + // is the expected one + expectedSocketPath := filepath.Join(tc.expectedStatePath, "data", "elastic-agent.sock") + if _, err := os.Stat(expectedSocketPath); err != nil { + t.Errorf("cannot stat expected socket ('%s'): %s", expectedSocketPath, err) + } + }) + } } From 916a96a51f29b08e67fb208fff591129d6a6f913 Mon Sep 17 00:00:00 2001 From: Tiago Queiroz Date: Wed, 12 Jun 2024 17:06:30 -0400 Subject: [PATCH 04/10] Add changelog --- changelog/fragments/1718226176-Ensure.yaml | 35 ++++++++++++++++++++++ 1 file changed, 35 insertions(+) create mode 100644 changelog/fragments/1718226176-Ensure.yaml diff --git a/changelog/fragments/1718226176-Ensure.yaml b/changelog/fragments/1718226176-Ensure.yaml new file mode 100644 index 00000000000..e9d105b6d54 --- /dev/null +++ b/changelog/fragments/1718226176-Ensure.yaml @@ -0,0 +1,35 @@ +# Kind can be one of: +# - breaking-change: a change to previously-documented behavior +# - deprecation: functionality that is being removed in a later release +# - bug-fix: fixes a problem in a previous version +# - enhancement: extends functionality but does not break or fix existing behavior +# - feature: new functionality +# - known-issue: problems that we are aware of in a given version +# - security: impacts on the security of a product or a user’s deployment. +# - upgrade: important information for someone upgrading from a prior version +# - other: does not fit into any of the other categories +kind: bug-fix + +# Change summary; a 80ish characters long description of the change. +summary: Ensure socket can be created even when STATE_PATH is too long for container command + +# Long description; in case the summary is not enough to describe the change +# this field accommodate a description without length limits. +# NOTE: This field will be rendered only for breaking-change and known-issue kinds at the moment. +description: | + When running the Elastic-Agent container command and `STATE_PATH` is + set to a value that is too long the Unix socket cannot be created, + so the Elastic-Agent will use the default state path instead. + +# Affected component; usually one of "elastic-agent", "fleet-server", "filebeat", "metricbeat", "auditbeat", "all", etc. +component: elastic-agent + +# PR URL; optional; the PR number that added the changeset. +# If not present is automatically filled by the tooling finding the PR where this changelog fragment has been added. +# NOTE: the tooling supports backports, so it's able to fill the original PR number instead of the backport PR number. +# Please provide it if you are adding a fragment for a different PR. +pr: https://github.com/elastic/elastic-agent/pull/4909 + +# Issue URL; optional; the GitHub issue related to this changeset (either closes or is part of). +# If not present is automatically filled by the tooling with the issue linked to the PR number. +#issue: https://github.com/owner/repo/1234 From ff5d781708ee1b2dd40265f903aff3bd484d6e92 Mon Sep 17 00:00:00 2001 From: Tiago Queiroz Date: Wed, 12 Jun 2024 17:07:23 -0400 Subject: [PATCH 05/10] Fix lint warning --- internal/pkg/agent/cmd/container.go | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/internal/pkg/agent/cmd/container.go b/internal/pkg/agent/cmd/container.go index 8b553152975..e42aaad4d48 100644 --- a/internal/pkg/agent/cmd/container.go +++ b/internal/pkg/agent/cmd/container.go @@ -971,9 +971,5 @@ func isContainer(detail component.PlatformDetail) component.PlatformDetail { func isStatePathTooLong(statePath string) bool { // This replicates the logic from `setPaths` socketPath := filepath.Join(statePath, "data", paths.ControlSocketName) - if len(socketPath) > 107 { - return true - } - - return false + return len(socketPath) > 107 } From ccf9439abbc4fb9de6e509e5591f8fa17325be8b Mon Sep 17 00:00:00 2001 From: Tiago Queiroz Date: Mon, 17 Jun 2024 10:34:03 -0400 Subject: [PATCH 06/10] Only change socket path --- internal/pkg/agent/cmd/container.go | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/internal/pkg/agent/cmd/container.go b/internal/pkg/agent/cmd/container.go index e42aaad4d48..c48940f67e4 100644 --- a/internal/pkg/agent/cmd/container.go +++ b/internal/pkg/agent/cmd/container.go @@ -34,6 +34,7 @@ import ( "github.com/elastic/elastic-agent/pkg/component" "github.com/elastic/elastic-agent/pkg/core/logger" "github.com/elastic/elastic-agent/pkg/core/process" + "github.com/elastic/elastic-agent/pkg/utils" "github.com/elastic/elastic-agent/version" ) @@ -771,7 +772,7 @@ func logToStderr(cfg *configuration.Configuration) { func setPaths(statePath, configPath, logsPath, socketPath string, writePaths bool) error { statePath = envWithDefault(statePath, "STATE_PATH") - if isStatePathTooLong(statePath) || statePath == "" { + if statePath == "" { statePath = defaultStateDirectory } @@ -780,8 +781,14 @@ func setPaths(statePath, configPath, logsPath, socketPath string, writePaths boo if configPath == "" { configPath = statePath } + if _, err := os.Stat(configPath); os.IsNotExist(err) { + if err := os.MkdirAll(configPath, 0755); err != nil { + return fmt.Errorf("cannot create folders for config path '%s': %w", configPath, err) + } + } + if socketPath == "" { - socketPath = fmt.Sprintf("unix://%s", filepath.Join(topPath, paths.ControlSocketName)) + socketPath = utils.SocketURLWithFallback(statePath, topPath) } // ensure that the directory and sub-directory data exists if err := os.MkdirAll(topPath, 0755); err != nil { From 9d135466a8081bccc04905d901f3f017c059044c Mon Sep 17 00:00:00 2001 From: Tiago Queiroz Date: Mon, 17 Jun 2024 17:21:47 -0400 Subject: [PATCH 07/10] Update tests to new approach --- testing/integration/container_cmd_test.go | 51 +++++++++++++++-------- 1 file changed, 34 insertions(+), 17 deletions(-) diff --git a/testing/integration/container_cmd_test.go b/testing/integration/container_cmd_test.go index 6b974d20139..a1565aae859 100644 --- a/testing/integration/container_cmd_test.go +++ b/testing/integration/container_cmd_test.go @@ -2,7 +2,7 @@ // or more contributor license agreements. Licensed under the Elastic License; // you may not use this file except in compliance with the Elastic License. -//go:build integration +//go:build integration && linux package integration @@ -176,25 +176,30 @@ func TestContainerCMDWithAVeryLongStatePath(t *testing.T) { } testCases := map[string]struct { - statePath string - expectedStatePath string - expectError bool + statePath string + expectedStatePath string + expectedSocketPath string + expectError bool }{ "small path": { // Use the set path - statePath: filepath.Join(os.TempDir(), "foo", "bar"), - expectedStatePath: filepath.Join(os.TempDir(), "foo", "bar"), + statePath: filepath.Join(os.TempDir(), "foo", "bar"), + expectedStatePath: filepath.Join(os.TempDir(), "foo", "bar"), + expectedSocketPath: "/tmp/foo/bar/data/smp7BzlzcwgrLK4PUxpu7G1O5UwV4adr.sock", }, "no path set": { // Use the default path - statePath: "", - expectedStatePath: "/usr/share/elastic-agent/state", + statePath: "", + expectedStatePath: "/usr/share/elastic-agent/state", + expectedSocketPath: "/usr/share/elastic-agent/state/data/Td8I7R-Zby36_zF_IOd9QVNlFblNEro3.sock", }, - "107 characters path": { // Longest path that still works for creating a unix socket - statePath: "/tmp/tttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttt", - expectedStatePath: "/tmp/tttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttt", + "107 characters path": { // Longest path that still works for creating a unix socket, but will use /tmp/elastic-agent + statePath: "/tmp/tttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttt", + expectedStatePath: "/tmp/tttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttt", + expectedSocketPath: "/tmp/elastic-agent/oKUUJbxrLlGSh3z6wZWYleLeMuUN4P0_.sock", }, - "long path": { // This will falback to the default path - statePath: filepath.Join(os.TempDir(), "ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff"), - expectedStatePath: "/usr/share/elastic-agent/state", + "long path": { // Path too long to create a unix socket, it will use /tmp/elastic-agent + statePath: "/tmp/ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff", + expectedStatePath: "/tmp/ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff", + expectedSocketPath: "/tmp/elastic-agent/Xegnlbb8QDcqNLPzyf2l8PhVHjWvlQgZ.sock", }, } @@ -239,9 +244,21 @@ func TestContainerCMDWithAVeryLongStatePath(t *testing.T) { // Now that the Elastic-Agent is healthy, check that the control socket path // is the expected one - expectedSocketPath := filepath.Join(tc.expectedStatePath, "data", "elastic-agent.sock") - if _, err := os.Stat(expectedSocketPath); err != nil { - t.Errorf("cannot stat expected socket ('%s'): %s", expectedSocketPath, err) + if _, err := os.Stat(tc.expectedStatePath); err != nil { + t.Errorf("cannot stat expected state path ('%s'): %s", tc.expectedStatePath, err) + } + if _, err := os.Stat(tc.expectedSocketPath); err != nil { + t.Errorf("cannot stat expected socket path ('%s'): %s", tc.expectedSocketPath, err) + } + + if t.Failed() { + containerPaths, err := os.ReadFile(filepath.Join(agentFixture.WorkDir(), "container-paths.yml")) + if err != nil { + t.Fatalf("could not read container-paths.yml: %s", err) + } + + t.Log("contents of 'container-paths-yml'") + t.Log(string(containerPaths)) } }) } From e46948b76770907cb0c5254e5c4fe68c2076f262 Mon Sep 17 00:00:00 2001 From: Tiago Queiroz Date: Mon, 17 Jun 2024 17:36:03 -0400 Subject: [PATCH 08/10] remove un-used function --- internal/pkg/agent/cmd/container.go | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/internal/pkg/agent/cmd/container.go b/internal/pkg/agent/cmd/container.go index c48940f67e4..24c553b2af5 100644 --- a/internal/pkg/agent/cmd/container.go +++ b/internal/pkg/agent/cmd/container.go @@ -970,13 +970,3 @@ func isContainer(detail component.PlatformDetail) component.PlatformDetail { detail.OS = component.Container return detail } - -// isStatePathTooLong returns true if the final Unix socket path -// will be too long and needs to be shortened. -// -// Source: https://www.man7.org/linux/man-pages/man7/unix.7.html -func isStatePathTooLong(statePath string) bool { - // This replicates the logic from `setPaths` - socketPath := filepath.Join(statePath, "data", paths.ControlSocketName) - return len(socketPath) > 107 -} From a6d91b029ae8c0b8ccf7bb391c8ac304fe1e9368 Mon Sep 17 00:00:00 2001 From: Tiago Queiroz Date: Tue, 18 Jun 2024 11:36:57 -0400 Subject: [PATCH 09/10] Update internal/pkg/agent/cmd/container.go Co-authored-by: Michel Laterman <82832767+michel-laterman@users.noreply.github.com> --- internal/pkg/agent/cmd/container.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/pkg/agent/cmd/container.go b/internal/pkg/agent/cmd/container.go index 24c553b2af5..63f02c41255 100644 --- a/internal/pkg/agent/cmd/container.go +++ b/internal/pkg/agent/cmd/container.go @@ -781,7 +781,7 @@ func setPaths(statePath, configPath, logsPath, socketPath string, writePaths boo if configPath == "" { configPath = statePath } - if _, err := os.Stat(configPath); os.IsNotExist(err) { + if _, err := os.Stat(configPath); errors.Is(err, fs.ErrNotExist) { if err := os.MkdirAll(configPath, 0755); err != nil { return fmt.Errorf("cannot create folders for config path '%s': %w", configPath, err) } From f611c407b912998a29bb2451a31836bdbe7993d7 Mon Sep 17 00:00:00 2001 From: Tiago Queiroz Date: Thu, 20 Jun 2024 08:31:37 -0400 Subject: [PATCH 10/10] fix build and PR suggestions --- internal/pkg/agent/cmd/container.go | 1 + testing/integration/container_cmd_test.go | 7 +------ 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/internal/pkg/agent/cmd/container.go b/internal/pkg/agent/cmd/container.go index 63f02c41255..db454cf5fa9 100644 --- a/internal/pkg/agent/cmd/container.go +++ b/internal/pkg/agent/cmd/container.go @@ -9,6 +9,7 @@ import ( "encoding/json" "fmt" "io" + "io/fs" "net/url" "os" "os/exec" diff --git a/testing/integration/container_cmd_test.go b/testing/integration/container_cmd_test.go index a1565aae859..fa20c65e27f 100644 --- a/testing/integration/container_cmd_test.go +++ b/testing/integration/container_cmd_test.go @@ -2,7 +2,7 @@ // or more contributor license agreements. Licensed under the Elastic License; // you may not use this file except in compliance with the Elastic License. -//go:build integration && linux +//go:build integration package integration @@ -191,11 +191,6 @@ func TestContainerCMDWithAVeryLongStatePath(t *testing.T) { expectedStatePath: "/usr/share/elastic-agent/state", expectedSocketPath: "/usr/share/elastic-agent/state/data/Td8I7R-Zby36_zF_IOd9QVNlFblNEro3.sock", }, - "107 characters path": { // Longest path that still works for creating a unix socket, but will use /tmp/elastic-agent - statePath: "/tmp/tttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttt", - expectedStatePath: "/tmp/tttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttt", - expectedSocketPath: "/tmp/elastic-agent/oKUUJbxrLlGSh3z6wZWYleLeMuUN4P0_.sock", - }, "long path": { // Path too long to create a unix socket, it will use /tmp/elastic-agent statePath: "/tmp/ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff", expectedStatePath: "/tmp/ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff",