Skip to content

Commit

Permalink
Merge #85095 #85777
Browse files Browse the repository at this point in the history
85095: ui: reintroduce end-to-end UI tests with cypress r=laurenbarker,nathanstilwell,rickystewart,srosenberg a=sjbarag

A recent commit [1] removed the stale, unused end-to-end tests. In their
place, add newly-written tests that can run headlessly in TeamCity (via
Docker) as a quick-failing health check and as a more exhaustive suite.

[1] 3d171b2 (ui: remove unused end-to-end UI tests, 2022-07-25)

Release note: None

85777: clusterversion,kvserver,server,security: remove 22.1 gates r=celiala a=celiala

This commit removes the following 22.1 version gates:

_(✅ denotes explicit LGTM)_

**8/8: initial PR version gates** 
- ✅  kvserver: ProbeRequest, 
- ✅ security,pgwire: SCRAMAuthentication,
- ✅ server: UnsafeLossOfQuorumRecoveryRangeLog,
- ✅ kvserver: AlterSystemProtectedTimestampAddColumn

**8/10 Update: Additionally, these version gates to added to this commit**

- ✅ backupccl: EnableProtectedTimestampsForTenant
- ✅ DeleteCommentsWithDroppedIndexes
- sql/catalog: RemoveIncompatibleDatabasePrivileges
- ✅ kvserver: AddRaftAppliedIndexTermMigration
- ✅ clusterversion: PostAddRaftAppliedIndexTermMigration
- kvserver: DontProposeWriteTimestampForLeaseTransfers
- ~✅ storage: EnablePebbleFormatVersionBlockProperties~ 8/12: TODO in future PR.
- ✅ sql: MVCCIndexBackfiller
- ✅ kvserver: LooselyCoupledRaftLogTruncation
- ✅ sql: EnableDeclarativeSchemaChanger
- ~✅ sql: RowLevelTTL~ 8/12: TODO in future PR.
- kvserver: EnableNewStoreRebalancer
- ~✅ sql: SuperRegions~ 8/12: TODO in future PR.
- ~changefeedccl: EnableNewChangefeedOptions~ 8/12: TODO in future PR.
- ✅ SpanCountTable
- ✅ spanconfig: PreSeedSpanCountTable, SeedSpanCountTable

The cleanup for these version gates was fairly straight-forward, using the guidance from [21.2 cleanup](#74270 (comment)):

> For the most part, if the gates were just simple if !version.IsActive { return x } or something, I just removed the block, and even if it was a little more complicated, like args = [x]; if version { args = append(args, y) }; foo(args) I still tried to mostly inline it such that it looked natural (i.e. remove that append and make it args = [x, y]).

22.1 version gates requiring more nuanced removal will be done in separate PRs.

Partially addresses #80663


Co-authored-by: Sean Barag <barag@cockroachlabs.com>
Co-authored-by: Celia La <celia@cockroachlabs.com>
  • Loading branch information
3 people committed Aug 12, 2022
3 parents ff3fc7e + c58279d + 0da16b7 commit f1c2047
Show file tree
Hide file tree
Showing 62 changed files with 3,204 additions and 569 deletions.
16 changes: 16 additions & 0 deletions WORKSPACE
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ workspace(
"@npm_protos": ["pkg/ui/workspaces/db-console/src/js/node_modules"],
"@npm_cluster_ui": ["pkg/ui/workspaces/cluster_ui/node_modules"],
"@npm_db_console": ["pkg/ui/workspaces/db-console/node_modules"],
"@npm_e2e_tests": ["pkg/ui/workspaces/e2e-tests/node_modules"],
},
)

Expand Down Expand Up @@ -263,6 +264,21 @@ yarn_install(
symlink_node_modules = True,
)

yarn_install(
name = "npm_e2e_tests",
args = [
"--offline",
],
data = [
"//pkg/ui:.yarnrc",
"@yarn_cache//:.seed",
],
package_json = "//pkg/ui/workspaces/e2e-tests:package.json",
strict_visibility = False,
yarn_lock = "//pkg/ui/workspaces/e2e-tests:yarn.lock",
symlink_node_modules = True,
)

yarn_install(
name = "npm_protos",
args = [
Expand Down
2 changes: 1 addition & 1 deletion build/deploy/cockroach.sh
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
# set -m: enable job control.
set -eum

cockroach_entrypoint="/cockroach/cockroach"
cockroach_entrypoint=${COCKROACH:-"/cockroach/cockroach"}

certs_dir="certs"
url=
Expand Down
19 changes: 19 additions & 0 deletions build/teamcity/cockroach/ci/tests/ui_e2e_test_all.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
#!/usr/bin/env bash
set -euo pipefail

build="$(dirname $(dirname $(dirname $(dirname $(dirname "${0}")))))"
# for tc_prepare, tc_start_block, and friends
source "$build/teamcity-support.sh"
# for build_docker_image and run_tests
source "$build/teamcity/cockroach/ci/tests/ui_e2e_test_impl.sh"

tc_prepare

tc_start_block "Load cockroachdb/cockroach-ci image"
load_cockroach_docker_image
tc_end_block "Load cockroachdb/cockroach-ci image"

tc_start_block "Run Cypress health checks"
cd $root/pkg/ui/workspaces/e2e-tests
run_tests health
tc_end_block "Run Cypress health checks"
19 changes: 19 additions & 0 deletions build/teamcity/cockroach/ci/tests/ui_e2e_test_health.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
#!/usr/bin/env bash
set -euo pipefail

build="$(dirname $(dirname $(dirname $(dirname $(dirname "${0}")))))"
# for tc_prepare, tc_start_block, and friends
source "$build/teamcity-support.sh"
# for build_docker_image and run_tests
source "$build/teamcity/cockroach/ci/tests/ui_e2e_test_impl.sh"

tc_prepare

tc_start_block "Load cockroachdb/cockroach-ci image"
load_cockroach_docker_image
tc_end_block "Load cockroachdb/cockroach-ci image"

tc_start_block "Run Cypress health checks"
cd $root/pkg/ui/workspaces/e2e-tests
run_tests health
tc_end_block "Run Cypress health checks"
14 changes: 14 additions & 0 deletions build/teamcity/cockroach/ci/tests/ui_e2e_test_impl.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
#!/usr/bin/env bash
function load_cockroach_docker_image() {
docker load --input upstream_artifacts/cockroach-docker-image.tar.gz &> artifacts/docker-load.log || (cat artifacts/docker-load.log && false)
rm artifacts/docker-load.log
}

function run_tests() {
SPEC_ARG=""
if [ "health" = "${1:-'EMPTY'}" ]; then
SPEC_ARG="--spec 'cypress/e2e/health-check/**'"
fi

run docker compose run cypress -- $SPEC_ARG
}
2 changes: 1 addition & 1 deletion dev
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ fi
set -euo pipefail

# Bump this counter to force rebuilding `dev` on all machines.
DEV_VERSION=52
DEV_VERSION=53

THIS_DIR=$(cd "$(dirname "$0")" && pwd)
BINARY_DIR=$THIS_DIR/bin/dev-versions
Expand Down
12 changes: 2 additions & 10 deletions pkg/ccl/backupccl/backup_job.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ import (
"github.com/cockroachdb/cockroach/pkg/ccl/utilccl"
"github.com/cockroachdb/cockroach/pkg/cloud"
"github.com/cockroachdb/cockroach/pkg/cloud/cloudpb"
"github.com/cockroachdb/cockroach/pkg/clusterversion"
"github.com/cockroachdb/cockroach/pkg/gossip"
"github.com/cockroachdb/cockroach/pkg/jobs"
"github.com/cockroachdb/cockroach/pkg/jobs/joberror"
Expand Down Expand Up @@ -491,15 +490,8 @@ func (b *backupResumer) Resume(ctx context.Context, execCtx interface{}) error {
// UUID so that it can be released on job completion. The updated details
// are persisted in the job record further down.
{
if p.ExecCfg().Settings.Version.IsActive(ctx, clusterversion.EnableProtectedTimestampsForTenant) {
protectedtsID := uuid.MakeV4()
details.ProtectedTimestampRecord = &protectedtsID
} else if len(backupManifest.Spans) > 0 && p.ExecCfg().Codec.ForSystemTenant() {
// Prior to clusterversion.EnableProtectedTimestampsForTenant only the
// system tenant can write a protected timestamp record.
protectedtsID := uuid.MakeV4()
details.ProtectedTimestampRecord = &protectedtsID
}
protectedtsID := uuid.MakeV4()
details.ProtectedTimestampRecord = &protectedtsID

if details.ProtectedTimestampRecord != nil {
if err := p.ExecCfg().DB.Txn(ctx, func(ctx context.Context, txn *kv.Txn) error {
Expand Down
113 changes: 0 additions & 113 deletions pkg/clusterversion/cockroach_versions.go
Original file line number Diff line number Diff line change
Expand Up @@ -180,35 +180,6 @@ const (
// EnableSpanConfigStore enables the use of the span configs infrastructure
// in KV.
EnableSpanConfigStore
// SCRAM authentication is available.
SCRAMAuthentication
// UnsafeLossOfQuorumRecoveryRangeLog adds a new value to RangeLogEventReason
// that correspond to range descriptor changes resulting from recovery
// procedures.
UnsafeLossOfQuorumRecoveryRangeLog
// AlterSystemProtectedTimestampAddColumn adds a target column to the
// system.protected_ts_records table that describes what is protected by the
// record.
AlterSystemProtectedTimestampAddColumn
// EnableProtectedTimestampsForTenant enables the use of protected timestamps
// in secondary tenants.
EnableProtectedTimestampsForTenant
// DeleteCommentsWithDroppedIndexes cleans up left over comments that belong
// to dropped indexes.
DeleteCommentsWithDroppedIndexes
// RemoveIncompatibleDatabasePrivileges adds the migration which guarantees that
// databases do not have incompatible privileges
RemoveIncompatibleDatabasePrivileges
// AddRaftAppliedIndexTermMigration is a migration that causes each range
// replica to start populating RangeAppliedState.RaftAppliedIndexTerm field.
AddRaftAppliedIndexTermMigration
// PostAddRaftAppliedIndexTermMigration is used for asserting that
// RaftAppliedIndexTerm is populated.
PostAddRaftAppliedIndexTermMigration
// DontProposeWriteTimestampForLeaseTransfers stops setting the WriteTimestamp
// on lease transfer Raft proposals. New leaseholders now forward their clock
// directly to the new lease start time.
DontProposeWriteTimestampForLeaseTransfers
// EnablePebbleFormatVersionBlockProperties enables a new Pebble SSTable
// format version for block property collectors.
// NB: this cluster version is paired with PebbleFormatBlockPropertyCollector
Expand All @@ -219,24 +190,12 @@ const (
// engine running at the required format major version, as do all other nodes
// in the cluster.
EnablePebbleFormatVersionBlockProperties
// MVCCIndexBackfiller supports MVCC-compliant index
// backfillers via a new BACKFILLING index state, delete
// preserving temporary indexes, and a post-backfill merging
// processing.
MVCCIndexBackfiller
// EnableLeaseHolderRemoval enables removing a leaseholder and transferring the lease
// during joint configuration, including to VOTER_INCOMING replicas.
EnableLeaseHolderRemoval
// LooselyCoupledRaftLogTruncation allows the cluster to reduce the coupling
// for raft log truncation, by allowing each replica to treat a truncation
// proposal as an upper bound on what should be truncated.
LooselyCoupledRaftLogTruncation
// ChangefeedIdleness is the version where changefeed aggregators forward
// idleness-related information alnog with resolved spans to the frontier
ChangefeedIdleness
// EnableDeclarativeSchemaChanger is the version where new declarative schema changer
// can be used to construct schema change plan node.
EnableDeclarativeSchemaChanger
// RowLevelTTL is the version where we allow row level TTL tables.
RowLevelTTL
// EnableNewStoreRebalancer enables the new store rebalancer introduced in
Expand All @@ -254,15 +213,6 @@ const (
// such as end_time, initial_scan_only, and setting the value of initial_scan
// to 'yes|no|only'
EnableNewChangefeedOptions
// SpanCountTable adds system.span_count to track the number of committed
// tenant spans.
SpanCountTable
// PreSeedSpanCountTable precedes PreSeedSpanCountTable, it enables span
// accounting for incremental schema changes.
PreSeedSpanCountTable
// SeedSpanCountTable seeds system.span_count with the number of committed
// tenant spans.
SeedSpanCountTable

// V22_1 is CockroachDB v22.1. It's used for all v22.1.x patch releases.
V22_1
Expand Down Expand Up @@ -414,69 +364,18 @@ var versionsSingleton = keyedVersions{
Key: EnableSpanConfigStore,
Version: roachpb.Version{Major: 21, Minor: 2, Internal: 40},
},
{
Key: SCRAMAuthentication,
Version: roachpb.Version{Major: 21, Minor: 2, Internal: 44},
},
{
Key: UnsafeLossOfQuorumRecoveryRangeLog,
Version: roachpb.Version{Major: 21, Minor: 2, Internal: 46},
},
{
Key: AlterSystemProtectedTimestampAddColumn,
Version: roachpb.Version{Major: 21, Minor: 2, Internal: 48},
},
{
Key: EnableProtectedTimestampsForTenant,
Version: roachpb.Version{Major: 21, Minor: 2, Internal: 50},
},
{
Key: DeleteCommentsWithDroppedIndexes,
Version: roachpb.Version{Major: 21, Minor: 2, Internal: 52},
},
{
Key: RemoveIncompatibleDatabasePrivileges,
Version: roachpb.Version{Major: 21, Minor: 2, Internal: 54},
},
{
Key: AddRaftAppliedIndexTermMigration,
Version: roachpb.Version{Major: 21, Minor: 2, Internal: 56},
},
{
Key: PostAddRaftAppliedIndexTermMigration,
Version: roachpb.Version{Major: 21, Minor: 2, Internal: 58},
},
{
Key: DontProposeWriteTimestampForLeaseTransfers,
Version: roachpb.Version{Major: 21, Minor: 2, Internal: 60},
},
{
Key: EnablePebbleFormatVersionBlockProperties,
Version: roachpb.Version{Major: 21, Minor: 2, Internal: 64},
},
{
Key: MVCCIndexBackfiller,
Version: roachpb.Version{Major: 21, Minor: 2, Internal: 68},
},
{
Key: EnableLeaseHolderRemoval,
Version: roachpb.Version{Major: 21, Minor: 2, Internal: 70},
},
// Internal: 72 was reverted (EnsurePebbleFormatVersionRangeKeys)
// Internal: 74 was reverted (EnablePebbleFormatVersionRangeKeys)
// Internal: 78 was reverted (ExperimentalMVCCRangeTombstones)
{
Key: LooselyCoupledRaftLogTruncation,
Version: roachpb.Version{Major: 21, Minor: 2, Internal: 80},
},
{
Key: ChangefeedIdleness,
Version: roachpb.Version{Major: 21, Minor: 2, Internal: 82},
},
{
Key: EnableDeclarativeSchemaChanger,
Version: roachpb.Version{Major: 21, Minor: 2, Internal: 86},
},
{
Key: RowLevelTTL,
Version: roachpb.Version{Major: 21, Minor: 2, Internal: 88},
Expand All @@ -501,18 +400,6 @@ var versionsSingleton = keyedVersions{
Key: EnableNewChangefeedOptions,
Version: roachpb.Version{Major: 21, Minor: 2, Internal: 106},
},
{
Key: SpanCountTable,
Version: roachpb.Version{Major: 21, Minor: 2, Internal: 108},
},
{
Key: PreSeedSpanCountTable,
Version: roachpb.Version{Major: 21, Minor: 2, Internal: 110},
},
{
Key: SeedSpanCountTable,
Version: roachpb.Version{Major: 21, Minor: 2, Internal: 112},
},
{
Key: V22_1,
Version: roachpb.Version{Major: 22, Minor: 1},
Expand Down
87 changes: 36 additions & 51 deletions pkg/clusterversion/key_string.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading

0 comments on commit f1c2047

Please sign in to comment.