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

fix(agent): cleanup rclone directory on errors #5830

Merged
merged 3 commits into from
Aug 15, 2024

Conversation

lc525
Copy link
Member

@lc525 lc525 commented Aug 13, 2024

Previously, any error occurring after the downloading of the model artifact but
before the model was properly registered in the agent's model repository would
result in the rclone path not being cleaned-up. This leads to the PVC filling up
and requiring end-user manual intervention.

This change fixes this, by cleaning the rclone path even when errors occur.

Which issue(s) this PR fixes:

  • Fixes #INFRA-1126 (internal): rclone directory is not cleaned-up

Previously, any error occurring after the downloading of the model artifact but
before the model was properly registered in the agent's model repository would
result in the rclone path not being cleaned-up. This leads to the PVC filling up
and requiring end-user manual intervention.

This change fixes this, by cleaning the rclone path even when errors occur.
@lc525 lc525 requested a review from sakoush as a code owner August 13, 2024 08:08
@lc525 lc525 added the v2 label Aug 13, 2024
Copy link
Member

@sakoush sakoush left a comment

Choose a reason for hiding this comment

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

lgtm. Is there a way to test this change?

Modifies the httpmock used for rclone testing to enable it to simulate
rclone download failures. Test also checks that the cleanup endpoint
was called at least once (if the download succeeds)
@lc525
Copy link
Member Author

lc525 commented Aug 14, 2024

I did add at least some minimal tests by improving on how we use httpmock.

@lc525 lc525 merged commit 081a856 into SeldonIO:v2 Aug 15, 2024
3 checks passed
@lc525 lc525 deleted the INFRA-1126/pvc-rclone-cleanup branch August 15, 2024 16:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants