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

Bug 1875338 - Update the clean-up script to manage each package in the product delivery repository #7

Merged
merged 5 commits into from
Mar 1, 2024

Conversation

gabrielBusta
Copy link
Member

@gabrielBusta gabrielBusta commented Feb 28, 2024

I hacked together the clean-up script as fast as I could to mitigate the Nightly channel growing. These changes update the script to mange each Mozilla product package in the product delivery repository.
See: https://bugzilla.mozilla.org/show_bug.cgi?id=1875338

@codecov-commenter
Copy link

codecov-commenter commented Feb 28, 2024

Codecov Report

Attention: Patch coverage is 22.97297% with 57 lines in your changes are missing coverage. Please review.

Project coverage is 25.00%. Comparing base (5c4e893) to head (9520374).

Files Patch % Lines
src/mozilla_linux_pkg_manager/cli.py 22.97% 57 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main       #7      +/-   ##
==========================================
+ Coverage   17.05%   25.00%   +7.94%     
==========================================
  Files           2        2              
  Lines         170      116      -54     
  Branches       32       16      -16     
==========================================
  Hits           29       29              
+ Misses        141       87      -54     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@gabrielBusta gabrielBusta force-pushed the bug-1875338 branch 3 times, most recently from 0fc7c3c to ad60f6e Compare February 28, 2024 22:19
…e product delivery repository

I hacked together the clean-up script as fast as I could to mitigate the Nightly channel growing. These changes update the script to mange each Mozilla product package in the product delivery repository.
@gabrielBusta
Copy link
Member Author

gabrielBusta commented Feb 29, 2024

I realize these are a lot of changes.

The main thing I did was switch from scraping the raw repository meta data files to using the artifact registry APIs.
I had to do this to get the create time of package versions without relying on a timestamp in the version (that is what I did for the nightly only version of this.)

The package selection is done using a regex now, so it is flexible enough to handle different products and product flavors.

Another thing to note, is that switching to the artifact registry APIs allows me to use the SDKs built-in retry mechanism.

These is an example of using the script to do a clean-up dry-run for devedition/beta packages:

mozilla-linux-pkg-manager \
clean-up \
--package "^firefox-(devedition|beta)(-l10n-.+)?$" \
--retention-days 60 \
--repository mozilla \
--region us \
--skip-delete \
2>&1 | tee clean-up-devedition-and-beta.log

Also, I started working on unit tests for this module, but I decided to branch off and do a follow up, so that devedition and beta don't get to bloated and we can keep the repository's index size hovering around the same number.

Here's what that looks like so far: 67b0832

@gabrielBusta gabrielBusta marked this pull request as ready for review February 29, 2024 19:16
@gabrielBusta
Copy link
Member Author

Here's a fresh log: devedition-beta-moz-fx-productdelivery-pr-38b5.log

@gabrielBusta
Copy link
Member Author

Here's one from the staging environment: devedition-beta-moz-fx-productdelivery-no-7d6a.log

and one here's on from dev: all-packages-moz-fx-dev-releng.log

Copy link

@bhearsum bhearsum left a comment

Choose a reason for hiding this comment

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

Do we have another place that these are stored for the long term, or are we OK deleting them forever? I will note that other nightlies and releases are kept forever on archive.mozilla.org for bisection and future debugging purposes - so deleting these packages forever would be going against the grain.

)
clean_up_parser.add_argument(
"--dry-run",
action="store_true",
help="Do a no-op run and print out a summary of the operations that will be executed",
default=False,
)
clean_up_parser.add_argument(
Copy link

Choose a reason for hiding this comment

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

What is the benefit of skip-delete vs. running in dry-run mode? It looks to me like the latter is a more extensive test, at perhaps the cost of some extra network costs?

Copy link
Member Author

@gabrielBusta gabrielBusta Mar 1, 2024

Choose a reason for hiding this comment

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

The difference is dry-run will fail if the SDK doesn't have delete permissions on the resource.

Copy link

Choose a reason for hiding this comment

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

Ah - I see. So this allows for running a dry run with a lower permissioned account - makes sense!

if pattern.match(name):
logging.info(f"Looking for expired package versions of {name}...")
versions = await list_versions(package)
async for version in versions:
Copy link

Choose a reason for hiding this comment

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

I think you either want to drop the async here or use async for version in list_versions(package). As far as I can tell, versions is already a simple list as things are written now (because you await'ed it). The same thing applies a bit further up with packages.

Copy link
Member Author

Choose a reason for hiding this comment

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

The client returns async iterators.
For ex. ListPackagesAsyncPager in the case of list_versions (not a plain list.)

Copy link

Choose a reason for hiding this comment

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

Oh my bad - sorry! Yes, you're absolutely right.

for batch in batches:
logging.info(
f"Deleting {format(len(batch), ',')} expired package versions for {package}."
f"Deleting {format(len(batch), ',')} expired package versions of {os.path.basename(package)}"
Copy link

Choose a reason for hiding this comment

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

Please change this log message when in dry run mode - "Deleting ..." is not accurate for that case. "Would delete ..." or something similar would be better.

@gabrielBusta
Copy link
Member Author

gabrielBusta commented Mar 1, 2024

Do we have another place that these are stored for the long term, or are we OK deleting them forever? I will note that other nightlies and releases are kept forever on archive.mozilla.org for bisection and future debugging purposes - so deleting these packages forever would be going against the grain.

These .debs have to be stored in the archive before they can be in the APT repository. We don't do direct uploads to the repository, we import the packages from the archive (and they are kept in the archive forever.)

@gabrielBusta
Copy link
Member Author

@bhearsum
Copy link

bhearsum commented Mar 1, 2024

Do we have another place that these are stored for the long term, or are we OK deleting them forever? I will note that other nightlies and releases are kept forever on archive.mozilla.org for bisection and future debugging purposes - so deleting these packages forever would be going against the grain.

These .debs have to be stored in the archive before the can be in the APT repository. We don't do direct uploads to the repository, we import the packages from the archive (and they are kept in the archive forever.)

OK, great!

@gabrielBusta gabrielBusta merged commit 6948104 into main Mar 1, 2024
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants