Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
87128: lint: Add commit/PR body linter for epic and issue refs r=nickvigilante a=nickvigilante

Before: Commits and PRs would inconsistently contain references to issues.
Sometimes the author would forget to add it. Some authors might not
think to add them. And there were no epic references provided.

Why: Adding issue and epic references to PRs and commits provides
traceability and context for the revenue teams, product, management and
engineers.

Now: A new GitHub Action workflow checks for references to issues the PR
closes or informs and epics the PR is part of. It looks for references
in the PR body and in commit messages and, if it doesn't find what it
expects, it fails the check.

This PR is a continuation of #77654.

Fixes #77376

Release note: None

Release justification: Adding linter for better epic tracking

89332: roachtest: ignore duplicated events in fingerprint validator r=srosenberg a=renatolabs

This commit updates the fingerprint validator (and its use in the
`cdc/mixed-versions` test) to ignore duplicated events received by the
validator.

A previously implicit assumption of the validator is that any events
that it receives are either not duplicated, or -- if they are
duplicated -- they are within the previous resolved timestamp and the
current resolved timestamp. However, that assumption is not justified
by the changefeed guarantees and depends on how frequently `resolved`
events are emitted and how often the changefeed checkpoints.

In the specific case of the `cdc/mixed-versions` roachtest, it was
possible for the changefeed to start from an old checkpoint (older
than the last received `resolved` timestamp), causing it to re-emit
old events that happened way before the previously observed resolved
event. As a consequence, when the validator applies the update
associated with that event, there is a mismatch with state of the
original table as of the update's timestamp, as the fingerprint
validator relies on the fact that updates are applied in order.

To fix the issue, we now skip events that happen before the timestamp
of the previous `resolved` event received. In addition, the caller can
also tell the validator to verify that such out-of-order messages
received by the validator have indeed been previously seen; if not,
that would represent a violation of the changefeed's guarantees.

Fixes: #87251.

Release note: None

89504: sql: fix panic in DROP ROLE when schemas have the same name r=ajwerner a=rafiss

fixes #89486

Release note (bug fix): Fix a crash that could occur when dropping a role that owned two schemas with the same name in different databases. The bug was introduced in v22.1.0.

89516: bazel,ci: check generated code and docs are up-to-date in CI r=rail a=rickystewart

When I created the file `build/bazelutil/checked_in_genfiles.txt` it was meant to contain an exhaustive list of all checked-in Go code, to be used by CI. In reality this file never had an exhaustive list and is not updated when new checked-in generated files are added. These days we have the `pkg/gen` infrastructure which *does* keep an exhaustive list of checked-in generated code, so the file is redundant anyway. Here I delete the file and just directly run `pkg/gen` for checking whether the generated code is up-to-date.

Closes #88744.

Release note: None

Co-authored-by: Nick Vigilante <[email protected]>
Co-authored-by: Renato Costa <[email protected]>
Co-authored-by: Rafi Shamim <[email protected]>
Co-authored-by: Ricky Stewart <[email protected]>
  • Loading branch information
5 people committed Oct 6, 2022
5 parents 516293f + a7ef46a + f350ab7 + 5ab9cf6 + 5536a1b commit bfbd07b
Show file tree
Hide file tree
Showing 22 changed files with 1,219 additions and 114 deletions.
1 change: 1 addition & 0 deletions .github/CODEOWNERS
Original file line number Diff line number Diff line change
Expand Up @@ -255,6 +255,7 @@
/pkg/cmd/gossipsim/ @cockroachdb/kv-prs
/pkg/cmd/import-tools/ @cockroachdb/dev-inf
/pkg/cmd/internal/issues/ @cockroachdb/test-eng
/pkg/cmd/lint-epic-issue-refs/ @cockroachdb/dev-inf
/pkg/cmd/mirror/ @cockroachdb/dev-inf
/pkg/cmd/prereqs/ @cockroachdb/dev-inf
/pkg/cmd/protoc-gen-gogoroach/ @cockroachdb/dev-inf
Expand Down
35 changes: 35 additions & 0 deletions .github/workflows/pr-epic-issue-ref-lint.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
name: Epic + Issue Ref Linter for PR Body or Commit Messages

on:
pull_request:
types: [opened, reopened, synchronize, edited]

jobs:
linter:
runs-on: ubuntu-latest
steps:
- name: Checkout
uses: actions/checkout@v2

- name: Bazel cache
id: bazel-cache
uses: actions/cache@v2
with:
path: |
~/.cache/bazel
key: ${{ runner.os }}-bazel-cache

- name: Install bazelisk
run: |
curl -fsSL https://github.com/bazelbuild/bazelisk/releases/download/v1.10.1/bazelisk-linux-amd64 > /tmp/bazelisk
sha256sum -c - <<EOF
4cb534c52cdd47a6223d4596d530e7c9c785438ab3b0a49ff347e991c210b2cd /tmp/bazelisk
EOF
mkdir -p "${GITHUB_WORKSPACE}/bin/"
mv /tmp/bazelisk "${GITHUB_WORKSPACE}/bin/bazel"
chmod +x "${GITHUB_WORKSPACE}/bin/bazel"
- name: Run lint
run: build/github/pr-epic-issue-ref-lint.sh
env:
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
9 changes: 0 additions & 9 deletions build/bazelutil/checked_in_genfiles.txt

This file was deleted.

7 changes: 7 additions & 0 deletions build/github/pr-epic-issue-ref-lint.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
#!/usr/bin/env bash

set -euo pipefail

PR_NUMBER="$(echo "$GITHUB_REF" | awk -F '/' '{print $3}')"

GITHUB_TOKEN="${GITHUB_TOKEN}" bin/bazel run //pkg/cmd/lint-epic-issue-refs:lint-epic-issue-refs "$PR_NUMBER"
3 changes: 1 addition & 2 deletions build/teamcity/cockroach/ci/builds/build_impl.sh
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,8 @@ EXTRA_TARGETS=
if [ "$CONFIG" == "crosslinux" ]
then
DOC_TARGETS=$(grep '^//' docs/generated/bazel_targets.txt)
GO_TARGETS=$(grep -v '^#' build/bazelutil/checked_in_genfiles.txt | cut -d'|' -f1)
BINARY_TARGETS="@com_github_cockroachdb_go_test_teamcity//:go-test-teamcity //pkg/cmd/dev //pkg/cmd/workload"
EXTRA_TARGETS="$DOC_TARGETS $GO_TARGETS $BINARY_TARGETS"
EXTRA_TARGETS="$DOC_TARGETS $BINARY_TARGETS"
fi

# Extra targets to build on Unix only.
Expand Down
18 changes: 1 addition & 17 deletions build/teamcity/cockroach/ci/builds/build_linux_x86_64.sh
Original file line number Diff line number Diff line change
Expand Up @@ -54,23 +54,7 @@ do
FAILED=1
fi
done
# Ensure checked-in generated files are byte-for-byte identical with what's in
# the checkout.
for LINE in $(grep -v '^#' $root/build/bazelutil/checked_in_genfiles.txt)
do
target=$(echo $LINE | cut -d'|' -f1)
dir=$(echo $target | sed 's|//||g' | cut -d: -f1)
old_basename=$(echo $LINE | cut -d'|' -f2)
new_basename=$(echo $LINE | cut -d'|' -f3)
RESULT=$(diff $root/artifacts/bazel-bin/$dir/$old_basename $root/$dir/$new_basename)
if [[ ! $? -eq 0 ]]
then
echo "Generated file $dir/$new_basename does not match with checked-in version. Got diff:"
echo "$RESULT"
echo "Run './dev generate go'"
FAILED=1
fi
done

# docs/generated/redact_safe.md needs special handling.
REAL_REDACT_SAFE=$($root/build/bazelutil/generate_redact_safe.sh)
RESULT=$(diff <(echo "$REAL_REDACT_SAFE") $root/docs/generated/redact_safe.md)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,15 @@ if grep TODO DEPS.bzl; then
fi
check_workspace_clean 'dev_generate_bazel' "Run \`./dev generate bazel\` to automatically regenerate these."

# Run `bazel run //pkg/gen` and ensure nothing changes. This ensures
# generated documentation and checked-in go code are up to date.
bazel run //pkg/gen
check_workspace_clean 'dev_generate' "Run \`./dev generate\` to automatically regenerate these."
# Run go mod tidy and ensure nothing changes.
# NB: If files are missing from any packages then `go mod tidy` will
# fail. So we need to make sure that `.pb.go` sources are populated.
bazel run //pkg/gen:go_proto
# This is part of what //pkg/gen does, in addition to generating Go code and
# docs.
bazel run @go_sdk//:bin/go --ui_event_filters=-DEBUG,-info,-stdout,-stderr --noshow_progress mod tidy
check_workspace_clean 'go_mod_tidy' "Run \`go mod tidy\` to automatically regenerate these."

Expand Down
5 changes: 5 additions & 0 deletions pkg/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,7 @@ ALL_TESTS = [
"//pkg/cmd/github-pull-request-make:github-pull-request-make_test",
"//pkg/cmd/internal/issues:issues_test",
"//pkg/cmd/label-merged-pr:label-merged-pr_test",
"//pkg/cmd/lint-epic-issue-refs:lint-epic-issue-refs_test",
"//pkg/cmd/mirror:mirror_test",
"//pkg/cmd/prereqs:prereqs_test",
"//pkg/cmd/publish-artifacts:publish-artifacts_test",
Expand Down Expand Up @@ -928,6 +929,9 @@ GO_TARGETS = [
"//pkg/cmd/label-merged-pr:label-merged-pr",
"//pkg/cmd/label-merged-pr:label-merged-pr_lib",
"//pkg/cmd/label-merged-pr:label-merged-pr_test",
"//pkg/cmd/lint-epic-issue-refs:lint-epic-issue-refs",
"//pkg/cmd/lint-epic-issue-refs:lint-epic-issue-refs_lib",
"//pkg/cmd/lint-epic-issue-refs:lint-epic-issue-refs_test",
"//pkg/cmd/mirror:mirror",
"//pkg/cmd/mirror:mirror_lib",
"//pkg/cmd/mirror:mirror_test",
Expand Down Expand Up @@ -2324,6 +2328,7 @@ GET_X_DATA_TARGETS = [
"//pkg/cmd/import-tools:get_x_data",
"//pkg/cmd/internal/issues:get_x_data",
"//pkg/cmd/label-merged-pr:get_x_data",
"//pkg/cmd/lint-epic-issue-refs:get_x_data",
"//pkg/cmd/mirror:get_x_data",
"//pkg/cmd/prereqs:get_x_data",
"//pkg/cmd/protoc-gen-gogoroach:get_x_data",
Expand Down
1 change: 0 additions & 1 deletion pkg/ccl/changefeedccl/cdctest/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ go_library(
"//pkg/util/hlc",
"//pkg/util/log",
"//pkg/util/randutil",
"//pkg/util/retry",
"//pkg/util/syncutil",
"//pkg/util/timeutil",
"@com_github_cockroachdb_errors//:errors",
Expand Down
2 changes: 1 addition & 1 deletion pkg/ccl/changefeedccl/cdctest/nemeses.go
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,7 @@ func RunNemesis(f TestFeedFactory, db *gosql.DB, isSinkless bool) (Validator, er
if err != nil {
return nil, err
}
fprintV, err := NewFingerprintValidator(db, `foo`, scratchTableName, foo.Partitions(), ns.maxTestColumnCount, false)
fprintV, err := NewFingerprintValidator(db, `foo`, scratchTableName, foo.Partitions(), ns.maxTestColumnCount)
if err != nil {
return nil, err
}
Expand Down
Loading

0 comments on commit bfbd07b

Please sign in to comment.