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

Garbage collect with provided retention options #638

Merged
merged 1 commit into from
Apr 7, 2022

Conversation

aryan9600
Copy link
Member

@aryan9600 aryan9600 commented Mar 25, 2022

Adds two ways (a ttl for each artifact and max number of artifact per objects) to customize garbage collection. Adds a timeout context to cancel garbage collection after a duration.

Ref: #602

Signed-off-by: Sanskar Jaiswal jaiswalsanskar078@gmail.com

@aryan9600 aryan9600 changed the title Add logic to garbage collect with the provided retention options Garbage collect with provided retention options Mar 25, 2022
main.go Outdated Show resolved Hide resolved
main.go Outdated Show resolved Hide resolved
controllers/storage.go Outdated Show resolved Hide resolved
@aryan9600 aryan9600 requested a review from pjbgf March 29, 2022 19:57
@aryan9600 aryan9600 force-pushed the garbage-retention branch 4 times, most recently from be78b48 to d9cf6de Compare March 30, 2022 11:17
@aryan9600 aryan9600 requested a review from hiddeco March 30, 2022 13:22
controllers/storage.go Outdated Show resolved Hide resolved
controllers/storage.go Outdated Show resolved Hide resolved
@aryan9600 aryan9600 force-pushed the garbage-retention branch 2 times, most recently from 5e6e6c8 to 68ef63d Compare April 1, 2022 14:10
if err := testStorage.MkdirAll(*obj.Status.Artifact); err != nil {
return err
}
if err := testStorage.AtomicWriteFile(obj.Status.Artifact, strings.NewReader(v), 0644); err != nil {
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
if err := testStorage.AtomicWriteFile(obj.Status.Artifact, strings.NewReader(v), 0644); err != nil {
if err := testStorage.AtomicWriteFile(obj.Status.Artifact, strings.NewReader(v), 0o644); err != nil {

if err := testStorage.MkdirAll(*obj.Status.Artifact); err != nil {
return err
}
if err := testStorage.AtomicWriteFile(obj.Status.Artifact, strings.NewReader("file"), 0644); err != nil {
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
if err := testStorage.AtomicWriteFile(obj.Status.Artifact, strings.NewReader("file"), 0644); err != nil {
if err := testStorage.AtomicWriteFile(obj.Status.Artifact, strings.NewReader("file"), 0o644); err != nil {

controllers/gitrepository_controller.go Outdated Show resolved Hide resolved
controllers/storage.go Outdated Show resolved Hide resolved
controllers/storage.go Outdated Show resolved Hide resolved
controllers/gitrepository_controller.go Outdated Show resolved Hide resolved
@aryan9600 aryan9600 force-pushed the garbage-retention branch 4 times, most recently from d03d02b to d98e93c Compare April 5, 2022 11:41
main.go Outdated Show resolved Hide resolved
@aryan9600 aryan9600 marked this pull request as ready for review April 5, 2022 21:40
controllers/storage_test.go Outdated Show resolved Hide resolved
@hiddeco hiddeco added the enhancement New feature or request label Apr 7, 2022
@aryan9600 aryan9600 force-pushed the garbage-retention branch 2 times, most recently from c46dd51 to dc6b9e5 Compare April 7, 2022 11:03
@hiddeco hiddeco added the area/storage Storage related issues and pull requests label Apr 7, 2022
Copy link
Member

@hiddeco hiddeco left a comment

Choose a reason for hiding this comment

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

This LGTM if the commits are smashed. Thank you @aryan9600 🥇

Copy link
Member

@stefanprodan stefanprodan left a comment

Choose a reason for hiding this comment

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

🚀

Introduce two new flags to configure the ttl of an artifact and the max
no. of files to retain for an artifact. Modify the gc process to
consider the options and use timeouts to prevent the controller from
hanging.
This helps in situations when the SC has already garbage collected the
current artifact but the advertised artifact url is still the same,
which leads to the server returning a 404.

Signed-off-by: Sanskar Jaiswal <jaiswalsanskar078@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/storage Storage related issues and pull requests enhancement New feature or request
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

None yet

5 participants