Skip to content

Shorten docker container name #1798

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Apr 25, 2024
Merged

Shorten docker container name #1798

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
558002a
Try to shorten docker container name
mrodm Apr 23, 2024
a161483
Use just elastic-agent plus RunID as hostname
mrodm Apr 24, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions internal/agentdeployer/agent.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ import (
)

const (
dockerTestAgentNamePrefix = "elastic-agent"
dockerTestAgentServiceName = "elastic-agent"
dockerTestAgentDockerCompose = "docker-agent-base.yml"
defaultAgentPolicyName = "Elastic-Agent (elastic-package)"
)
Expand Down Expand Up @@ -163,7 +163,7 @@ func (d *DockerComposeAgentDeployer) SetUp(ctx context.Context, agentInfo AgentI
}

// Service name defined in the docker-compose file
agentInfo.Name = dockerTestAgentNamePrefix
agentInfo.Name = dockerTestAgentServiceName
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if with this solution we are still at risk of having the same issue if some package or data stream name is longer.

Could we truncate the package/data stream part?

So for elastic-agent-carbon_black_cloud-asset_vulnerability_summary-62403 we'd have the truncated version elastic-agent-carbon_black_cloud-asset_vulnerability_s-62403.

I would be ok though with applying both changes, the change in the prefix and the change to truncate.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or maybe we could just call it elastic-agent plus the run id, that should be unique: elastic-agent-62403.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The goal to add the package and datastream was to help the user to find the agent in the UI. It's true that as it is right now, there could be issues again if some package or data stream name is longer, so this solution would not work if that happen.

Until now the agent was set as docker-fleet-agent. Maybe it's not needed to set in the agent name all that information, since the agents could be found filtering by the Agent Policy. Do you thin that would be good @jsoriano ?

So I think it could be set just adding as a suffix the RunID: elastic-agent-12345

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed here: a161483

agentName := agentInfo.Name

opts := compose.CommandOptions{
Expand Down Expand Up @@ -223,7 +223,7 @@ func (d *DockerComposeAgentDeployer) SetUp(ctx context.Context, agentInfo AgentI
}

func (d *DockerComposeAgentDeployer) agentHostname() string {
return fmt.Sprintf("%s-%s-%s", dockerTestAgentNamePrefix, d.agentName(), d.agentRunID)
return fmt.Sprintf("%s-%s", dockerTestAgentServiceName, d.agentRunID)
}

func (d *DockerComposeAgentDeployer) agentName() string {
Expand Down