From 308b025eadac6fdb7f1b13d1b259f3c0eb63ca6e Mon Sep 17 00:00:00 2001 From: Miral Gadani Date: Tue, 3 Oct 2023 13:19:44 -0400 Subject: [PATCH 1/9] scripts: add azure nightly roachtest Epic: CC-25185 Release note: none --- .../cockroach/nightlies/roachtest_nightly_azure.sh | 11 +++++++++++ build/teamcity/util/roachtest_util.sh | 8 +++++++- 2 files changed, 18 insertions(+), 1 deletion(-) create mode 100755 build/teamcity/cockroach/nightlies/roachtest_nightly_azure.sh diff --git a/build/teamcity/cockroach/nightlies/roachtest_nightly_azure.sh b/build/teamcity/cockroach/nightlies/roachtest_nightly_azure.sh new file mode 100755 index 000000000000..8d5c5e844221 --- /dev/null +++ b/build/teamcity/cockroach/nightlies/roachtest_nightly_azure.sh @@ -0,0 +1,11 @@ +#!/usr/bin/env bash + +set -exuo pipefail + +dir="$(dirname $(dirname $(dirname $(dirname "${0}"))))" + +source "$dir/teamcity-support.sh" # For $root +source "$dir/teamcity-bazel-support.sh" # For run_bazel + +BAZEL_SUPPORT_EXTRA_DOCKER_ARGS="-e LITERAL_ARTIFACTS_DIR=$root/artifacts -e AZURE_CLIENT_ID -e AZURE_CLIENT_SECRET -e AZURE_SUBSCRIPTION_ID -e AZURE_TENANT_ID -e BUILD_VCS_NUMBER -e CLOUD -e COCKROACH_DEV_LICENSE -e TESTS -e COUNT -e GITHUB_API_TOKEN -e GITHUB_ORG -e GITHUB_REPO -e GOOGLE_EPHEMERAL_CREDENTIALS -e SLACK_TOKEN -e TC_BUILDTYPE_ID -e TC_BUILD_BRANCH -e TC_BUILD_ID -e TC_SERVER_URL -e SELECT_PROBABILITY" \ + run_bazel build/teamcity/cockroach/nightlies/roachtest_nightly_impl.sh diff --git a/build/teamcity/util/roachtest_util.sh b/build/teamcity/util/roachtest_util.sh index 1e36e9291e0d..ef29229ff230 100644 --- a/build/teamcity/util/roachtest_util.sh +++ b/build/teamcity/util/roachtest_util.sh @@ -70,13 +70,19 @@ case "${CLOUD}" in # - "aws" to ensure we select tests that now no longer have "default" because they have the "aws" tag # Ideally, refactor the tags themselves to be explicit about what cloud they are for and when they can run. # https://github.com/cockroachdb/cockroach/issues/100605 - FILTER="tag:aws tag:default" + FILTER="tag:aws tag:default tag:azure" ;; aws) if [ -z "${FILTER}" ]; then FILTER="tag:aws" fi ;; + azure) + if [ -z "${FILTER}" ]; then + # Soon to go away with Radu's tag changes. + FILTER="tag:azure" + fi + ;; *) echo "unknown cloud ${CLOUD}" exit 1 From de374f0b791a86c9944d3fb5edf096ab3d8f016c Mon Sep 17 00:00:00 2001 From: Miral Gadani Date: Tue, 3 Oct 2023 13:35:45 -0400 Subject: [PATCH 2/9] roachprod: azure authentication This PR ensures that Azure SDK is granted access in either 1 of 2 ways. 1. If the required environment variables are present, then, `NewAuthorizerFromEnvironment` is used. 2. Else, `az`, the CLI tool must be installed and authenticated and `NewAuthorizerFromCLI` is used. The former is used by TeamCity, the latter is likely to be used my developers. --- pkg/roachprod/vm/azure/auth.go | 21 ++++++++++++++++----- pkg/roachprod/vm/azure/azure.go | 16 ++++++++++++---- 2 files changed, 28 insertions(+), 9 deletions(-) diff --git a/pkg/roachprod/vm/azure/auth.go b/pkg/roachprod/vm/azure/auth.go index d0f988420e04..fdffd5efe96a 100644 --- a/pkg/roachprod/vm/azure/auth.go +++ b/pkg/roachprod/vm/azure/auth.go @@ -12,12 +12,18 @@ package azure import ( "net/http" + "os" "github.com/Azure/go-autorest/autorest" "github.com/Azure/go-autorest/autorest/azure/auth" "github.com/cockroachdb/errors" ) +// We look to the environment for sdk authentication first. +var hasEnvAuth = os.Getenv("AZURE_TENANT_ID") != "" && + os.Getenv("AZURE_CLIENT_ID") != "" && + os.Getenv("AZURE_CLIENT_SECRET") != "" + // getAuthorizer returns an Authorizer, which uses the Azure CLI // to log into the portal. // @@ -33,29 +39,34 @@ func (p *Provider) getAuthorizer() (ret autorest.Authorizer, err error) { return } - // Use the azure CLI to bootstrap our authentication. + // Use the environment or azure CLI to bootstrap our authentication. // https://docs.microsoft.com/en-us/go/azure/azure-sdk-go-authorization - ret, err = auth.NewAuthorizerFromCLI() + if hasEnvAuth { + ret, err = auth.NewAuthorizerFromEnvironment() + } else { + ret, err = auth.NewAuthorizerFromCLI() + } + if err == nil { p.mu.Lock() p.mu.authorizer = ret p.mu.Unlock() } else { - err = errors.Wrap(err, "could got get Azure auth token") + err = errors.Wrap(err, "could not get Azure auth token") } return } // getAuthToken extracts the JWT token from the active Authorizer. func (p *Provider) getAuthToken() (string, error) { - auth, err := p.getAuthorizer() + authorizer, err := p.getAuthorizer() if err != nil { return "", err } // We'll steal the auth Bearer token by creating a fake HTTP request. fake := &http.Request{} - if _, err := auth.WithAuthorization()(&stealAuth{}).Prepare(fake); err != nil { + if _, err := authorizer.WithAuthorization()(&stealAuth{}).Prepare(fake); err != nil { return "", err } return fake.Header.Get("Authorization")[7:], nil diff --git a/pkg/roachprod/vm/azure/azure.go b/pkg/roachprod/vm/azure/azure.go index 24387640d903..24a2668b0872 100644 --- a/pkg/roachprod/vm/azure/azure.go +++ b/pkg/roachprod/vm/azure/azure.go @@ -53,14 +53,19 @@ var providerInstance = &Provider{} func Init() error { const cliErr = "please install the Azure CLI utilities " + "(https://docs.microsoft.com/en-us/cli/azure/install-azure-cli)" - const authErr = "please use `az login` to login to Azure" + const authErr = "unable to authenticate; please use `az login` or double check environment variables" providerInstance = New() providerInstance.OperationTimeout = 10 * time.Minute providerInstance.SyncDelete = false - if _, err := exec.LookPath("az"); err != nil { - vm.Providers[ProviderName] = flagstub.New(&Provider{}, cliErr) - return err + + // If the appropriate environment variables are not set for api access, + // then the authenticated CLI must be installed. + if !hasEnvAuth { + if _, err := exec.LookPath("az"); err != nil { + vm.Providers[ProviderName] = flagstub.New(&Provider{}, cliErr) + return err + } } if _, err := providerInstance.getAuthToken(); err != nil { vm.Providers[ProviderName] = flagstub.New(&Provider{}, authErr) @@ -1050,6 +1055,9 @@ func (p *Provider) createVNets( } groupsClient := resources.NewGroupsClient(*sub.SubscriptionID) + if groupsClient.Authorizer, err = p.getAuthorizer(); err != nil { + return nil, err + } vnetResourceGroupTags := make(map[string]*string) vnetResourceGroupTags[tagComment] = to.StringPtr("DO NOT DELETE: Used by all roachprod clusters") From 94d8d26bc8083d9dc7d8b70fb51886b1241faebd Mon Sep 17 00:00:00 2001 From: Miral Gadani Date: Tue, 3 Oct 2023 13:45:31 -0400 Subject: [PATCH 3/9] roachprod: azure default subscription id Previously, the subscription id associated with all the SDK requests defaulted to using the first one when listing all of them. This does not allow us to query against a particular one. This PR introduces code to check for an environment variable of name `AZURE_SUBSCRIPTION_ID` which, if present is used. Otherwise, we fall back to selecting the first one. Additionally, we just memoize the simple string representation of the subscriptionId instead of the struct. Epic: CC-25185 Release note: none --- pkg/roachprod/vm/azure/azure.go | 94 ++++++++++++++++++--------------- 1 file changed, 51 insertions(+), 43 deletions(-) diff --git a/pkg/roachprod/vm/azure/azure.go b/pkg/roachprod/vm/azure/azure.go index 24a2668b0872..f3730862901d 100644 --- a/pkg/roachprod/vm/azure/azure.go +++ b/pkg/roachprod/vm/azure/azure.go @@ -87,7 +87,7 @@ type Provider struct { syncutil.Mutex authorizer autorest.Authorizer - subscription subscriptions.Subscription + subscriptionId string resourceGroups map[string]resources.Group subnets map[string]network.Subnet securityGroups map[string]network.SecurityGroup @@ -245,8 +245,7 @@ func (p *Provider) Create( location := providerOpts.Locations[locIdx] // Create a resource group within the location. - group, err := p.getOrCreateResourceGroup( - ctx, getClusterResourceGroupName(location), location, clusterTags) + group, err := p.getOrCreateResourceGroup(ctx, getClusterResourceGroupName(location), location, clusterTags) if err != nil { return err } @@ -287,7 +286,7 @@ func (p *Provider) Delete(l *logger.Logger, vms vm.List) error { if err != nil { return err } - client := compute.NewVirtualMachinesClient(*sub.ID) + client := compute.NewVirtualMachinesClient(sub) if client.Authorizer, err = p.getAuthorizer(); err != nil { return err } @@ -335,7 +334,7 @@ func (p *Provider) DeleteCluster(l *logger.Logger, name string) error { if err != nil { return err } - client := resources.NewGroupsClient(*sub.SubscriptionID) + client := resources.NewGroupsClient(sub) if client.Authorizer, err = p.getAuthorizer(); err != nil { return err } @@ -386,7 +385,7 @@ func (p *Provider) Extend(l *logger.Logger, vms vm.List, lifetime time.Duration) if err != nil { return err } - client := compute.NewVirtualMachinesClient(*sub.SubscriptionID) + client := compute.NewVirtualMachinesClient(sub) if client.Authorizer, err = p.getAuthorizer(); err != nil { return err } @@ -466,7 +465,7 @@ func (p *Provider) List(l *logger.Logger, opts vm.ListOptions) (vm.List, error) } // We're just going to list all VMs and filter. - client := compute.NewVirtualMachinesClient(*sub.SubscriptionID) + client := compute.NewVirtualMachinesClient(sub) if client.Authorizer, err = p.getAuthorizer(); err != nil { return nil, err } @@ -549,7 +548,7 @@ func (p *Provider) List(l *logger.Logger, opts vm.ListOptions) (vm.List, error) // Such a cluster won't be found by listing all azure VMs like above. // Normally we don't want to access these clusters except for deleting them. if opts.IncludeEmptyClusters { - groupsClient := resources.NewGroupsClient(*sub.SubscriptionID) + groupsClient := resources.NewGroupsClient(sub) if groupsClient.Authorizer, err = p.getAuthorizer(); err != nil { return nil, err } @@ -651,7 +650,7 @@ func (p *Provider) createVM( return } - client := compute.NewVirtualMachinesClient(*sub.SubscriptionID) + client := compute.NewVirtualMachinesClient(sub) if client.Authorizer, err = p.getAuthorizer(); err != nil { return } @@ -816,7 +815,7 @@ func (p *Provider) createNIC( if err != nil { return } - client := network.NewInterfacesClient(*sub.SubscriptionID) + client := network.NewInterfacesClient(sub) if client.Authorizer, err = p.getAuthorizer(); err != nil { return } @@ -872,7 +871,7 @@ func (p *Provider) getOrCreateNetworkSecurityGroup( if err != nil { return network.SecurityGroup{}, err } - client := network.NewSecurityGroupsClient(*sub.SubscriptionID) + client := network.NewSecurityGroupsClient(sub) if client.Authorizer, err = p.getAuthorizer(); err != nil { return network.SecurityGroup{}, err } @@ -1054,7 +1053,7 @@ func (p *Provider) createVNets( return nil, err } - groupsClient := resources.NewGroupsClient(*sub.SubscriptionID) + groupsClient := resources.NewGroupsClient(sub) if groupsClient.Authorizer, err = p.getAuthorizer(); err != nil { return nil, err } @@ -1178,7 +1177,7 @@ func (p *Provider) createVNet( if err != nil { return } - client := network.NewVirtualNetworksClient(*sub.SubscriptionID) + client := network.NewVirtualNetworksClient(sub) if client.Authorizer, err = p.getAuthorizer(); err != nil { return } @@ -1231,7 +1230,7 @@ func (p *Provider) createVNetPeerings( if err != nil { return err } - client := network.NewVirtualNetworkPeeringsClient(*sub.SubscriptionID) + client := network.NewVirtualNetworkPeeringsClient(sub) if client.Authorizer, err = p.getAuthorizer(); err != nil { return err } @@ -1296,7 +1295,7 @@ func (p *Provider) createIP( if err != nil { return } - ipc := network.NewPublicIPAddressesClient(*sub.SubscriptionID) + ipc := network.NewPublicIPAddressesClient(sub) if ipc.Authorizer, err = p.getAuthorizer(); err != nil { return } @@ -1340,12 +1339,12 @@ func (p *Provider) fillNetworkDetails(ctx context.Context, m *vm.VM, nicID azure return err } - nicClient := network.NewInterfacesClient(*sub.SubscriptionID) + nicClient := network.NewInterfacesClient(sub) if nicClient.Authorizer, err = p.getAuthorizer(); err != nil { return err } - ipClient := network.NewPublicIPAddressesClient(*sub.SubscriptionID) + ipClient := network.NewPublicIPAddressesClient(sub) ipClient.Authorizer = nicClient.Authorizer iface, err := nicClient.Get(ctx, nicID.resourceGroup, nicID.resourceName, "" /*expand*/) @@ -1411,7 +1410,7 @@ func (p *Provider) getOrCreateResourceGroup( return resources.Group{}, err } - client := resources.NewGroupsClient(*sub.SubscriptionID) + client := resources.NewGroupsClient(sub) if client.Authorizer, err = p.getAuthorizer(); err != nil { return resources.Group{}, err } @@ -1452,7 +1451,7 @@ func (p *Provider) createUltraDisk( return compute.Disk{}, err } - client := compute.NewDisksClient(*sub.SubscriptionID) + client := compute.NewDisksClient(sub) if client.Authorizer, err = p.getAuthorizer(); err != nil { return compute.Disk{}, err } @@ -1486,39 +1485,48 @@ func (p *Provider) createUltraDisk( return disk, err } -// getSubscription chooses the first available subscription. The value -// is memoized in the Provider instance. -func (p *Provider) getSubscription( - ctx context.Context, -) (sub subscriptions.Subscription, err error) { - sub = func() subscriptions.Subscription { +// getSubscription returns env.AZURE_SUBSCRIPTION_ID if it exists +// or the first subscription when listing all available via an API call. +// The value is memoized in the Provider instance. +func (p *Provider) getSubscription(ctx context.Context) (string, error) { + subscriptionId := func() string { p.mu.Lock() defer p.mu.Unlock() - return p.mu.subscription + return p.mu.subscriptionId }() - if sub.SubscriptionID != nil { - return + if subscriptionId != "" { + return subscriptionId, nil } - sc := subscriptions.NewClient() - if sc.Authorizer, err = p.getAuthorizer(); err != nil { - return - } + subscriptionId = os.Getenv("AZURE_SUBSCRIPTION_ID") - page, err := sc.List(ctx) - if err == nil { - if len(page.Values()) == 0 { - err = errors.New("did not find Azure subscription") - return sub, err + // Fallback to retrieving the first subscription + if subscriptionId == "" { + authorizer, err := p.getAuthorizer() + if err != nil { + return "", err + } + sc := subscriptions.NewClient() + sc.Authorizer = authorizer + + page, err := sc.List(ctx) + if err == nil { + if len(page.Values()) == 0 { + err = errors.New("did not find Azure subscription") + return "", err + } + s := page.Values()[0].SubscriptionID + if s != nil { + subscriptionId = *s + } } - sub = page.Values()[0] - - p.mu.Lock() - defer p.mu.Unlock() - p.mu.subscription = page.Values()[0] } - return + + p.mu.Lock() + defer p.mu.Unlock() + p.mu.subscriptionId = subscriptionId + return subscriptionId, nil } // getResourceGroupByName receives a string name and returns From fc1796983dcfa307d7c13be755762c28d59656ba Mon Sep 17 00:00:00 2001 From: Miral Gadani Date: Tue, 3 Oct 2023 13:38:08 -0400 Subject: [PATCH 4/9] roachprod: azure add kafka inbound security rule Epic: CC-25185 Release note: none --- pkg/roachprod/vm/azure/azure.go | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/pkg/roachprod/vm/azure/azure.go b/pkg/roachprod/vm/azure/azure.go index f3730862901d..e14fab26fd11 100644 --- a/pkg/roachprod/vm/azure/azure.go +++ b/pkg/roachprod/vm/azure/azure.go @@ -1019,6 +1019,19 @@ func (p *Provider) getOrCreateNetworkSecurityGroup( DestinationPortRange: to.StringPtr("9090"), }, }, + { + Name: to.StringPtr("Kafka_Inbound"), + SecurityRulePropertiesFormat: &network.SecurityRulePropertiesFormat{ + Priority: to.Int32Ptr(346), + Protocol: network.SecurityRuleProtocolTCP, + Access: network.SecurityRuleAccessAllow, + Direction: network.SecurityRuleDirectionInbound, + SourceAddressPrefix: to.StringPtr("*"), + SourcePortRange: to.StringPtr("*"), + DestinationAddressPrefix: to.StringPtr("*"), + DestinationPortRange: to.StringPtr("9092"), + }, + }, }, }, Location: resourceGroup.Location, From e7dd3ec6781254473fbc3c699b70d6e725086f94 Mon Sep 17 00:00:00 2001 From: Miral Gadani Date: Tue, 3 Oct 2023 13:26:47 -0400 Subject: [PATCH 5/9] roachprod: azure default location roachtest: apt update before go install Update default location to `eastus`, since there is more quota there right now. Also, `apt-get update` before installing go (cdc) Epic: CC-25185 Release note: none --- pkg/roachprod/install/install.go | 4 +++- pkg/roachprod/roachprod.go | 2 +- pkg/roachprod/vm/azure/flags.go | 2 +- 3 files changed, 5 insertions(+), 3 deletions(-) diff --git a/pkg/roachprod/install/install.go b/pkg/roachprod/install/install.go index 2d437f2c5825..eb71574a8d67 100644 --- a/pkg/roachprod/install/install.go +++ b/pkg/roachprod/install/install.go @@ -102,7 +102,9 @@ sudo apt-get update; sudo apt-get install -y gcc; `, - "go": `sudo apt --yes install golang-go;`, + "go": ` +sudo apt-get update; +sudo apt-get install -y golang-go;`, "haproxy": ` sudo apt-get update; diff --git a/pkg/roachprod/roachprod.go b/pkg/roachprod/roachprod.go index 383bbf8a8b17..6d787d9cb526 100644 --- a/pkg/roachprod/roachprod.go +++ b/pkg/roachprod/roachprod.go @@ -1379,7 +1379,7 @@ func Create( if err := cleanupFailedCreate(l, clusterName); err != nil { l.Errorf("Error while cleaning up partially-created cluster: %s\n", err) } else { - l.Errorf("Cleaning up OK\n") + l.Printf("Cleaning up OK\n") } }() } else { diff --git a/pkg/roachprod/vm/azure/flags.go b/pkg/roachprod/vm/azure/flags.go index 4af27dde51f9..29f6ed4b04a6 100644 --- a/pkg/roachprod/vm/azure/flags.go +++ b/pkg/roachprod/vm/azure/flags.go @@ -32,7 +32,7 @@ type ProviderOpts struct { } var defaultLocations = []string{ - "eastus2", + "eastus", "westus", "westeurope", } From d17831d32696d27b3f8c751d7896d3b86210ff3d Mon Sep 17 00:00:00 2001 From: Radu Berinde Date: Wed, 4 Oct 2023 10:17:51 -0700 Subject: [PATCH 6/9] go.mod: bump Pebble to b013ca78e9dc b013ca78 db: keep track of virtual sstable size sum 62251e69 db: make checkpoint test even more deterministic c7c47d6b db: turn testingAlwaysWaitForCleanup into an option a05b0192 db: keep track of virtual sstable count in metrics 3c778710 db: add test for virtual sstable checkpointing cb4dab66 db: add metrics for num backing sstables and size 8317cf38 db: incrementally keep tracking of backing table size 0f80e184 Update index.html aa077af6 go.mod: specify Go 1.20 ccb9a7dc manifest: add invariant check for duplicate file backings 699fc0e8 db: only create one CreatedBackingTables entry per sstable b2da10c6 db: remove trailing whitespace from compacting log line 1d696c79 db: cleanup btree obsoletion logic Fixes: #111674 Release note: none --- DEPS.bzl | 6 +++--- build/bazelutil/distdir_files.bzl | 2 +- go.mod | 2 +- go.sum | 4 ++-- pkg/cli/interactive_tests/test_encryption.tcl | 4 ++-- 5 files changed, 9 insertions(+), 9 deletions(-) diff --git a/DEPS.bzl b/DEPS.bzl index 74b984830648..c1cdc77a5c24 100644 --- a/DEPS.bzl +++ b/DEPS.bzl @@ -1599,10 +1599,10 @@ def go_deps(): patches = [ "@com_github_cockroachdb_cockroach//build/patches:com_github_cockroachdb_pebble.patch", ], - sha256 = "9418eb96febfa50da2f02f87501d0459a8205fea8f91b539300a03c7f62a6d61", - strip_prefix = "github.com/cockroachdb/pebble@v0.0.0-20230927205513-725ebe297867", + sha256 = "da88d66f323769d609581b310f42a9bcc245cb3c321f0fae2aea7d2ed7c9fe13", + strip_prefix = "github.com/cockroachdb/pebble@v0.0.0-20231003235536-b013ca78e9dc", urls = [ - "https://storage.googleapis.com/cockroach-godeps/gomod/github.com/cockroachdb/pebble/com_github_cockroachdb_pebble-v0.0.0-20230927205513-725ebe297867.zip", + "https://storage.googleapis.com/cockroach-godeps/gomod/github.com/cockroachdb/pebble/com_github_cockroachdb_pebble-v0.0.0-20231003235536-b013ca78e9dc.zip", ], ) go_repository( diff --git a/build/bazelutil/distdir_files.bzl b/build/bazelutil/distdir_files.bzl index 696fcf0d0dd1..2fd855672322 100644 --- a/build/bazelutil/distdir_files.bzl +++ b/build/bazelutil/distdir_files.bzl @@ -321,7 +321,7 @@ DISTDIR_FILES = { "https://storage.googleapis.com/cockroach-godeps/gomod/github.com/cockroachdb/go-test-teamcity/com_github_cockroachdb_go_test_teamcity-v0.0.0-20191211140407-cff980ad0a55.zip": "bac30148e525b79d004da84d16453ddd2d5cd20528e9187f1d7dac708335674b", "https://storage.googleapis.com/cockroach-godeps/gomod/github.com/cockroachdb/gostdlib/com_github_cockroachdb_gostdlib-v1.19.0.zip": "c4d516bcfe8c07b6fc09b8a9a07a95065b36c2855627cb3514e40c98f872b69e", "https://storage.googleapis.com/cockroach-godeps/gomod/github.com/cockroachdb/logtags/com_github_cockroachdb_logtags-v0.0.0-20230118201751-21c54148d20b.zip": "ca7776f47e5fecb4c495490a679036bfc29d95bd7625290cfdb9abb0baf97476", - "https://storage.googleapis.com/cockroach-godeps/gomod/github.com/cockroachdb/pebble/com_github_cockroachdb_pebble-v0.0.0-20230927205513-725ebe297867.zip": "9418eb96febfa50da2f02f87501d0459a8205fea8f91b539300a03c7f62a6d61", + "https://storage.googleapis.com/cockroach-godeps/gomod/github.com/cockroachdb/pebble/com_github_cockroachdb_pebble-v0.0.0-20231003235536-b013ca78e9dc.zip": "da88d66f323769d609581b310f42a9bcc245cb3c321f0fae2aea7d2ed7c9fe13", "https://storage.googleapis.com/cockroach-godeps/gomod/github.com/cockroachdb/redact/com_github_cockroachdb_redact-v1.1.5.zip": "11b30528eb0dafc8bc1a5ba39d81277c257cbe6946a7564402f588357c164560", "https://storage.googleapis.com/cockroach-godeps/gomod/github.com/cockroachdb/returncheck/com_github_cockroachdb_returncheck-v0.0.0-20200612231554-92cdbca611dd.zip": "ce92ba4352deec995b1f2eecf16eba7f5d51f5aa245a1c362dfe24c83d31f82b", "https://storage.googleapis.com/cockroach-godeps/gomod/github.com/cockroachdb/sentry-go/com_github_cockroachdb_sentry_go-v0.6.1-cockroachdb.2.zip": "fbb2207d02aecfdd411b1357efe1192dbb827959e36b7cab7491731ac55935c9", diff --git a/go.mod b/go.mod index 955521cd4563..83fbe9b5e4c6 100644 --- a/go.mod +++ b/go.mod @@ -113,7 +113,7 @@ require ( github.com/cockroachdb/go-test-teamcity v0.0.0-20191211140407-cff980ad0a55 github.com/cockroachdb/gostdlib v1.19.0 github.com/cockroachdb/logtags v0.0.0-20230118201751-21c54148d20b - github.com/cockroachdb/pebble v0.0.0-20230927205513-725ebe297867 + github.com/cockroachdb/pebble v0.0.0-20231003235536-b013ca78e9dc github.com/cockroachdb/redact v1.1.5 github.com/cockroachdb/returncheck v0.0.0-20200612231554-92cdbca611dd github.com/cockroachdb/stress v0.0.0-20220803192808-1806698b1b7b diff --git a/go.sum b/go.sum index 5cf4d3599f9f..84f07e51a2db 100644 --- a/go.sum +++ b/go.sum @@ -489,8 +489,8 @@ github.com/cockroachdb/gostdlib v1.19.0/go.mod h1:+dqqpARXbE/gRDEhCak6dm0l14AaTy github.com/cockroachdb/logtags v0.0.0-20211118104740-dabe8e521a4f/go.mod h1:Vz9DsVWQQhf3vs21MhPMZpMGSht7O/2vFW2xusFUVOs= github.com/cockroachdb/logtags v0.0.0-20230118201751-21c54148d20b h1:r6VH0faHjZeQy818SGhaone5OnYfxFR/+AzdY3sf5aE= github.com/cockroachdb/logtags v0.0.0-20230118201751-21c54148d20b/go.mod h1:Vz9DsVWQQhf3vs21MhPMZpMGSht7O/2vFW2xusFUVOs= -github.com/cockroachdb/pebble v0.0.0-20230927205513-725ebe297867 h1:O/fBxpVLLedWnVw+kRDe2rcybSGFLiLXnnHgyrT/Pr0= -github.com/cockroachdb/pebble v0.0.0-20230927205513-725ebe297867/go.mod h1:nindLFinxeDPjP4qI9LtVHAwDef57/0s5KMfWgdktQc= +github.com/cockroachdb/pebble v0.0.0-20231003235536-b013ca78e9dc h1:eyGg4MSAQSEBY4fYPu7KNsCCFNIXwhkbEaSciMRfDWs= +github.com/cockroachdb/pebble v0.0.0-20231003235536-b013ca78e9dc/go.mod h1:6hk1eMY/u5t+Cf18q5lFMUA1Rc+Sm5I6Ra1QuPyxXCo= github.com/cockroachdb/redact v1.1.3/go.mod h1:BVNblN9mBWFyMyqK1k3AAiSxhvhfK2oOZZ2lK+dpvRg= github.com/cockroachdb/redact v1.1.5 h1:u1PMllDkdFfPWaNGMyLD1+so+aq3uUItthCFqzwPJ30= github.com/cockroachdb/redact v1.1.5/go.mod h1:BVNblN9mBWFyMyqK1k3AAiSxhvhfK2oOZZ2lK+dpvRg= diff --git a/pkg/cli/interactive_tests/test_encryption.tcl b/pkg/cli/interactive_tests/test_encryption.tcl index 73cc7701331d..557365501a7b 100644 --- a/pkg/cli/interactive_tests/test_encryption.tcl +++ b/pkg/cli/interactive_tests/test_encryption.tcl @@ -52,7 +52,7 @@ end_test start_test "Run pebble debug tool." send "$argv debug pebble db lsm $storedir\r" -eexpect "level | tables size val-bl | score | in | tables size | tables size | tables size | read | r w\r" +eexpect "level | tables size val-bl vtables | score | in | tables size | tables size | tables size | read | r w\r" end_test start_test "Restart with plaintext." @@ -106,7 +106,7 @@ end_test start_test "Run pebble debug tool with AES-256." send "$argv debug pebble db lsm $storedir --enterprise-encryption=path=$storedir,key=$keydir/aes-256.key,old-key=$keydir/aes-256.key\r" -eexpect "level | tables size val-bl | score | in | tables size | tables size | tables size | read | r w\r" +eexpect "level | tables size val-bl vtables | score | in | tables size | tables size | tables size | read | r w\r" # Try running without the encryption flag. send "$argv debug pebble db lsm $storedir\r" eexpect "If this is an encrypted store, make sure the correct encryption key is set." From 4ec167194af41e5c8950f21490e64205be174755 Mon Sep 17 00:00:00 2001 From: adityamaru Date: Fri, 29 Sep 2023 12:19:35 -0400 Subject: [PATCH 7/9] api: increase timeout to request execution details In large clusters requesting execution details can definitely take longer than 5 seconds. This is because it involves collecting cluster wide traces, goroutines and contacting the coordinator node of the job to dump its execution details. Since this is a debug only tool we bump the timeout to 300s to give it all the time it needs. Release note: None --- pkg/ui/workspaces/cluster-ui/src/api/jobProfilerApi.ts | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/pkg/ui/workspaces/cluster-ui/src/api/jobProfilerApi.ts b/pkg/ui/workspaces/cluster-ui/src/api/jobProfilerApi.ts index c9410aebfb30..6ded4c665599 100644 --- a/pkg/ui/workspaces/cluster-ui/src/api/jobProfilerApi.ts +++ b/pkg/ui/workspaces/cluster-ui/src/api/jobProfilerApi.ts @@ -10,7 +10,11 @@ import { cockroach } from "@cockroachlabs/crdb-protobuf-client"; import { fetchData } from "./fetchData"; -import { SqlExecutionRequest, executeInternalSql } from "./sqlApi"; +import { + SqlExecutionRequest, + executeInternalSql, + LONG_TIMEOUT, +} from "./sqlApi"; import { propsToQueryString } from "../util"; const JOB_PROFILER_PATH = "_status/job_profiler_execution_details"; @@ -75,6 +79,7 @@ export function collectExecutionDetails({ const req: SqlExecutionRequest = { execute: true, statements: [collectExecutionDetails], + timeout: LONG_TIMEOUT, }; return executeInternalSql(req).then(res => { From e9335c1ea7ca76dbf8c78ccf8652d0da0e65bd9a Mon Sep 17 00:00:00 2001 From: Andy Yang Date: Tue, 19 Sep 2023 14:50:59 -0400 Subject: [PATCH 8/9] gcjob_test: add more logging to TestGCJobRetry This patch adds more logging to `TestGCJobRetry` to help debug occasional flakes. Release note: None --- pkg/sql/gcjob_test/gc_job_test.go | 23 +++++++++++++++++++---- 1 file changed, 19 insertions(+), 4 deletions(-) diff --git a/pkg/sql/gcjob_test/gc_job_test.go b/pkg/sql/gcjob_test/gc_job_test.go index ed272647673f..131bce71aeb3 100644 --- a/pkg/sql/gcjob_test/gc_job_test.go +++ b/pkg/sql/gcjob_test/gc_job_test.go @@ -317,10 +317,25 @@ SELECT job_id FROM [SHOW JOBS] WHERE job_type = 'SCHEMA CHANGE GC' AND description LIKE '%foo%';`, ).Scan(&jobID) - tdb.CheckQueryResultsRetry(t, - "SELECT running_status FROM crdb_internal.jobs WHERE job_id = "+jobID, - [][]string{{string(sql.RunningStatusWaitingForMVCCGC)}}, - ) + + const expectedRunningStatus = string(sql.RunningStatusWaitingForMVCCGC) + testutils.SucceedsSoon(t, func() error { + var status, runningStatus, lastRun, nextRun, numRuns, jobErr string + tdb.QueryRow(t, fmt.Sprintf(` +SELECT status, running_status, error, last_run, next_run, num_runs +FROM crdb_internal.jobs +WHERE job_id = %s`, jobID)).Scan(&status, &runningStatus, &jobErr, &lastRun, &nextRun, &numRuns) + + t.Logf(`details about SCHEMA CHANGE GC job: {status: %q, running_status: %q, error: %q, last_run: %q, next_run: %q, num_runs: %q}`, + status, runningStatus, jobErr, lastRun, nextRun, numRuns) + + if runningStatus != expectedRunningStatus { + return errors.Newf(`running_status %s does not match expected status %s`, + runningStatus, expectedRunningStatus) + } + + return nil + }) } // TestGCTenant is lightweight test that tests the branching logic in Resume From 6eb934ae57768cd054f00ee5995354debd435a7e Mon Sep 17 00:00:00 2001 From: rharding6373 Date: Tue, 3 Oct 2023 16:17:58 -0700 Subject: [PATCH 9/9] sql: add tests for privileges for statements in udfs This PR adds test coverage for privileges in UDFs, e.g., SELECT and INSERT privileges. Epic: CRDB-25388 Informs: #87289 Release note: None --- .../testdata/logic_test/udf_privileges | 94 +++++++++++++++++++ 1 file changed, 94 insertions(+) diff --git a/pkg/sql/logictest/testdata/logic_test/udf_privileges b/pkg/sql/logictest/testdata/logic_test/udf_privileges index 5ed2758025fc..d39deae9b920 100644 --- a/pkg/sql/logictest/testdata/logic_test/udf_privileges +++ b/pkg/sql/logictest/testdata/logic_test/udf_privileges @@ -783,4 +783,98 @@ SET ROLE tester statement ok SELECT test.my_add(1,2) +statement ok +SET ROLE root + +subtest end + +subtest mutations + +statement ok +CREATE TABLE t (a INT, b INT); +CREATE FUNCTION f_insert() RETURNS VOID LANGUAGE SQL AS $$ INSERT INTO t VALUES (1,2); $$; +CREATE FUNCTION f_select() RETURNS INT LANGUAGE SQL AS $$ SELECT b FROM t WHERE a = 1; $$; +CREATE FUNCTION f_update() RETURNS VOID LANGUAGE SQL AS $$ UPDATE t SET b = 3 WHERE a = 1; $$; +CREATE FUNCTION f_delete() RETURNS VOID LANGUAGE SQL AS $$ DELETE FROM t WHERE a = 1; $$; +CREATE USER test_user; + +statement ok +SET ROLE test_user + +statement error pq: user test_user does not have INSERT privilege on relation t +select f_insert(); + +statement error pq: user test_user does not have SELECT privilege on relation t +select f_select(); + +statement error pq: user test_user does not have UPDATE privilege on relation t +select f_update(); + +statement error pq: user test_user does not have DELETE privilege on relation t +select f_delete(); + +statement ok +SET ROLE root + +statement ok +GRANT SELECT, INSERT, DELETE, UPDATE ON t TO test_user; + +statement ok +SET ROLE test_user + + +statement ok +SELECT f_insert(); + +query I +SELECT f_select(); +---- +2 + +statement ok +SELECT f_update(); + +query II +SELECT * FROM t; +---- +1 3 + +statement ok +SELECT f_delete(); + +query II +SELECT * FROM t; +---- + +statement ok +SET ROLE root + +statement ok +REVOKE SELECT, INSERT, DELETE, UPDATE ON t FROM test_user; + +statement ok +SET ROLE test_user + +statement error pq: user test_user does not have SELECT privilege on relation t +select f_select(); + +statement error pq: user test_user does not have INSERT privilege on relation t +select f_insert(); + +statement error pq: user test_user does not have UPDATE privilege on relation t +select f_update(); + +statement error pq: user test_user does not have DELETE privilege on relation t +select f_delete(); + +statement ok +SET ROLE root + +statement ok +DROP FUNCTION f_insert; +DROP FUNCTION f_select; +DROP FUNCTION f_update; +DROP FUNCTION f_delete; +DROP USER test_user; + subtest end