[devbin] Generate cleanup policies for GCP artifact registry #13489
Conversation
| image = image.strip() | ||
| package = image.split(':')[0] | ||
| if package not in third_party_packages: | ||
| third_party_packages.append(package) |
There was a problem hiding this comment.
Can we keep only the tags listed there?
There was a problem hiding this comment.
I guess that means we need #-of-package policies, which is currently 11.
There was a problem hiding this comment.
I grabbed all of the tags if they exist, but I'm concerned because these are tag prefix matches so it's not 100% specific to the package.
There was a problem hiding this comment.
Yeah, I think if we can't do something exact we should do nothing at all. Better to not have a false sense of security.
|
|
||
|
|
||
| deploy_packages = [] | ||
| with open('build.yaml', 'r') as f: |
There was a problem hiding this comment.
Also get the images from ci/test/resources/build.yaml
| return data | ||
|
|
||
|
|
||
| class ConditionalKeepPolicy(CleanupPolicy): |
There was a problem hiding this comment.
This is just a KeepPolicy, right? Or, otherwise, the above is a ConditionalDeletePolicy?
There was a problem hiding this comment.
I just used their nomenclature in the docs
| DeletePolicy('delete_test_pr', 'tagged', tag_prefixes=['test-pr-'], older_than='3d'), | ||
| DeletePolicy('delete_test_deploy', 'tagged', tag_prefixes=['test-deploy-'], older_than='3d'), | ||
| DeletePolicy('delete_cache', 'tagged', tag_prefixes=['cache-pr-'], older_than='7d'), | ||
| DeletePolicy('delete_cache', 'tagged', tag_prefixes=['cache-'], older_than='30d'), |
There was a problem hiding this comment.
Are the names required to be unique?
There was a problem hiding this comment.
I'm referring to the names of the policies themselves; delete_cache appears twice.
|
@daniel-goldstein is it possible to run a script and use that as input to terraform? Or is it easier to bake in a policy file and then verify in CI that the policy file has not changed? |
|
In the former, do you mean to run terraform in CI? The latter seems easier to me given our current system. That being said, our current system isn't great since it does not let you have more than 1 concurrent PR that wants to make terraform changes. |
|
I was thinking either:
The latter seems better to me since we know the current state of the cleanup policy on any commit by looking at a file rather than executing some code. |
|
Ah, I see. 1. is definitely possible, but I agree with you that I prefer 2 because it is checked by our system. |
There was a problem hiding this comment.
Excellent, my final request is to:
- Run this and check in the result (store it in
infra/gcp-broad/). - Add a build.yaml step that runs this and
diffs against the checked in file. Fail if they differ. - Install the policy in hail-vdc.
- Create a bug issue that asks us to import this into terraform once hashicorp/terraform-provider-google-beta@bc4aa51 is publicly released (see also the overarching tracking issue). It's not in 4.79.0 (see what was added since then). Releases appear to happen ~once a week, so we should be able to import into terraform in September.
|
Once you've looked over the policy generated, I'll apply it. Otherwise, this is ready to go. |
|
Once it merges, please apply it |
jigold
force-pushed
the
ar-cleanup-policy
branch
from
5d1407f to
16c2fb4
Compare
Fixes #13441
Usage:
python3 devbin/generate_gcp_ar_cleanup_policy.py > my_policy_file.txtReference: https://cloud.google.com/artifact-registry/docs/repositories/cleanup-policy
Note, we have a maximum of 10 policies available.