server: support nodes API endpoint for secondary shared tenants#131644
Merged
craig[bot] merged 1 commit intocockroachdb:masterfrom Nov 1, 2024
Merged
server: support nodes API endpoint for secondary shared tenants#131644craig[bot] merged 1 commit intocockroachdb:masterfrom
craig[bot] merged 1 commit intocockroachdb:masterfrom
Conversation
Member
|
This change is |
1ba8423 to
56dc3e7
Compare
dhartunian
approved these changes
Oct 1, 2024
stevendanna
approved these changes
Oct 3, 2024
b6eaa48 to
b617593
Compare
b617593 to
499eac9
Compare
stevendanna
approved these changes
Oct 21, 2024
499eac9 to
e81ac68
Compare
The "nodes" status API endpoint should work with "shared" secondary tenants. Since nodes are common to all the tenants, this API endpoint behaves similar to that of system tenant. Shared service tenants implicitly have all capabilities. Tenant connector implements TenantStatusServer interface. Status server in secondary tenants make use of tenant connector to fetch nodes information. Support for external tenants is tracked in a separate ticket. Fixes: cockroachdb#110023 Epic: CRDB-38968 Release note (multi-tenancy): The "nodes" endpoint should work for "shared" secondary tenants. Since nodes are common to all the tenants, this API endpoint behaves similar to that of system tenant.
e81ac68 to
d8fd187
Compare
cthumuluru-crdb
commented
Oct 30, 2024
| [cluster] requesting nodes... received response... | ||
| [cluster] requesting nodes: last request failed: rpc error: ... | ||
| [cluster] requesting nodes: creating error output: debug/nodes.json.err.txt... done | ||
| [cluster] requesting nodes... received response... writing JSON output: debug/nodes.json... done |
Contributor
Author
There was a problem hiding this comment.
@stevendanna - sorry for the noise on this PR. After pushing the changes I noticed a couple of test failures in the CI run. They are related to data driven tests. I fixed the expected debug zip output to match the code fixes.
More importantly, I was curious why testzip_external_process_virtualization test was not failing with capability missing error. After looking at both the test for differences, data driven tests in zip_tenant_test.go are running in insecure mode due to #77173 and that's the reason they were passing without any issues.
cockroach/pkg/cli/zip_tenant_test.go
Lines 76 to 86 in c68c559
I'll add this issue to CRDB-38970 if it is not already being tracked. This PR is ready to be committed now. Can you please take a look and bless it one more time?
stevendanna
approved these changes
Oct 30, 2024
Contributor
Author
|
bors r+ |
Contributor
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
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 "nodes" status API endpoint should work with "shared" secondary tenants. Since nodes are common to all the tenants, this API endpoint behaves similar to that of system tenant. Shared service tenants implicitly have all capabilities.
Tenant connector implements TenantStatusServer interface. Status server in secondary tenants make use of tenant connector to fetch nodes information.
Support for external tenants is tracked in a separate ticket.
Fixes: #110023
Epic: CRDB-38968
Release note (multi-tenancy): The "nodes" endpoint should work for "shared" secondary tenants. Since nodes are common to all the tenants, this API endpoint behaves similar to that of system tenant.