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

release: check remote tag before pushing #65555

Merged
merged 1 commit into from
May 27, 2021
Merged

Conversation

rail
Copy link
Member

@rail rail commented May 21, 2021

Previously, prior to pushing the release git tag, we created and pushed
most of the release artifacts (docker images, s3). If we run the script
multiple times against the same version, it may rewrite the artifacts
and will fail pushing the exiting git tag, leaving the release process
in a partially released state.

This patch adds a check if the release git tag already pushed to the
GitHub repo and fails before any artifacts are created.

Fixes #54814

Release note: None

@rail rail requested a review from jlinder May 21, 2021 12:29
@rail rail self-assigned this May 21, 2021
@cockroach-teamcity
Copy link
Member

This change is Reviewable

Copy link
Collaborator

@jlinder jlinder left a comment

Choose a reason for hiding this comment

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

Reviewed 2 of 2 files at r1.
Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (waiting on @rail)


build/teamcity-common-support.sh, line 35 at r1 (raw file):

git_wrapped() {
  GIT_SSH_COMMAND="ssh -i .cockroach-teamcity-key" git "$@"
}

Why not just make push_to_git more generic?


build/release/teamcity-publish-release.sh, line 62 at r1 (raw file):

tc_start_block "Check remote tag"
if git_wrapped ls-remote --exit-code --tags . "${build_name}"; then

Does the . need to be ssh://[email protected]/cockroachdb/cockroach.git so it checks the remote repo and not the local repo?

Copy link
Member Author

@rail rail left a comment

Choose a reason for hiding this comment

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

Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (waiting on @jlinder)


build/teamcity-common-support.sh, line 35 at r1 (raw file):

Previously, jlinder (James H. Linder) wrote…
git_wrapped() {
  GIT_SSH_COMMAND="ssh -i .cockroach-teamcity-key" git "$@"
}

Why not just make push_to_git more generic?

I thought about that, but didn't want to touch multiple invocations. I can easily refactor it for sure.


build/release/teamcity-publish-release.sh, line 62 at r1 (raw file):

Previously, jlinder (James H. Linder) wrote…

Does the . need to be ssh://[email protected]/cockroachdb/cockroach.git so it checks the remote repo and not the local repo?

. works fine, I double checked it by creating a new tag locally and running it. It exited with code 2, what is expected. I can be more explicit and set the remote.

Previously, prior to pushing the release git tag, we created and pushed
most of the release artifacts (docker images, s3). If we run the script
multiple times against the same version, it may rewrite the artifacts
and will fail pushing the exiting git tag, leaving the release process
in a partially released state.

This patch adds a check if the release git tag already pushed to the
GitHub repo and fails before any artifacts are created. It also
refactors the `push_to_git` function to be more generic.

Fixes cockroachdb#54814

Release note: None
@rail rail requested a review from jlinder May 27, 2021 02:23
Copy link
Member Author

@rail rail left a comment

Choose a reason for hiding this comment

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

Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (waiting on @jlinder)


build/teamcity-common-support.sh, line 35 at r1 (raw file):

Previously, rail (Rail Aliiev) wrote…

I thought about that, but didn't want to touch multiple invocations. I can easily refactor it for sure.

Done.


build/release/teamcity-publish-release.sh, line 62 at r1 (raw file):

Previously, rail (Rail Aliiev) wrote…

. works fine, I double checked it by creating a new tag locally and running it. It exited with code 2, what is expected. I can be more explicit and set the remote.

Done.

Copy link
Collaborator

@jlinder jlinder left a comment

Choose a reason for hiding this comment

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

Reviewed 4 of 4 files at r2.
Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (waiting on @rail)


build/teamcity-common-support.sh, line 35 at r1 (raw file):

Previously, rail (Rail Aliiev) wrote…

Done.

Cool. I thought that may have been the reason (which I understand).


build/release/teamcity-publish-release.sh, line 62 at r1 (raw file):

Previously, rail (Rail Aliiev) wrote…

Done.

Hrm. When I tested I got these results:

$ git tag -d v21.1.1
Deleted tag 'v21.1.1' (was f6630b3f52)
$ git ls-remote --exit-code --tags . v21.1.1
$ echo $?
2
$ git ls-remote --exit-code --tags ssh://[email protected]/cockroachdb/cockroach.git v21.1.1
f6630b3f524f3a5e52b330220037af01369b1ff2	refs/tags/v21.1.1
$ echo $?
0

Is that what you were expecting?

I also remembered that, since the origin repo on this checkout is from cockroachlabs/release-staging, the origin remote won't have the tag in it when the tag is in cockroachdb/cockroach , so the remote needs to be explicitly stated since git wouldn't know about it otherwise.

Copy link
Member Author

@rail rail left a comment

Choose a reason for hiding this comment

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

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


build/release/teamcity-publish-release.sh, line 62 at r1 (raw file):

Previously, jlinder (James H. Linder) wrote…

Hrm. When I tested I got these results:

$ git tag -d v21.1.1
Deleted tag 'v21.1.1' (was f6630b3f52)
$ git ls-remote --exit-code --tags . v21.1.1
$ echo $?
2
$ git ls-remote --exit-code --tags ssh://[email protected]/cockroachdb/cockroach.git v21.1.1
f6630b3f524f3a5e52b330220037af01369b1ff2	refs/tags/v21.1.1
$ echo $?
0

Is that what you were expecting?

I also remembered that, since the origin repo on this checkout is from cockroachlabs/release-staging, the origin remote won't have the tag in it when the tag is in cockroachdb/cockroach , so the remote needs to be explicitly stated since git wouldn't know about it otherwise.

Hmmm, now I don't now how I tested :D

@rail
Copy link
Member Author

rail commented May 27, 2021

bors r+

@craig
Copy link
Contributor

craig bot commented May 27, 2021

Build succeeded:

@craig craig bot merged commit 1c754e1 into cockroachdb:master May 27, 2021
rail added a commit to rail/cockroach that referenced this pull request Jun 1, 2021
In cockroachdb#65555 we added check to make sure we don't try to push the same tag
twice. The `git ls-remote` command, which uses `ssh://` was missing a
step that configures the SSH key.

This patch configures SSH earlier in the process.

Release note: None
craig bot pushed a commit that referenced this pull request Jun 1, 2021
…65952 #65953 #65958

65397: cloud: add new stanza to specify custom certs for Prometheus r=taroface a=taroface

Add a stanza to the Prometheus manifest that allows the user to specify custom certs. The `cockroach-node` specifies the default node secret used by the K8s Operator, as advised by @chrisseto.

Relates to cockroachdb/cockroach-operator#469.

A doc update to the Prometheus tutorial will refer to this change.

65871: sql: fix a bug which prevented self referencing NOT VALID foreign keys r=fqazi a=ajwerner

This bug is due to both using the same object in descs.Txn (sort of) and the
fact the fact that MakeMutationComplete does not remove the mutation any
longer. I have a feeling, but have not checked, that we lost this back-
reference in older versions.

Release note (bug fix): Fixed a bug which prevented adding self-referencing
FOREIGN KEY constraints in the NOT VALID state.

65938: sql: fix bug in column backfill with virtual NOT NULL columns r=mgartner a=ajwerner

Prior to this change we'd inform the column backfiller that it needed to read
the virtual columns. These virtual columns don't exist and thus won't be read.
If the columns are marked as NOT NULL, then an assertion will fire from inside
the row fetcher. This PR fixes the bug by not requesting the virtual columns.

Fixes #65915.

Release note (bug fix): Fixed a bug which prevented adding columns to tables
which contain data and use NOT NULL virtual columns

65943: release: configure ssh key before using git r=rail a=rail

In #65555 we added check to make sure we don't try to push the same tag
twice. The `git ls-remote` command, which uses `ssh://` was missing a
step that configures the SSH key.

This patch configures SSH earlier in the process.

Release note: None

65945: authors: add JeffSwenson to authors r=JeffSwenson a=JeffSwenson

Release note: None

65946: authors: add adwittumuluri to authors r=adwittumuluri a=adwittumuluri

Release note: None

65947: authors: add todd to authors r=matthewtodd a=matthewtodd

Release note: None

65949: authors: add sarkesian to authors r=AlexTalks a=AlexTalks

Release note: None

65950: Adding Toshi to Authors r=noguchitoshi a=noguchitoshi

[title]

65952: authors: add "Duoc Nguyen" to authors r=duoclikebook a=duoclikebook

Release note: None

65953: add nancy.vargas to authors r=nancy-vargas a=nancy-vargas



65958: authors: add <livlobo> to authors r=livlobo a=livlobo

Release note: None

Co-authored-by: taroface <[email protected]>
Co-authored-by: Andrew Werner <[email protected]>
Co-authored-by: Rail Aliiev <[email protected]>
Co-authored-by: Jeff Swenson <[email protected]>
Co-authored-by: Adwit Tumuluri <[email protected]>
Co-authored-by: Matthew Todd <[email protected]>
Co-authored-by: Alex Sarkesian <[email protected]>
Co-authored-by: Toshi Noguchi <[email protected]>
Co-authored-by: Duoc Nguyen <[email protected]>
Co-authored-by: Nancy Vargas Balderas <[email protected]>
Co-authored-by: Liv Lobo <[email protected]>
rail added a commit to rail/cockroach that referenced this pull request Jun 2, 2021
In cockroachdb#65555 we added check to make sure we don't try to push the same tag
twice. The `git ls-remote` command, which uses `ssh://` was missing a
step that configures the SSH key.

This patch configures SSH earlier in the process.

Release note: None
rail added a commit to rail/cockroach that referenced this pull request Jun 3, 2021
In cockroachdb#65555 we added check to make sure we don't try to push the same tag
twice. The `git ls-remote` command, which uses `ssh://` was missing a
step that configures the SSH key.

This patch configures SSH earlier in the process.

Release note: None
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants