internal/testrunner/runners/script: provide mechanism to inspect agent containers by efd6 · Pull Request #3107 · elastic/elastic-package

efd6 · 2025-11-25T03:50:49Z

Please take a look.

The addition to the get_docs test demonstrates how this can be used with a trivial example.

…t containers

efd6 · 2025-11-25T03:53:28Z

internal/testrunner/script/agents.go

+		ts.Setenv(*networkNameLabel, depInfo.NetworkName)
+	}
+	if *containerNameLabel != "" {
+		ts.Setenv(*containerNameLabel, fmt.Sprintf("elastic-package-agent-%s-%s-%s-%s-1", filepath.Base(pkgRoot), ds, info.Test.RunID, depInfo.Name))


@mrodm Is this a reasonable way to reconstruct this? I did not see any direct representation of the container name for the container running the agent.

I expect that the suffix will ~always be "-1" since the run ID is a random value and we would not expect to have collisions between running agents, even in the case that a script started two or more agents (something that I would imagine would be vanishingly rare in itself).

LGTM.
As a reference, this is the code where the docker project name is built:

elastic-package/internal/agentdeployer/agent.go

Line 136 in 6a690bf

composeProjectName := fmt.Sprintf("elastic-package-agent-%s-%s", d.agentName(), agentInfo.Test.RunID)

elastic-package/internal/agentdeployer/agent.go

Lines 235 to 241 in 6a690bf

func (d *DockerComposeAgentDeployer) agentName() string {

name := d.packageName

if d.dataStream != "" && d.dataStream != "." {

name = fmt.Sprintf("%s-%s", name, d.dataStream)

}

return name

}

An example from nginx would be (data stream access):

elastic-package-agent-nginx-access-11932-elastic-agent-1

The only difference would be if input packages are being tested.

In that case, the docker project name would not contain the dataStream string.

elastic-package-agent-sql_input-46311-elastic-agent-1

mrodm

LGTM
Just a couple of comments to rename some messages to refer to the right function.

internal/testrunner/script/agents.go

Not sure about adding the new method to the DockerComposeAgentDeployer without adding it to the AgentDeployer interface.

mrodm · 2025-12-01T12:20:09Z

internal/agentdeployer/agent.go

+// ProjectName returns the Docker Compose project name for the agent.
+func (d *DockerComposeAgentDeployer) ProjectName(runID string) string {
+	return fmt.Sprintf("elastic-package-agent-%s-%s", d.agentName(), runID)
+}


Just re-reviewing this PR, I realised that DockerComposeAgentDeployer implements the interface AgentDeployer.

elastic-package/internal/agentdeployer/deployer.go

Line 11 in ffd949a

type AgentDeployer interface {

And this ProjectName method is not there.

I think it would be better to add this method to the interface too.

That would enforce to add this method also to KubernetesAgentDeployer, but I'm not sure that makes sense there, because in that case the agent runs directly in k8s and not in a docker-compose scenario.

elastic-package/internal/agentdeployer/kubernetes.go

Line 109 in ffd949a

func (ksd *KubernetesAgentDeployer) SetUp(ctx context.Context, agentInfo AgentInfo) (DeployedAgent, error) {

@efd6 @elastic/ecosystem WDYT about this ?

I think it would be better to add this method to the interface too.

I guess it is not an issue to keep that method as a public one but not being part of the AgentDeployer interface. Specially, because this new method does not make sense in KubernetesAgentDeployer as mentioned in:

That would enforce to add this method also to KubernetesAgentDeployer, but I'm not sure that makes sense there, because in that case the agent runs directly in k8s and not in a docker-compose scenario.

mrodm · 2025-12-01T14:41:23Z

internal/agentdeployer/agent.go

+// ProjectName returns the Docker Compose project name for the agent.
+func (d *DockerComposeAgentDeployer) ProjectName(runID string) string {
+	return fmt.Sprintf("elastic-package-agent-%s-%s", d.agentName(), runID)
+}


I think it would be better to add this method to the interface too.

I guess it is not an issue to keep that method as a public one but not being part of the AgentDeployer interface. Specially, because this new method does not make sense in KubernetesAgentDeployer as mentioned in:

That would enforce to add this method also to KubernetesAgentDeployer, but I'm not sure that makes sense there, because in that case the agent runs directly in k8s and not in a docker-compose scenario.

efd6 · 2025-12-01T19:59:17Z

Not sure about adding the new method to the DockerComposeAgentDeployer without adding it to the AgentDeployer interface.

This was intentional and follows the Go approach to progressive code repair. The interface advertises the generic consumer's interface, but we are not the generic consumer. It's not needed right now (and likely never will be needed), so it should not be added until it is. This is for three reasons: 1. it's a breaking change, 2. a dummy method would have to be added to all the other implementers of the interface (related to 1.) and 3. "The bigger the interface, the weaker the abstraction."). This is also discussed in this "Hacking with Andrew and Brad" episode (1h, and I don't recall the exact time stamp, sorry); the gist of the discussion then is that Go is not Java and so it's not necessary to define interface inclusion on types before-hand.

I believe it would be a mistake and an antipattern to add the method to the interface at this stage.

mrodm · 2025-12-02T10:40:00Z

Not sure about adding the new method to the DockerComposeAgentDeployer without adding it to the AgentDeployer interface.

This was intentional and follows the Go approach to progressive code repair. The interface advertises the generic consumer's interface, but we are not the generic consumer. It's not needed right now (and likely never will be needed), so it should not be added until it is. This is for three reasons: 1. it's a breaking change, 2. a dummy method would have to be added to all the other implementers of the interface (related to 1.) and 3. "The bigger the interface, the weaker the abstraction."). This is also discussed in this "Hacking with Andrew and Brad" episode (1h, and I don't recall the exact time stamp, sorry); the gist of the discussion then is that Go is not Java and so it's not necessary to define interface inclusion on types before-hand.

I believe it would be a mistake and an antipattern to add the method to the interface at this stage.

Agreed, let's keep that function there just in DockerComposeAgentDeployer.
Thanks for the references!

teresaromero

👍🏻

andrewkroh

LGTM.

andrewkroh · 2025-12-09T23:35:24Z

internal/testrunner/script/agents.go

+		ts.Fatalf("unsupported: ! compile_registry_state")
+	}
+	clearStdStreams(ts)
+	flg := flag.NewFlagSet("uninstall", flag.ContinueOnError)


Suggested change

flg := flag.NewFlagSet("uninstall", flag.ContinueOnError)

flg := flag.NewFlagSet("compile_registry_state", flag.ContinueOnError)

andrewkroh · 2025-12-09T23:38:34Z

docs/howto/script_testing.md

-  - `install_agent [-profile <profile>] [-timeout <duration>] [<network_name_label>]`: install an Elastic Agent policy, setting the environment variable named in the positional argument
+  - `install_agent [-profile <profile>] [-timeout <duration>] [-container_name <container_name_label>] [-network_name <network_name_label>]`: install an Elastic Agent policy, setting the environment variables named in the container_name and network_name arguments
  - `uninstall_agent [-profile <profile>] [-timeout <duration>]`: remove an installed Elastic Agent policy
+  - `compile_registry_state [-start <first_id_to_use>] <path_to_registry_log>`: compile a Filebeat registry log.json file into a registry state and print it to stdout


There's also the -pretty flag to document.

efd6 · 2025-12-10T00:53:02Z

/test

efd6 · 2025-12-10T01:17:23Z

/test

elasticmachine · 2025-12-10T01:47:15Z

💚 Build Succeeded

Buildkite Build
Commit: 2569657

History

💔 Build #6792 failed 2569657
💔 Build #6791 failed 2569657
💚 Build #6736 succeeded 46595e0
💚 Build #6711 succeeded cecdc18
💔 Build #6710 failed c7f5df1
💔 Build #6709 failed 75b1f2c

cc @efd6

internal/testrunner/runners/script: provide mechanism to inspect agen…

e9ae6a0

…t containers

efd6 self-assigned this Nov 25, 2025

efd6 added enhancement New feature or request Team:Ecosystem Label for the Packages Ecosystem team Team:Security-Service Integrations Security Service Integrations Team labels Nov 25, 2025

efd6 commented Nov 25, 2025

View reviewed changes

efd6 marked this pull request as ready for review November 25, 2025 05:32

efd6 requested review from a team and andrewkroh November 25, 2025 05:32

move project name rendering to agentdeployer package

b126a3d

efd6 force-pushed the script_test_agent_container_identity branch from 75b1f2c to c7f5df1 Compare November 27, 2025 00:15

add compile_registry_state command

cecdc18

efd6 force-pushed the script_test_agent_container_identity branch from c7f5df1 to cecdc18 Compare November 27, 2025 00:19

mrodm previously approved these changes Dec 1, 2025

View reviewed changes

internal/testrunner/script/agents.go Outdated Show resolved Hide resolved

internal/testrunner/script/agents.go Outdated Show resolved Hide resolved

mrodm reviewed Dec 1, 2025

View reviewed changes

mrodm approved these changes Dec 1, 2025

View reviewed changes

address pr comment

46595e0

mrodm approved these changes Dec 2, 2025

View reviewed changes

teresaromero approved these changes Dec 2, 2025

View reviewed changes

andrewkroh reviewed Dec 9, 2025

View reviewed changes

address pr comments

2569657

efd6 merged commit 5f2e6f3 into elastic:main Dec 10, 2025
3 checks passed

	func (d *DockerComposeAgentDeployer) agentName() string {
	name := d.packageName
	if d.dataStream != "" && d.dataStream != "." {
	name = fmt.Sprintf("%s-%s", name, d.dataStream)
	}
	return name
	}

	flg := flag.NewFlagSet("uninstall", flag.ContinueOnError)
	flg := flag.NewFlagSet("compile_registry_state", flag.ContinueOnError)

Conversation

efd6 commented Nov 25, 2025

Uh oh!

efd6 Nov 25, 2025

Choose a reason for hiding this comment

Uh oh!

mrodm Nov 26, 2025

Choose a reason for hiding this comment

Uh oh!

mrodm left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

mrodm Dec 1, 2025

Choose a reason for hiding this comment

Uh oh!

mrodm Dec 1, 2025

Choose a reason for hiding this comment

Uh oh!

mrodm Dec 1, 2025

Choose a reason for hiding this comment

Uh oh!

efd6 commented Dec 1, 2025

Uh oh!

mrodm commented Dec 2, 2025

Uh oh!

teresaromero left a comment

Choose a reason for hiding this comment

Uh oh!

andrewkroh left a comment

Choose a reason for hiding this comment

Uh oh!

andrewkroh Dec 9, 2025

Choose a reason for hiding this comment

Uh oh!

andrewkroh Dec 9, 2025

Choose a reason for hiding this comment

Uh oh!

efd6 commented Dec 10, 2025

Uh oh!

efd6 commented Dec 10, 2025

Uh oh!

elasticmachine commented Dec 10, 2025

💚 Build Succeeded

History

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants