-
Notifications
You must be signed in to change notification settings - Fork 13
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
cmd/plume: Add a prune command to clean up developer images #115
Conversation
bb16fc3
to
8391196
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great, thanks a lot, looks good but just a few small changes are needed.
cmd/plume/prune.go
Outdated
duration := now.Sub(lastModifiedDate) | ||
daysOld := int(duration.Hours() / 24) | ||
if daysOld >= days { | ||
plog.Infof("Old blob %q: %d days old\n", blob.Name, daysOld) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you add a check for the blob name prefix to be flatcar-linux-
and suffix be .vhd
? Then it will not delete other things like flatcar-beta-cert-test_02_28_2020_03_08_18.zip
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, and also check the channel, forgot that in the review (--channel edge
currently still enumerates other channels like flatcar-linux-1772.0.0-alpha.vhd
).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Uhm... Ok. I had only tested it with the developer channel, which goes to a different bucket, so it's not relevant. I'll figure out a way of filtering the blobs per channel.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Alright this was a bit more involved than expected, but it's working now. It checks that the stored metadata filename is the one from the spec, and also that the blob name includes the channel name being processed.
cmd/plume/prune.go
Outdated
pruneDryRun bool | ||
cmdPrune = &cobra.Command{ | ||
Use: "prune [options]", | ||
Short: "Prune old release images.", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have to specify --version abc
because the other commands require it. Can you please document that --version
will be ignored?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
and maybe document that it takes --channel
into account
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I mentioned that in the how to test command (I was using --version none
), but I figured that instead of documenting this brokenness, it's better to just override it. I've added a line to always set the version to none.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've documented the fact that it requires the channel now.
cmd/plume/prune.go
Outdated
} | ||
} | ||
imageFileName := spec.AWS.Image | ||
s3ObjectPath := fmt.Sprintf("%s-usr/%s/%s", *image.Architecture, version, strings.TrimSuffix(imageFileName, filepath.Ext(imageFileName))) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
*image.Architecture
returns x86_64
instead of amd64
and needs some postprocessing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, I think I've fixed this although I don't think it matters because the S3 bucket is empty (I think whatever gets uploaded there gets deleted once it turns into a snapshot).
aa02a1f
to
150a689
Compare
When running developer builds, plume uploads prerelease images to both Azure and AWS. This change adds a new "prune" subcommand for plume, to get rid of artifacts uploaded to old builds. It also adds or improves logging messages on functions. This extra logging was needed for debugging while developing the code.
150a689
to
5e00af9
Compare
cmd/plume: Add a prune command to clean up developer images
When running developer builds, plume uploads prerelease images to both Azure and AWS. This change adds a new "prune" subcommand for plume, to get rid of artifacts uploaded to old builds.
It also adds or improves logging messages on functions. This extra logging was needed for debugging while developing the code.
How to use / Testing done
I've already run this for Azure and AWS with days=20, so to test it, it needs to be run with a smaller number or it won't do anything.
Needed for: kinvolk/PROJECT-flatcar-linux#294