roachtest,roachprod: detect tenant-scope certs via help, unskip tests #83703
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
This change is |
|
@cucaroach I'm this |
stevendanna
force-pushed
the
more-tenant-scoped-cert-stuff
branch
2 times, most recently
from
7de76c6 to
0c0c8f5
Compare
stevendanna
force-pushed
the
more-tenant-scoped-cert-stuff
branch
from
0c0c8f5 to
0157831
Compare
stevendanna
added a commit
to stevendanna/cockroach
that referenced
this pull request
Jul 1, 2022
The secure URL refers to paths on disk on the clusters in the node. Since we only create the tenant-scoped certs on the tenant node, we need to run workload from that node. Fixes cockroachdb#82266 Depends on cockroachdb#83703 Release note: None
rimadeodhar
approved these changes
Jul 5, 2022
There was a problem hiding this comment.
Reviewed 4 of 4 files at r1, all commit messages.
Reviewable status:complete! 0 of 0 LGTMs obtained (waiting on @cucaroach and @erikgrinaker)
The version comparison code to detect tenant scoped certificates was still incorrect because the "alpha" portion of semver versions is compare lexicographically. Since our alpha versions contain a count of commits, this broke when we hit commit 1000 since the last tag. Here, we search the help on the command to see if it supports the tenant-scoped certs flag. If it does, we assume we will need them. There is duplication between the multitenant tests and roachprod still that we should clean up. Release note: None
stevendanna
force-pushed
the
more-tenant-scoped-cert-stuff
branch
from
0157831 to
c1202a8
Compare
cucaroach
approved these changes
Jul 6, 2022
|
bors r=rimadeodhar,cucaroach |
|
Build succeeded: |
craig bot
pushed a commit
that referenced
this pull request
Jul 7, 2022
83014: ui: add internal app filter to active statements and transactions pages r=ericharmeling a=ericharmeling This PR adds a single internal app filter option on to the Active Statements and Active Transactions pages. Active statements and transactions run by internal apps are no longer displayed by default. See commit message for release note. https://user-images.githubusercontent.com/27286675/174156635-39d8649a-df91-4550-adb5-b3c167d54ed5.mov Fixes #81072. 83707: roachtest: run workload from the tenant node r=knz a=stevendanna The secure URL refers to paths on disk on the clusters in the node. Since we only create the tenant-scoped certs on the tenant node, we need to run workload from that node. Fixes #82266 Depends on #83703 Release note: None 84003: storage: close pebble iter gracefully when NewPebbleSSTIterator fails r=erikgrinaker a=msbutler Currently, if `pebble.NewExternalIter` sets pebbleIterator.inuse to True, but then fails, the subsequent `pebbleIterator.destroy()` will panic unecessarily, since the caller of `pebble.NewExternalIter` is not actually using the iter. This bug causes TestBackupRestoreChecksum to flake in #83984. To fix, this patch uses pebble.Close() to gracefully close the pebbleIterator if `pebble.NewExternalIter` fails. Release Note: None 84039: prometheus: use older node_exporter r=nicktrav a=tbg v1.3.1, the most up to date released version, has a bug that inflates the bytes written by ~8x for NVMe drives (which in particular includes the default drives for our GCE roachprod machines). Fundamentally this is caused by the fact that these devices use a 4K sector size whereas the kernel will always report based on a 512B sector size. This took us a while to figure out, and to avoid repeating this exercise periodically, downgrade node_exporter to 1.2.2, which pre-dates a refactor that introduces the regression. See: prometheus/node_exporter#2310 Release note: None Co-authored-by: Eric Harmeling <[email protected]> Co-authored-by: Steven Danna <[email protected]> Co-authored-by: Michael Butler <[email protected]> Co-authored-by: Tobias Grieger <[email protected]>
abarganier
added a commit
to abarganier/cockroach-go
that referenced
this pull request
Jul 18, 2022
Previously, whether the test server created tenant-scoped client certificates for tests was based on a hardcoded version gate. This was sufficient in the past, but as tenant-scoped client certs are now being backported to older cockroachdb versions, a more dynamic approach to determine whether or not these certificates are available is needed. This patch adds a mechanism to do so. The new approach runs the `cockroach cert create-client --help` command to view the available flags for the current cockroach binary. If the `--tenant-scope` flag is present in the help text, then we can say with confidence that tenant scoped client certificates are available. We can use this to signal the broader system to make use of these certificates when running tests in secure mode. This follows the approach used in: cockroachdb/cockroach#83703
abarganier
pushed a commit
to abarganier/cockroach
that referenced
this pull request
Jul 19, 2022
The secure URL refers to paths on disk on the clusters in the node. Since we only create the tenant-scoped certs on the tenant node, we need to run workload from that node. Fixes cockroachdb#82266 Depends on cockroachdb#83703 Release note: None
abarganier
pushed a commit
to abarganier/cockroach
that referenced
this pull request
Jul 20, 2022
The secure URL refers to paths on disk on the clusters in the node. Since we only create the tenant-scoped certs on the tenant node, we need to run workload from that node. BACKPORT NOTES: Also adds the `multi-tenant` owner to the roachtest registry and TEAMS.yml, which were not yet added to the release branch. Fixes cockroachdb#82266 Depends on cockroachdb#83703 Release note: None
30 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The version comparison code to detect tenant scoped certificates was
still incorrect because the "alpha" portion of semver versions is
compare lexicographically. Since our alpha versions contain a count of
commits, this broke when we hit commit 1000 since the last tag.
Here, we search the help on the command to see if it supports the
tenant-scoped certs flag. If it does, we assume we will need them.
There is duplication between the multitenant tests and roachprod still
that we should clean up.
Fixes #82926
Release note: None