Skip to content

publish-artifacts: change GCS latest key prefix for Workload binary#155789

Merged
craig[bot] merged 1 commit intocockroachdb:masterfrom
williamchoe3:wchoe/publish-artifacts-change-gcs-key-for-workload-binary
Oct 22, 2025
Merged

publish-artifacts: change GCS latest key prefix for Workload binary#155789
craig[bot] merged 1 commit intocockroachdb:masterfrom
williamchoe3:wchoe/publish-artifacts-change-gcs-key-for-workload-binary

Conversation

@williamchoe3
Copy link
Copy Markdown
Contributor

@williamchoe3 williamchoe3 commented Oct 21, 2025

Informs #154274

In gcs gs://cockroach-edge-artifacts-prod, change latest Workload binary key prefix from cockroach to workload
i.e.

gs://cockroach-edge-artifacts-prod/cockroach/workload.release-24.1
to
gs://cockroach-edge-artifacts-prod/workload/workload.release-24.1

This will allow latest workload binary to not be deleted after 30 days so tests can download and use workload binaries from unsupported versions (that no longer have nightlies running against them)

Notes:

  • Will need to backfill currently unsupported versions
  • ".../workload/workload...." while descriptive also sounds redundant, open to suggestions if folks don't like that prefix
  • The non latest copy of the obj i.e. gs://cockroach-edge-artifacts-prod/cockroach/workload.ee80eab77ca6434f4be7c33c0f6d29dd0b9d07b1 is still prefixed with cockroach so these objects will get deleted after 30 days with the current lifecycle policy

Edited PutNonRelease's docstring to remove "redirect" terminology, looks to be from a previous s3 implementation which is not a feature of GCS.

  • Note: I don't feel that strongly about the docstring, it just confused me a bit when reading the code and just wanted it to match what the function is actually doing

Also noticed there's no unit test coverage for non master branches, added 1 release branch test with simple coverage

Also noticed there's a slight difference between mockGCSProvider.PutObject & release.PutObject

# obj latest key names on gs://cockroach-edge-artifacts-prod
gs://cockroach-edge-artifacts-prod/cockroach/workload.release-23.2

# obj latest key names in unit tests
gs://edge-binaries-bucket/workload/workload.release-25.4/no-cache

Actual release.PutObject does not modify the key name based on the PutObjectInput.CacheControl field, but in mockGCSProvider.PutObject the "/no-cache" gets appended
I changed the behavior in the mock to match the actual function, and modified the unit tests
FWIW PutObjectInput.CacheControl is only set to False currently

  • It looks like this might be a hold over from the S3 implementation? I didn't read that diff but let me know if I should revert this change

This PR is a dependency for these roachtest changes: #155397
Also discussed other options like using a new bucket or using the release bucket here: https://cockroachlabs.slack.com/archives/C03SG8QKYRJ/p1760977289072049?thread_ts=1760976858.868889&cid=C03SG8QKYRJ
Decided on this approach for simplicity

For verification, I'm relying on the unit tests. Just assumed this wouldn't work on my local, but if there's a simple way to verify this E2E let me know and i'll do it

Will squash and write a proper commit msg after review, leaving commit separate incase i need to revert anything

@cockroach-teamcity
Copy link
Copy Markdown
Member

This change is Reviewable

@williamchoe3 williamchoe3 changed the title publish-artifacts: change GCS latest key for Workload binary publish-artifacts: change GCS latest key prefix for Workload binary Oct 21, 2025
@williamchoe3 williamchoe3 marked this pull request as ready for review October 21, 2025 21:37
@williamchoe3 williamchoe3 requested a review from a team as a code owner October 21, 2025 21:37
Change previous latest key prefix from 'cockroach' to 'workload' for workload
binary to allow for separate lifecycle policies for workload binary objects.
Also cleaned up some comments and tests.

Informs cockroachdb#154274

Epic: None
Release note: None
@williamchoe3 williamchoe3 force-pushed the wchoe/publish-artifacts-change-gcs-key-for-workload-binary branch from d6adf42 to 6469695 Compare October 22, 2025 14:42
@williamchoe3
Copy link
Copy Markdown
Contributor Author

TFTR!

bors r=rail

@craig
Copy link
Copy Markdown
Contributor

craig bot commented Oct 22, 2025

@craig craig bot merged commit b59054b into cockroachdb:master Oct 22, 2025
41 of 44 checks passed
@williamchoe3
Copy link
Copy Markdown
Contributor Author

blathers backport all

@blathers-crl
Copy link
Copy Markdown

blathers-crl bot commented Oct 22, 2025

Encountered an error creating backports. Some common things that can go wrong:

  1. The backport branch might have already existed.
  2. There was a merge conflict.
  3. The backport branch contained merge commits.

You might need to create your backport manually using the backport tool.


unexpected status code: 404 Not Found

Backport to branch all failed. See errors above.


🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf.

craig bot pushed a commit that referenced this pull request Oct 23, 2025
155783: sql: several preliminary changes to making tests work with secondary tenants r=yuzefovich a=yuzefovich

This PR contains several commits which audit tests in `pkg/sql` directory to ensure the right JobRegistry, codec, LeaseManager, and PGUrl are used. Almost all were pretty mechanical changes. Note that no new tests are enabled with secondary tenants yet - this will be done separately.

Epic: CRDB-48945


156002: workload/tpch: add a couple of missed values for fixtures generation r=yuzefovich a=yuzefovich

When we added the fixtures generation of TPCH spec, we forgot a couple of values - one for "priorities" and one for "containers". This commit fixes that omission. In particular, missing priority value made us return 4 rows instead of 5 for Q4, and the expectation has been adjusted accordingly.

Note that this omission was exposed via a roachtest that uses correctly generated fixtures of SF 100.

Fixes: #155834.

Release note: None

156008: Revert "publish-artifacts: change GCS latest key prefix for Workload … r=rail a=williamchoe3

Original PR: #155789

Reverting to split into 2 commits for easier backport

Co-authored-by: Yahor Yuzefovich <yahor@cockroachlabs.com>
Co-authored-by: William Choe <williamchoe3@gmail.com>
craig bot pushed a commit that referenced this pull request Oct 23, 2025
156008: Revert "publish-artifacts: change GCS latest key prefix for Workload … r=rail a=williamchoe3

Original PR: #155789

Reverting to split into 2 commits for easier backport

Co-authored-by: William Choe <williamchoe3@gmail.com>
rishabh7m added a commit to crltest-issue-syncing/cockroach-fork that referenced this pull request Nov 20, 2025
rishabh7m added a commit to crltest-issue-syncing/cockroach-fork that referenced this pull request Nov 21, 2025
rishabh7m added a commit to crltest-issue-syncing/cockroach-fork that referenced this pull request Nov 21, 2025
rishabh7m added a commit to crltest-issue-syncing/cockroach-fork that referenced this pull request Dec 1, 2025
rishabh7m added a commit to crltest-issue-syncing/cockroach-fork that referenced this pull request Dec 10, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants