-
Notifications
You must be signed in to change notification settings - Fork 118
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.
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
Shorten docker container name #1798
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, but I wonder if we can have a more future-proof approach.
@@ -163,7 +164,7 @@ func (d *DockerComposeAgentDeployer) SetUp(ctx context.Context, agentInfo AgentI | |||
} | |||
|
|||
// Service name defined in the docker-compose file | |||
agentInfo.Name = dockerTestAgentNamePrefix | |||
agentInfo.Name = dockerTestAgentServiceName |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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
.
There was a problem hiding this comment.
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
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changed here: a161483
💚 Build Succeeded
History
cc @mrodm |
Relates #787
Relates elastic/integrations#9660
Until now the elastic agent names created (
hostname
) are set as:elastic-agent-<package>-<data-stream>-<run_id>
Using that pattern, it caused that for some packages the resulting name is longer than 64 characters causing an error in docker-compose.
This PR changes the prefix to be just
agent
. For the example above it results in 58 characters: