Added dry-run mode command line option (-n, --dry-run). #283
Conversation
When invoked, Docuum will report what images would have been deleted during it's initial vacuuming run at start up then exit. In order to do this, Docuum now creates a list of images to delete ensuring that by deleting them it will meet the space requirements set forth by the user.
mardambey
force-pushed
the
dry-run-mode
branch
from
462ecd5
to
beb9c7d
Compare
|
Hi @mardambey, thanks for creating this pull request! I'll review it when I can. In the meantime, would you mind rebasing onto the latest commit on |
This should be up to date with Thank you! |
There was a problem hiding this comment.
Added some initial feedback, but I haven't had time for a full review yet (it might take some time—my apologies).
Since this PR affects how non-dry-run behavior works, we will have to be extremely careful. Some big companies including Netflix and Airbnb rely on this.
| .long(DRY_RUN_OPTION) | ||
| .required(false) | ||
| .takes_value(false) | ||
| .help("Dry run mode, prevents deletion of any images."), |
There was a problem hiding this comment.
For consistency with the other flag descriptions, don't punctuate as a sentence
.help("Dry run mode, prevents deletion of any images"),| @@ -85,6 +85,7 @@ struct ImageRecord { | |||
| parent_id: Option<String>, | |||
| created_since_epoch: Duration, | |||
| repository_tags: Vec<RepositoryTag>, // [ref:at_least_one_repository_tag] | |||
| size: u128, | |||
There was a problem hiding this comment.
For clarity:
size: u128, // In bytes(It is in bytes, right?)
| @@ -96,6 +97,15 @@ struct ImageNode { | |||
| ancestors: usize, // 0 for images with no parent or missing parent | |||
| } | |||
|
|
|||
| // Builds an image's name from as repository:tag | |||
| @@ -928,6 +995,7 @@ mod tests { | |||
| repository: String::from("alpine"), | |||
| tag: String::from("latest"), | |||
| }], | |||
| size: 30000, | |||
There was a problem hiding this comment.
Why not 30_000 for consistency with the other sizes?
| @@ -858,6 +917,14 @@ pub fn run(settings: &Settings, state: &mut State, first_run: &mut bool) -> io:: | |||
| )) | |||
| } | |||
|
|
|||
| // Spawn `docker events --format '{{json .}}'`. | |||
| fn docker_events_listener() -> io::Result<Child> { | |||
There was a problem hiding this comment.
Can you make this function return the ScopeGuard (or just leave it inlined as it was)? The concern is that this creates a runaway process that is easy to forget about.
| "--format", | ||
| "{{.ID}}\\t{{.Repository}}\\t{{.Tag}}\\t{{.CreatedAt}}", | ||
| "{{range .Images}}{{.ID}}\\t{{.Repository}}\\t\ | ||
| {{.Tag}}\\t{{.CreatedAt}}\\t{{.UniqueSize}}\n{{end}}", |
There was a problem hiding this comment.
Is there a reason to double escape the tabs but only single escape the line break?
| match image_records.entry(id.to_owned()) { | ||
| // The id is in the format: | ||
| // sha256:745895703263072416be27b333d19eff4494b287001f6c6adddd22b963a3429d | ||
| // We only want the first 12 characters of the hash. |
| // The id is in the format: | ||
| // sha256:745895703263072416be27b333d19eff4494b287001f6c6adddd22b963a3429d | ||
| // We only want the first 12 characters of the hash. | ||
| let id = sha256_id.get(7..19).unwrap(); |
There was a problem hiding this comment.
Repository convention: all unwraps should be justified with a comment (even if obvious).
| } | ||
|
|
||
| let new_space = space_usage()?; |
There was a problem hiding this comment.
So this is using a different mechanism to calculate the space usage than the loop that computes images_to_delete. I'm a bit concerned about what happens if these different methodologies disagree.
|
(Closing old PRs due to inactivity.) |
When invoked, Docuum will report what images would have been deleted during it's initial vacuuming run at start up then exit.
In order to do this, Docuum now creates a list of images to delete ensuring that by deleting them it will meet the space requirements set forth by the user.
Fixes: #242