Skip to content

feat: clean stale provisioner files #9545

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

Merged
merged 27 commits into from
Sep 11, 2023
Merged

feat: clean stale provisioner files #9545

merged 27 commits into from
Sep 11, 2023

Conversation

mtojek
Copy link
Member

@mtojek mtojek commented Sep 6, 2023

Fixes: #7107

This PR introduces cleanup logic for stale provisioner files:

  • unfinished, orphan provisioner sessions
  • unused plugins based on the last "access time" attribute

I tried to implement a really dumb sweeper that doesn't care about the content but wouldn't like to end up with partially removed plugin files, hence the implementation is plugin-aware.

TODO:

  • unit tests for provisioner/terraform/cleanup.go
  • unit tests for provisionersdk/cleanup.go

@mtojek mtojek self-assigned this Sep 6, 2023
@mtojek mtojek marked this pull request as ready for review September 7, 2023 19:01
@mtojek mtojek requested a review from johnstcn September 7, 2023 19:01
@@ -197,6 +197,8 @@ require (
tailscale.com v1.46.1
)

require github.com/djherbis/times v1.5.0
Copy link
Member

Choose a reason for hiding this comment

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

It's super annoying that this isn't exposed in the standard library.
I'm slightly concerned that this module doesn't appear to be very active, but it has no dependencies, so maybe that's OK?

Copy link
Member Author

Choose a reason for hiding this comment

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

I did the self-evaluation of the module, and I agree with you. Fortunately, this is just a basic syscall, so in theory, we could "port" it to coder.

return err
}

accessTime := info.ModTime() // fallback to modTime if accessTime is not available (afero)
Copy link
Member

Choose a reason for hiding this comment

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

Just be aware that modtime is not guaranteed (see: https://www.kernel.org/doc/html/latest/filesystems/api-summary.html?highlight=nocmtime#c.file_update_time)

Howver, I would assume it to at least be the time the file was created.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, that is correct. I was thinking about running a self-test on the cache directory, but on the other hand I wouldn't like to overcomplicate things.

@mtojek
Copy link
Member Author

mtojek commented Sep 8, 2023

@johnstcn If you don't mind, I will merge this after the weekend to prevent any unexpected issues 👍

@johnstcn
Copy link
Member

johnstcn commented Sep 8, 2023

@johnstcn If you don't mind, I will merge this after the weekend to prevent any unexpected issues 👍

👍

chmod 444 /proc/sys/days/friday

@mtojek mtojek enabled auto-merge (squash) September 11, 2023 07:30
@mtojek mtojek merged commit 67fe3ae into main Sep 11, 2023
@mtojek mtojek deleted the 7107-clean-stale-files branch September 11, 2023 07:37
@github-actions github-actions bot locked and limited conversation to collaborators Sep 11, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

clean up coder cache
2 participants