Skip to content
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

Update cleanup to remove service logs from independent Elastic Agents #2033

Merged
merged 4 commits into from
Aug 29, 2024

Conversation

mrodm
Copy link
Contributor

@mrodm mrodm commented Aug 22, 2024

Update elastic-package cleanup to remove also service logs folders created for independent Elastic Agents.

Tested with nginx (from test package) and sql_input (from integrations repository).

How to test locally

elastic-package stack up -v -d

cd test/packages/parallel/nginx

# run tests and press Ctrl-C once the process is waiting for elastic-agent containers to be healthy
elastic-package test system -v

# check that some folders have been created in the profile default folder:
ls -l ~/.elastic-package/profiles/default/service_logs

# for instance
# drwxr-xr-x 2 user user 4096 ago 28 12:40 agent-nginx-access-85847
# drwxr-xr-x 2 user user 4096 ago 28 12:40 agent-nginx-error-59191
# drwxr-xr-x 2 user user 4096 ago 28 12:40 agent-nginx-stubstatus-91770

# run cleanup command
elastic-package clean -v

# the previous folders in the service logs dir should be deleted
ls -l ~/.elastic-package/profiles/default/service_logs

@mrodm mrodm self-assigned this Aug 22, 2024
packageFolder := filepath.Base(packageRootPath)
serviceLogsDir := agentdeployer.ServiceLogsDir(profile)

folderGlob := fmt.Sprintf("%s/agent-%s-*", serviceLogsDir, packageFolder)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

As this package removes the built packages of the given package (folder), I tried to remove just the service logs folders for the given package.

Copy link
Member

Choose a reason for hiding this comment

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

Do we use the name of the directory for these logs directories or the name of the package in the manifest? (There are packages with names different to their directories).
Maybe we could have a function to obtain this directory path, so we use the same when creating and when deleting it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Looking at the code, it was the folder package name what it was used.
But it is true, that this was not clear and easy to know from the code.

I tried to refactor the code a little in 031c2cc to help about that.

@mrodm mrodm marked this pull request as ready for review August 22, 2024 16:03
@mrodm mrodm requested a review from a team August 22, 2024 16:04
internal/cleanup/service_logs.go Outdated Show resolved Hide resolved
packageFolder := filepath.Base(packageRootPath)
serviceLogsDir := agentdeployer.ServiceLogsDir(profile)

folderGlob := fmt.Sprintf("%s/agent-%s-*", serviceLogsDir, packageFolder)
Copy link
Member

Choose a reason for hiding this comment

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

Do we use the name of the directory for these logs directories or the name of the package in the manifest? (There are packages with names different to their directories).
Maybe we could have a function to obtain this directory path, so we use the same when creating and when deleting it.

Co-authored-by: Jaime Soriano Pastor <jaime.soriano@elastic.co>
@elasticmachine
Copy link
Collaborator

💚 Build Succeeded

History

cc @mrodm

@mrodm mrodm requested a review from jsoriano August 28, 2024 13:57
@mrodm mrodm merged commit 7775af6 into elastic:main Aug 29, 2024
3 checks passed
@mrodm mrodm deleted the update-cleanup-command branch August 29, 2024 08:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants