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

roachtest: codify longer ttl external storage buckets #110288

Merged
merged 1 commit into from
Sep 14, 2023

Conversation

msbutler
Copy link
Collaborator

@msbutler msbutler commented Sep 8, 2023

Previously, the only codified external buckets for roachtests to back up to was
the cockroachdb-backup-testing buckets in s3 and gcs which each had a ttl of
1 day. This low ttl is not suitable for roachtests that produce backups that
the test failure investigator may want to inspect. This patch codifies the new
cockroachdb-backup-testing-long-ttl buckets in s3 and gcs, which currently
have a ttl of 20 days, the same ttl that team city artifacts have.

This patch also points the c2c, backup-restore/mixed-version, and
disagg-rebalance roachtests to use these new buckets.

Note this PR only points roachtests that run in public TC environments to the
new buckets. A future PR will set the BACKUP_TESTING_BUCKET_LONG_TTL env var
for private roacttests to a new bucket with a longer ttl.

Epic: none

Release note: none

@msbutler msbutler added the T-testeng TestEng Team label Sep 8, 2023
@msbutler msbutler requested a review from renatolabs September 8, 2023 21:47
@msbutler msbutler requested a review from a team as a code owner September 8, 2023 21:47
@msbutler msbutler self-assigned this Sep 8, 2023
@msbutler msbutler requested review from DarrylWong and removed request for a team September 8, 2023 21:47
@cockroach-teamcity
Copy link
Member

This change is Reviewable

@msbutler
Copy link
Collaborator Author

msbutler commented Sep 8, 2023

unrelated extended ci flake with the bench job

@msbutler msbutler requested a review from itsbilal September 13, 2023 13:46
Copy link
Contributor

@renatolabs renatolabs left a comment

Choose a reason for hiding this comment

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

This is great, thanks for working on this!

A couple of small requests:

  • Can we update build/teamcity/internal/cockroach/nightlies/private_roachtest.sh to set the new env var there as well? We should have a similar setup in that project, and tests that run on the private config should not write to the public buckets.
  • Can we update the mixed-version backup-restore test to use the long TTL bucket?

Also, I don't see a TTL configured for the "long TTL" version of the bucket on GCS or for the "short TTL" version of the bucket on AWS. Shouldn't we do it before we merge this?

Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (waiting on @DarrylWong, @itsbilal, and @msbutler)


-- commits line 9 at r1:
FWIW, TC is configured to keep roachtest artifacts for 20 days. Not that I'm suggesting we mirror it in the bucket necessarily, but maybe we amend the commit message to avoid confusion.

Screenshot 2023-09-13 at 11.05.15 AM.png

@renatolabs
Copy link
Contributor

Actually the GCS "long TTL" bucket has a TTL of 30 days which is not aligned with what the code is saying.

Copy link
Member

@itsbilal itsbilal left a comment

Choose a reason for hiding this comment

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

Aside from Renato's comments, this LGTM from a storage perspective.

@msbutler msbutler force-pushed the butler-long-ttl-bucket branch from e723530 to 2bb02ad Compare September 13, 2023 21:59
@msbutler msbutler requested a review from a team as a code owner September 13, 2023 21:59
Copy link
Collaborator Author

@msbutler msbutler left a comment

Choose a reason for hiding this comment

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

Thanks for the review! Some notes:

  • I've changed the ttl for these buckets to 20 days, to align with our tc retention policy. I've updated the commit message and comments to reflect this.
  • I added that env var to privated_roachtest.sh. Do i need I populate that env var somewhere in TC as well?
  • mixed version backup restore now uses the long ttl bucket as well
  • I was planning to delete and remake the short ttl aws bucket a couple days after this merges since it lives in us-east-1 and it should live in us-east-2. I'm hoping we don't need to do a live migration 🙏

Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (waiting on @DarrylWong and @renatolabs)


-- commits line 9 at r1:

Previously, renatolabs (Renato Costa) wrote…

FWIW, TC is configured to keep roachtest artifacts for 20 days. Not that I'm suggesting we mirror it in the bucket necessarily, but maybe we amend the commit message to avoid confusion.

Screenshot 2023-09-13 at 11.05.15 AM.png

huh. It seems most intuitive to mimic how long tc artifacts are around so i changed the ttl's to be 20 days.

@renatolabs
Copy link
Contributor

No changes needed in TC, but the private_roachtest script should set a long TTL bucket as well:

export BACKUP_TESTING_BUCKET="cockroach-backup-testing-private"

I see that the "short TTL" bucket above has no TTL. Do you mind setting a 1-day TTL on that bucket, creating a new, 20-day TTL bucket on the private project (e2e-infra-381422), and setting BACKUP_TESTING_BUCKET_LONG_TTL in that script as well?

Previously, the only codified external buckets for roachtests to back up to was
the `cockroachdb-backup-testing` buckets in s3 and gcs which each had a ttl of
1 day. This low ttl is not suitable for roachtests that produce backups that
the test failure investigator may want to inspect. This patch codifies the new
`cockroachdb-backup-testing-long-ttl` buckets in s3 and gcs, which currently
have a ttl of 20 days, the same ttl that team city artifacts have.

This patch also points the c2c, backup-restore/mixed-version, and
disagg-rebalance roachtests to use these new buckets.

Note this PR only points roachtests that run in public TC environments to the
new buckets. A future PR will set the BACKUP_TESTING_BUCKET_LONG_TTL env var
for private roacttests to a new bucket with a longer ttl.

Epic: none

Release note: none
@msbutler msbutler force-pushed the butler-long-ttl-bucket branch from 2bb02ad to 973ddb1 Compare September 14, 2023 14:51
@msbutler
Copy link
Collaborator Author

I couldn't update/create buckets in the private gce project, so I left a todo and updated the commit message to reflects this. Opening up an internal jira ticket for dev inf.

@msbutler
Copy link
Collaborator Author

TFTR!

bors r=renatolabs

@craig
Copy link
Contributor

craig bot commented Sep 14, 2023

Build succeeded:

@renatolabs
Copy link
Contributor

@msbutler should we backport this to 23.1?

@msbutler
Copy link
Collaborator Author

thanks for the reminder! over here #111086

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T-testeng TestEng Team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants