Prune update by vrutkovs · Pull Request #839 · coreos/coreos-assembler

vrutkovs · 2019-10-10T16:36:58Z

This PR reworks coreos-assembler prune and adds new commands:

bump_timestamp used during the build to update builds.json timestamp
cleanup-bucket to remove unreferenced builds in the bucket
prune is now python and removes old builds from build.json (this won't be started during build procedure)

TODO:

Fetch arches from unreferenced builds (currently x86_64 is hardcoded)
Cleanup azure and gcp instances
~~Removed --skip-prune option breaks RHCOS pipeline~~

jlebon · 2019-10-10T18:51:57Z

Woah, that's a lot of commits.

bash wrapper was replaced with python version

Awesome! 👍

Can we start with splitting this out into some prep PRs with fewer changes? (Roughly keeping the "new features" here, and splitting out the "cleanups", both loosely defined :) ).

cgwalters · 2019-10-10T19:52:00Z

Definitely agree on splitting out a prep cleanup PR first, looks like you already have things split up nicely!

One other big topic is...before this, cmd-prune was just about the local storage. Having it turn from a local-only command into a remote command is...tricky.

See also #823 which introduced some notion of "local state" vs "remote state" into cosa.

I wonder if what we really want is cmd-remote-prune or so? Or, I guess just error out if one has local changes?

src/cmd-prune

miabbott · 2019-10-11T14:05:32Z

Love the direction this is heading. One thing I'll mention is that we need a way to indicate that some builds should never be pruned; i.e. official releases. Maybe needs changes to builds.json schema to have a field for this?

Doesn't (and probably shouldn't) have to be fully implemented here, but something to keep in mind.

vrutkovs · 2019-10-11T14:53:32Z

some builds should never be pruned; i.e. official releases.

Agree, the defaults are to keep all builds and remove just unreferenced builds (stored in S3, but not found in builds.json). Perhaps its worth splitting this into a dedicated subcommand instead?

jlebon · 2019-10-11T15:46:36Z

One thing I'll mention is that we need a way to indicate that some builds should never be pruned; i.e. official releases.

We can also use tags for that. The prune logic already saves those.

Also slightly leaning towards a new command for this, though no strong opinion either way.

vrutkovs · 2019-10-15T12:07:39Z

One thing I'll mention is that we need a way to indicate that some builds should never be pruned; i.e. official releases. Maybe needs changes to builds.json schema to have a field for this?

Prune won't remove tagged builds

Also slightly leaning towards a new command for this, though no strong opinion either way.

Implemented this - bump-timestamp won't touch the builds and simply prepare builds/builds.json. prune and cleanup-bucket won't be called in the pipeline (yet) and would remove builds or unreferenced keys in the bucket

src/cmd-build

src/cmd-cleanup-bucket

src/bump-timestamp

cgwalters · 2019-10-15T14:53:46Z

Agree, the defaults are to keep all builds and remove just unreferenced builds (stored in S3, but not found in builds.json).

One thing to keep in mind (doesn't/shouldn't be done in this PR) is that we also need "unreferenced tagged resource" pruning for e.g. AMIs.

vrutkovs · 2019-10-30T11:31:44Z

All issues fixed, PTAL

jlebon

Just one comment, otherwise LGTM! Can you rebase, drop the debugging bits, and clean up the commit history?

src/cmd-build

cgwalters · 2019-10-30T13:31:38Z

.cci.jenkinsfile

              cosa_cmd("fetch")
              cosa_cmd("build")
-              cosa_cmd("kola run")
+              //cosa_cmd("kola run")


Please drop this now that #879 landed

cgwalters · 2019-10-30T13:32:07Z

.cci.jenkinsfile

+              // cosa_cmd("buildextend-openstack")
+              // cosa_cmd("buildextend-vmware")
+              // cosa_cmd("compress")
+              // cosa_cmd("buildupload --dry-run s3 --acl=public-read my-nonexistent-bucket/my/prefix")


Also need to uncomment this before we merge.

.cci.jenkinsfile

cgwalters · 2019-10-30T13:45:55Z

src/cmd-prune

+#!/usr/bin/python3 -u
+
+'''
+    This script removes previous builds. DO NOT USE on production pipelines


Mmmm. I think it's a bit more nuanced than that. For RHCOS we can prune bootimages not pinned by the installer, for example.

But...this gets to an interesting topic. I've been thinking we should explicitly copy/promote bootimages we want to pin to a separate "stream". But anyways, just noting this.

Prune won't touch "tagged" builds - seems this field should be updated with pinned images used in installer

vrutkovs · 2019-10-30T17:23:57Z

Are we happy with test results? I'm going to drop prune debug commit and squash into a series of commits

.cci.jenkinsfile

jlebon

Can we squash the commits down? How about just two:

one that reworks cmd-prune + auxiliary
one that introduces cmd-remote-prune

.cci.jenkinsfile

jlebon

Cool, this looks good to me. Thanks a lot for working on this! There are a few extraneous lines that need to be cleaned up, but not going to block on it.

Will leave it open for a bit for folks to have a chance to have a final look.

src/cmd-prune

src/cosalib/prune.py

jlebon · 2019-10-31T20:31:59Z

Just offhand, a few follow-up steps I see for remote-prune after this:

Instead of relying on AMIs listed in the build metadata, we should query EC2 directly for the list of AMIs and cross-check. That way, if we're interrupted or they're desynchronized somehow, we don't leak images.
Support other clouds as well.

* Split "insert_build" helper out of cmd-prune * Rework cmd-prune in python * Add prune tests

This subcommand cleans up unreferenced builds on remote location. Currently only s3 buckets are implemented

vrutkovs requested review from cgwalters and jlebon October 10, 2019 16:37

vrutkovs mentioned this pull request Oct 11, 2019

cosalib: create libs for s3 and ec2 operations #840

Merged

miabbott reviewed Oct 11, 2019

View reviewed changes

src/cmd-prune Outdated Show resolved Hide resolved

vrutkovs force-pushed the prune-rewrite branch 15 times, most recently from 3864f6f to 2706252 Compare October 15, 2019 11:28

vrutkovs changed the title ~~WIP Prune update~~ Prune update Oct 15, 2019

vrutkovs force-pushed the prune-rewrite branch 2 times, most recently from 08eb5b3 to aff4b07 Compare October 15, 2019 12:59

jlebon reviewed Oct 15, 2019

View reviewed changes

src/cmd-build Outdated Show resolved Hide resolved

src/cmd-cleanup-bucket Show resolved Hide resolved

src/bump-timestamp Outdated Show resolved Hide resolved

vrutkovs force-pushed the prune-rewrite branch 3 times, most recently from aeba046 to 8ed89a1 Compare October 30, 2019 10:58

jlebon reviewed Oct 30, 2019

View reviewed changes

src/cmd-build Outdated Show resolved Hide resolved

cgwalters reviewed Oct 30, 2019

View reviewed changes

vrutkovs force-pushed the prune-rewrite branch 2 times, most recently from 631adbc to c6fcef4 Compare October 30, 2019 17:23

vrutkovs force-pushed the prune-rewrite branch from c6fcef4 to 1a3fb5d Compare October 30, 2019 17:51

jlebon reviewed Oct 30, 2019

View reviewed changes

.cci.jenkinsfile Outdated Show resolved Hide resolved

.cci.jenkinsfile Outdated Show resolved Hide resolved

.cci.jenkinsfile Outdated Show resolved Hide resolved

.cci.jenkinsfile Outdated Show resolved Hide resolved

vrutkovs force-pushed the prune-rewrite branch 8 times, most recently from 4da4bc0 to db277a5 Compare October 31, 2019 17:42

jlebon reviewed Oct 31, 2019

View reviewed changes

.cci.jenkinsfile Outdated Show resolved Hide resolved

vrutkovs force-pushed the prune-rewrite branch from db277a5 to 2b351b0 Compare October 31, 2019 18:39

jlebon approved these changes Oct 31, 2019

View reviewed changes

src/cmd-prune Outdated Show resolved Hide resolved

src/cosalib/prune.py Outdated Show resolved Hide resolved

Vadim Rutkovsky added 2 commits November 1, 2019 11:20

Rework cmd-prune

a8d9031

* Split "insert_build" helper out of cmd-prune * Rework cmd-prune in python * Add prune tests

Add "remote-prune" subcommand

ccec60d

This subcommand cleans up unreferenced builds on remote location. Currently only s3 buckets are implemented

vrutkovs force-pushed the prune-rewrite branch from 2b351b0 to ccec60d Compare November 1, 2019 10:20

jlebon merged commit 3f6f29c into coreos:master Nov 1, 2019

This was referenced Nov 4, 2019

remote-prune should support other clouds #889

Open

remote-prune should query EC2 for AMIs #890

Open

vrutkovs mentioned this pull request Nov 5, 2019

[4.3] Rework prune #893

Closed

Conversation

vrutkovs commented Oct 10, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jlebon commented Oct 10, 2019

Uh oh!

cgwalters commented Oct 10, 2019

Uh oh!

Uh oh!

miabbott commented Oct 11, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

vrutkovs commented Oct 11, 2019

Uh oh!

jlebon commented Oct 11, 2019

Uh oh!

vrutkovs commented Oct 15, 2019

Uh oh!

Uh oh!

Uh oh!

Uh oh!

cgwalters commented Oct 15, 2019

Uh oh!

vrutkovs commented Oct 30, 2019

Uh oh!

jlebon left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

cgwalters Oct 30, 2019

Choose a reason for hiding this comment

Uh oh!

cgwalters Oct 30, 2019

Choose a reason for hiding this comment

Uh oh!

Uh oh!

cgwalters Oct 30, 2019

Choose a reason for hiding this comment

Uh oh!

vrutkovs Oct 30, 2019

Choose a reason for hiding this comment

Uh oh!

vrutkovs commented Oct 30, 2019

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jlebon left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

jlebon left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

jlebon commented Oct 31, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

vrutkovs commented Oct 10, 2019 •

edited

Loading

miabbott commented Oct 11, 2019 •

edited

Loading