Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CC API docs updates for new plans/spec #18931

Merged
merged 9 commits into from
Sep 25, 2024
Merged

CC API docs updates for new plans/spec #18931

merged 9 commits into from
Sep 25, 2024

Conversation

mikeCRL
Copy link
Contributor

@mikeCRL mikeCRL commented Sep 23, 2024

src/current/cockroachcloud/cloud-api.md

Key updates to match the latest spec and offer additional details:

  • Updated intro to
    • refer to latest API 'version'
    • add note about TF provider
    • flag the need to update code for former Serverless clusters
  • Modified cluster creation example to use STANDARD instead of Serverless
  • Updated cluster creation payload to include plan, and usage_limits instead of spendLimit
  • Modified the response examples for cluster creation, getting a cluster, getting all clusters, to align on the json object return for a cluster per spec example (Standard example only) and define more fields
  • Split ## Set the maximum resource limits of a Serverless cluster section into two new ones:
    • Change resource limits for a Basic cluster & Change provisioned vCPUs for a Standard cluster
    • Updated the endpoint and payload for changing resource limits
  • Updated various references from "Serverless" to "Basic" or "Standard" throughout the document

Notes:

  • Without improved Basic coverage, there wasn't an obvious place for delete_protection, so I didn't add it yet - want to confirm with you all first.
  • There are areas where the doc has been inconsistent with coverage in the spec and it's unclear where that's intentional. Generally, I'd like to plan a follow up review and iteration.
  • See comment about additional iteration after reviews, and follow-up plans

src/current/v24.2/api-support-policy.md

  • Improved the markdown structure of table, for easier visibility and easier editing in the future.
  • This may still be hard to view for this diff, so here are my proposed changes just to a few entries in the Availability column, for each interface. I'd appreciate verification of these edits and any other suggestions you may have; e.g. whether to add any stability/versioning mentions on CC API or in the main api doc.
    • Prometheus endpoint: Dedicated -> Standard+Advanced (maybe metrics export should be a different line item, or not count at all here, i.e. drop the Standard mention. CC @kevin-v-ngo )
    • Cluster API: Dedicated -> Advanced
    • DB Console: Dedicated -> Advanced
    • Logging: Dedicated -> Standard+Advanced (similarly maybe Standard needs a separate row? CC @kevin-v-ngo)
    • ccloud CLI: No change ('applies to CC')
    • Cloud API: No change ('applies to CC')
    • Cloud Console: No change ('applies to CC')

To do:

  • Replicate v24.2 changes in all supported versions

Closes DOC-8920 DOC-9735 DOC-9730

Copy link

netlify bot commented Sep 23, 2024

Netlify Preview

Name Link
🔨 Latest commit 23d1ba6
🔍 Latest deploy log https://app.netlify.com/sites/cockroachdb-docs/deploys/66f458232481bc0008b1e149
😎 Deploy Preview https://deploy-preview-18931--cockroachdb-docs.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@mikeCRL mikeCRL changed the title API updates for Cloud 2.0 API updates for new plans/spec Sep 23, 2024
@mikeCRL mikeCRL changed the title API updates for new plans/spec CC API docs updates for new plans/spec Sep 23, 2024
@mikeCRL mikeCRL marked this pull request as ready for review September 23, 2024 17:47
src/current/cockroachcloud/cloud-api.md Outdated Show resolved Hide resolved
src/current/cockroachcloud/cloud-api.md Outdated Show resolved Hide resolved
src/current/cockroachcloud/cloud-api.md Show resolved Hide resolved
src/current/cockroachcloud/cloud-api.md Outdated Show resolved Hide resolved
"spec": {
"serverless": {
"regions": [
"{region_name}"
],
"spendLimit": {spend_limit}
"usage_limits": {
"provisioned_virtual_cpus": "2"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you update this example to use 4 vCPUs? 2 isn't allowed on Azure and 4 is our lower limit recommendation for production. It needs an jupdate to the --data field in the curl command too.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mdlinville I think you're confusing provisioned_virtual_cpus with the Dedicated Hardware config field num_virtual_cpus. It doesn't look like provisioned_virtual_cpus has any such restrictions. @andy-kimball are all positive values valid here?

@@ -669,16 +799,17 @@ The service account associated with the secret key must have the Cluster Adminis
{% include_cached copy-clipboard.html %}
~~~ shell
curl --request DELETE \
--url 'https://cockroachlabs.cloud/api/v1/clusters/{cluster_id}/sql-users/{sql_username}'' \
--url 'https://cockroachlabs.cloud/api/v1/clusters/{cluster_id}/sql-users/{sql_username}' \
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: Poor alignment. I'd align --url with --request.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Leaving for consistency with other examples for now

{{site.data.alerts.end}}
- `{sql_username}` is the username of the SQL user you want to delete.
- `{secret_key}` is the secret key for the service account.

If the request was successful, the client will receive a response with the name of the deleted user.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we need to show the output.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Refined like the others

src/current/cockroachcloud/cloud-api.md Outdated Show resolved Hide resolved
src/current/cockroachcloud/cloud-api.md Show resolved Hide resolved
src/current/v24.2/api-support-policy.md Show resolved Hide resolved
@mikeCRL
Copy link
Contributor Author

mikeCRL commented Sep 24, 2024

Thanks for the reviews. Working on this now.

Co-authored-by: Matt Linville (he/him) <[email protected]>
Copy link
Contributor Author

@mikeCRL mikeCRL left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

About to push an update. I believe I've addressed or replied to all comments.

Major changes:

  • Split the cluster creation into 3 sections, one each for Basic, Standard, Advanced.
  • Removed a number of unneeded response objects and field definitions, often linking out to the spec for details.
  • Confirmed that updating config does in fact return the updated cluster object (existing docs were wrong).

Follow-up items to log in an issue and revisit:

  • Establish consistency in how we link out to the spec for more information for a given section.
  • Establish a way to use content like field definitions directly from the spec.
  • Include latest API version number automatically by using a remote_include and extracting it from the cloud manifest.json
  • Include the CC version header in all example calls
  • Use includes for repeated text
  • Consider changing field definitions that say “fieldname is” to “fieldname:” or some other consistent format.
  • Revisit use of { vs < for field name definitions.
  • Remove raw json tabs or just show formatted json in the curl request.
  • General reexamination of details about the API included vs. omitted here, and if omitted, whether they should be included or referenced by linking to the spec.

src/current/cockroachcloud/cloud-api.md Outdated Show resolved Hide resolved
src/current/cockroachcloud/cloud-api.md Outdated Show resolved Hide resolved
@@ -408,7 +504,7 @@ Where:

- `{cluster_id}` is the unique ID of this cluster.
{{site.data.alerts.callout_info}}
The cluster ID used in the Cloud API is different from the routing ID used when [connecting to clusters]({% link cockroachcloud/connect-to-a-basic-cluster.md %}).
The cluster ID used in the Cloud API is different from the routing ID used when [connecting to clusters]({% link cockroachcloud/connect-to-your-cluster.md %}).
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Saving this as a follow-up action

@@ -501,54 +596,71 @@ If the request was successful, the client will receive a list of all clusters wi
{
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Swapped to a high level representation, instead, representing real output from my testing

{
  "clusters": [
    {<cluster_object1>},
    {<cluster_object2>},
    {<cluster_object3>}
  ],
  "pagination": null
}     

}
}
~~~

Where:

- `{cloud_provider}` is the name of the cloud infrastructure provider. Possible values are: `GCP`, `AWS`, `AZURE`.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Dropping these, but adding a follow up task to standardize field definition format

@@ -658,9 +784,13 @@ If the request was successful, the client will receive a response with the name

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Streamlining and fixing braces instead of removing for now

@@ -669,16 +799,17 @@ The service account associated with the secret key must have the Cluster Adminis
{% include_cached copy-clipboard.html %}
~~~ shell
curl --request DELETE \
--url 'https://cockroachlabs.cloud/api/v1/clusters/{cluster_id}/sql-users/{sql_username}'' \
--url 'https://cockroachlabs.cloud/api/v1/clusters/{cluster_id}/sql-users/{sql_username}' \
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Leaving for consistency with other examples for now

{{site.data.alerts.end}}
- `{sql_username}` is the username of the SQL user you want to delete.
- `{secret_key}` is the secret key for the service account.

If the request was successful, the client will receive a response with the name of the deleted user.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Refined like the others

src/current/cockroachcloud/cloud-api.md Show resolved Hide resolved
src/current/v24.2/api-support-policy.md Show resolved Hide resolved
@@ -142,7 +157,7 @@ For example, to create a new free Serverless cluster named "notorious-moose" usi
curl --request POST \
--url https://cockroachlabs.cloud/api/v1/clusters \
--header 'Authorization: Bearer {secret_key}' \
--data '{"name":"notorious-moose","provider":"GCP","spec":{"serverless":{"regions":["us-central1"],"spendLimit":0}}}'
--json '{"name":"basic-test","provider":"GCP","plan":"BASIC","spec":{"serverless":{"regions":["us-central1"],"usage_limits":{"storage_mib_limit":"5242880","request_unit_limit":"50000000"}}}}'

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's rare for users to need limits; I don't think we should show an example using them. Instead, just like before, we should show the "unlimited" case where there's no "usage_limits" section. This is what we were doing before, when setting spendLimit = 0; I don't think we should change that.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good. Done.

src/current/cockroachcloud/cloud-api.md Show resolved Hide resolved
curl --request POST \
--url https://cockroachlabs.cloud/api/v1/clusters \
--header 'Authorization: Bearer {secret_key}' \
--json '{"name":"{cluster_name}","provider":"{cloud_provider}","plan":"ADVANCED","spec":{"dedicated":{"region_nodes":{"{region_name}":3},"hardware":{"machine_spec":{"machine_type":"{machine_type}"}},"cockroach_version":"{version}"}}}'

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should not be encouraging users to use machine_spec with Advanced. Instead, we should be pointing them towards num_virtual_cpus.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

## Set the maximum resource limits of a Serverless cluster
<section class="filter-content" markdown="1" data-scope="curl">

{% include_cached copy-clipboard.html %}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the right place to show usage limits in use, rather than above.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍


For details about returned fields, refer to the [response example and schema](https://www.cockroachlabs.com/docs/api/cloud/v1.html#patch-/api/v1/clusters/-cluster_id-) in the API reference.

## Change the number of provisioned vCPUs for a Standard cluster

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This might be a good place to mention that the number of provisioned vCPUs can be increased any number of times, but can only be decreased up to 3 times per week.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done. Thanks.

@@ -567,7 +567,7 @@ For an in-depth explanation of CockroachDB {{ site.data.products.serverless }} p

<h3 id="2023-01-09-api-changes"> Cloud API changes </h3>

- The [create cluster]({% link cockroachcloud/cloud-api.md %}#create-a-new-cluster) request now exposes the `restrict-egress-traffic` boolean field to allow dedicated clusters to be created with a [deny-by-default egress traffic policy]({% link cockroachcloud/egress-perimeter-controls.md %}#use-a-deny-by-default-egress-traffic-policy). This field and the broader egress perimeter controls capability can be used only with [private dedicated clusters]({% link cockroachcloud/private-clusters.md %}), which require the `network-visibility` field to be set to `NETWORK_VISIBILITY_PRIVATE`.
- The [create cluster]({% link cockroachcloud/cloud-api.md %}#create-a-cluster) request now exposes the `restrict-egress-traffic` boolean field to allow dedicated clusters to be created with a [deny-by-default egress traffic policy]({% link cockroachcloud/egress-perimeter-controls.md %}#use-a-deny-by-default-egress-traffic-policy). This field and the broader egress perimeter controls capability can be used only with [private dedicated clusters]({% link cockroachcloud/private-clusters.md %}), which require the `network-visibility` field to be set to `NETWORK_VISIBILITY_PRIVATE`.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

dedicated => Advanced

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are many old plan mentions in that doc; we're leaving them as-is, at least for now. A new release note today will cover the name changes.

Copy link
Contributor Author

@mikeCRL mikeCRL left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@andy-kimball Updated per your review. Thank you. Would you please take a look at the changes in the latest commit? (I won't hold up merging for this if you're busy prior to today's launch.)

@@ -142,7 +157,7 @@ For example, to create a new free Serverless cluster named "notorious-moose" usi
curl --request POST \
--url https://cockroachlabs.cloud/api/v1/clusters \
--header 'Authorization: Bearer {secret_key}' \
--data '{"name":"notorious-moose","provider":"GCP","spec":{"serverless":{"regions":["us-central1"],"spendLimit":0}}}'
--json '{"name":"basic-test","provider":"GCP","plan":"BASIC","spec":{"serverless":{"regions":["us-central1"],"usage_limits":{"storage_mib_limit":"5242880","request_unit_limit":"50000000"}}}}'
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good. Done.

curl --request POST \
--url https://cockroachlabs.cloud/api/v1/clusters \
--header 'Authorization: Bearer {secret_key}' \
--json '{"name":"{cluster_name}","provider":"{cloud_provider}","plan":"ADVANCED","spec":{"dedicated":{"region_nodes":{"{region_name}":3},"hardware":{"machine_spec":{"machine_type":"{machine_type}"}},"cockroach_version":"{version}"}}}'
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

src/current/cockroachcloud/cloud-api.md Show resolved Hide resolved
@@ -305,7 +339,7 @@ Where:

- `{cluster_id}` is the cluster ID returned after creating the cluster.
{{site.data.alerts.callout_info}}
The cluster ID used in the Cloud API is different from the routing ID used when [connecting to clusters]({% link cockroachcloud/connect-to-a-basic-cluster.md %}).
The cluster ID used in the Cloud API is different from the routing ID used when [connecting to clusters]({% link cockroachcloud/connect-to-your-cluster.md %}).
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed all unnecessary 'copy' includes.

## Set the maximum resource limits of a Serverless cluster
<section class="filter-content" markdown="1" data-scope="curl">

{% include_cached copy-clipboard.html %}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

src/current/cockroachcloud/cloud-api.md Outdated Show resolved Hide resolved

For details about returned fields, refer to the [response example and schema](https://www.cockroachlabs.com/docs/api/cloud/v1.html#patch-/api/v1/clusters/-cluster_id-) in the API reference.

## Change the number of provisioned vCPUs for a Standard cluster
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done. Thanks.

To update the provisioned vCPUs for a cluster, send a `PATCH` request to the `/v1/clusters/{cluster_id}` endpoint.

{{site.data.alerts.callout_success}}
The service account associated with the secret key must have the Cluster Administrator or Cluster Developer [role]({% link cockroachcloud/authorization.md %}#organization-user-roles).
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed


Where:

- `{cluster_id}` is the unique ID of this cluster.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

saved as follow up task

@@ -567,7 +567,7 @@ For an in-depth explanation of CockroachDB {{ site.data.products.serverless }} p

<h3 id="2023-01-09-api-changes"> Cloud API changes </h3>

- The [create cluster]({% link cockroachcloud/cloud-api.md %}#create-a-new-cluster) request now exposes the `restrict-egress-traffic` boolean field to allow dedicated clusters to be created with a [deny-by-default egress traffic policy]({% link cockroachcloud/egress-perimeter-controls.md %}#use-a-deny-by-default-egress-traffic-policy). This field and the broader egress perimeter controls capability can be used only with [private dedicated clusters]({% link cockroachcloud/private-clusters.md %}), which require the `network-visibility` field to be set to `NETWORK_VISIBILITY_PRIVATE`.
- The [create cluster]({% link cockroachcloud/cloud-api.md %}#create-a-cluster) request now exposes the `restrict-egress-traffic` boolean field to allow dedicated clusters to be created with a [deny-by-default egress traffic policy]({% link cockroachcloud/egress-perimeter-controls.md %}#use-a-deny-by-default-egress-traffic-policy). This field and the broader egress perimeter controls capability can be used only with [private dedicated clusters]({% link cockroachcloud/private-clusters.md %}), which require the `network-visibility` field to be set to `NETWORK_VISIBILITY_PRIVATE`.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are many old plan mentions in that doc; we're leaving them as-is, at least for now. A new release note today will cover the name changes.

Base automatically changed from cloud-2.0 to main September 25, 2024 18:21
Copy link

netlify bot commented Sep 25, 2024

Deploy Preview for cockroachdb-interactivetutorials-docs canceled.

Name Link
🔨 Latest commit 23d1ba6
🔍 Latest deploy log https://app.netlify.com/sites/cockroachdb-interactivetutorials-docs/deploys/66f45823248a480008b2f1e1

Copy link

netlify bot commented Sep 25, 2024

Deploy Preview for cockroachdb-api-docs canceled.

Name Link
🔨 Latest commit 23d1ba6
🔍 Latest deploy log https://app.netlify.com/sites/cockroachdb-api-docs/deploys/66f4582306f65d0008bae5cb

@mikeCRL mikeCRL merged commit 8f2d690 into main Sep 25, 2024
6 checks passed
@mikeCRL mikeCRL deleted the api-updates-cloud-2.0 branch September 25, 2024 18:53
@mikeCRL mikeCRL mentioned this pull request Sep 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants