build,release: provide way to inject build tag override, use in MAPB #101943

rickystewart · 2023-04-20T19:11:42Z

When we build nightly builds, we build them in "release" configuration. Because we do this, the build tag reported by these binaries from cockroach version has been identical to that of an actual release binary, which is very confusing. Here we update the script to build these nightlies (make-and-publish-build-artifacts.sh) to inject an appropriate, identifiable build tag.

It is probably wrong that we are building these nightlies as if they were "releases". We'll follow up with work to fix this and refine the build tags further.

Closes #100532.
Epic: None
Release note (build change): Update reported Build Tag for nightly (non-release) builds

cockroach-teamcity · 2023-04-20T19:11:51Z

This change is

jlinder

A couple questions:

Can we also check that Publish Bleeding Edge has a good value for Build Tag and, if not, update those builds too?

jlinder · 2023-04-20T19:29:16Z

build/teamcity/internal/release/process/make-and-publish-build-artifacts.sh

@@ -52,14 +52,14 @@ git tag "${build_name}"
 tc_end_block "Tag the release"

 tc_start_block "Compile and publish artifacts"
-BAZEL_SUPPORT_EXTRA_DOCKER_ARGS="-e TC_BUILDTYPE_ID -e TC_BUILD_BRANCH=$build_name -e gcs_credentials -e gcs_bucket=$gcs_bucket" run_bazel << 'EOF'
+BAZEL_SUPPORT_EXTRA_DOCKER_ARGS="-e TC_BUILDTYPE_ID -e TC_BUILD_BRANCH=$build_name -e build_name=$build_name -e gcs_credentials -e gcs_bucket=$gcs_bucket" run_bazel << 'EOF'


I'm curious. Why make build_name lowercase here?

Also, why not just use TC_BUILD_BRANCH given they both have the same value?

I'm curious. Why make build_name lowercase here?

I didn't "make it lowercase" -- the variable already exists and has a lowercase name. 🤷 Just keeping it the same way it is.

Also, why not just use TC_BUILD_BRANCH given they both have the same value?

Out of an abundance of caution I am not doing this. I am not sure if the value of TC_BUILD_BRANCH from the host or the container would be used because bash scripts are confusing. (The values differ between the host and container for some reason.) In this case the values of build_name are the same on host and container so no matter which way place the string interpolation happens, it should be fine.

jlinder · 2023-04-20T19:34:33Z

pkg/cmd/publish-provisional-artifacts/main.go

 	var doProvisional bool
 	var isRelease bool
 	var doBless bool
 	flag.BoolVar(&isRelease, "release", false, "build in release mode instead of bleeding-edge mode")
 	flag.StringVar(&gcsBucket, "gcs-bucket", "", "GCS bucket")
 	flag.StringVar(&outputDirectory, "output-directory", "",
 		"Save local copies of uploaded release archives in this directory")
+	flag.StringVar(&tagOverride, "build-tag-override", "", "override the version from version.txt")


Why drop the build in tagOverride? Asking because the word tag by itself is overloaded and explicitly saying build tag makes it clearer where the value will end up going.

rail

The logic looks sane to me. There are some go fmt suggestions and James' comments that need to be addressed before you can merge.

Reviewed 7 of 7 files at r1, all commit messages.
Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @celiala and @rickystewart)

rickystewart · 2023-04-20T21:21:08Z

Can we also check that Publish Bleeding Edge has a good value for Build Tag and, if not, update those builds too?

Already done. These binaries are -dev- binaries that have sensible Build Tags already.

rickystewart · 2023-04-20T21:28:02Z

bors r=rail

rickystewart · 2023-04-20T21:33:19Z

bors r-

craig · 2023-04-20T21:33:21Z

Canceled.

When we build nightly builds, we build them in "release" configuration. Because we do this, the build tag reported by these binaries from `cockroach version` has been identical to that of an actual release binary, which is very confusing. Here we update the script to build these nightlies (`make-and-publish-build-artifacts.sh`) to inject an appropriate, identifiable build tag. It is probably wrong that we are building these nightlies as if they were "releases". We'll follow up with work to fix this and refine the build tags further. Closes cockroachdb#100532. Epic: None Release note (build change): Update reported `Build Tag` for nightly (non-release) builds

rickystewart · 2023-04-20T21:42:42Z

bors r=rail

craig · 2023-04-20T22:45:59Z

Build succeeded:

Bazel Essential CI (Cockroach)

blathers-crl · 2023-04-20T22:46:07Z

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

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

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

error creating merge commit from cbad692 to blathers/backport-release-23.1-101943: POST https://api.github.com/repos/cockroachdb/cockroach/merges: 409 Merge conflict []

you may need to manually resolve merge conflicts with the backport tool.

Backport to branch 23.1.x failed. See errors above.

error creating merge commit from cbad692 to blathers/backport-release-23.1.0-101943: POST https://api.github.com/repos/cockroachdb/cockroach/merges: 409 Merge conflict []

you may need to manually resolve merge conflicts with the backport tool.

Backport to branch 23.1.0 failed. See errors above.

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

rickystewart requested review from rail and celiala April 20, 2023 19:11

rickystewart requested a review from a team as a code owner April 20, 2023 19:11

jlinder reviewed Apr 20, 2023

View reviewed changes

rail approved these changes Apr 20, 2023

View reviewed changes

rickystewart force-pushed the nightlytag branch 2 times, most recently from e54988c to ecbb86b Compare April 20, 2023 21:26

rickystewart requested a review from jlinder April 20, 2023 21:27

rickystewart force-pushed the nightlytag branch from ecbb86b to cbad692 Compare April 20, 2023 21:34

rickystewart added backport-23.1.x Flags PRs that need to be backported to 23.1 backport-23.1.0 labels Apr 20, 2023

craig bot merged commit 5ab6451 into cockroachdb:master Apr 20, 2023

cockroach-teamcity mentioned this pull request Apr 21, 2023

PR #101943 - build,release: provide way to inject build tag override, use in MAPB cockroachdb/docs#16817

Closed

This was referenced Apr 21, 2023

release-23.1.0: build,release: provide way to inject build tag override, use in MAPB #101997

Merged

release-23.1: build,release: provide way to inject build tag override, use in MAPB #101998

Merged

This was referenced Apr 21, 2023

build: plumb release build type #100801

Closed

release: verify reported version info from release binaries #100745

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

build,release: provide way to inject build tag override, use in MAPB #101943

build,release: provide way to inject build tag override, use in MAPB #101943

rickystewart commented Apr 20, 2023

cockroach-teamcity commented Apr 20, 2023

jlinder left a comment

jlinder Apr 20, 2023

rickystewart Apr 20, 2023 •

edited

Loading

jlinder Apr 20, 2023

rickystewart Apr 20, 2023

rail left a comment

rickystewart commented Apr 20, 2023

rickystewart commented Apr 20, 2023

rickystewart commented Apr 20, 2023

craig bot commented Apr 20, 2023

rickystewart commented Apr 20, 2023

craig bot commented Apr 20, 2023

blathers-crl bot commented Apr 20, 2023

build,release: provide way to inject build tag override, use in MAPB #101943

build,release: provide way to inject build tag override, use in MAPB #101943

Conversation

rickystewart commented Apr 20, 2023

cockroach-teamcity commented Apr 20, 2023

jlinder left a comment

Choose a reason for hiding this comment

jlinder Apr 20, 2023

Choose a reason for hiding this comment

rickystewart Apr 20, 2023 • edited Loading

Choose a reason for hiding this comment

jlinder Apr 20, 2023

Choose a reason for hiding this comment

rickystewart Apr 20, 2023

Choose a reason for hiding this comment

rail left a comment

Choose a reason for hiding this comment

rickystewart commented Apr 20, 2023

rickystewart commented Apr 20, 2023

rickystewart commented Apr 20, 2023

craig bot commented Apr 20, 2023

rickystewart commented Apr 20, 2023

craig bot commented Apr 20, 2023

blathers-crl bot commented Apr 20, 2023

rickystewart Apr 20, 2023 •

edited

Loading