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-30640] allow manual version control for standard clusters #252

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,12 @@ All notable changes to this project will be documented in this file.
The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).

## [1.11.0] - 2024-12-05

### Added

- Added support for manual version control of Standard tier clusters.

## [1.10.0] - 2024-11-15

### Fixed
Expand Down
2 changes: 1 addition & 1 deletion docs/resources/cluster.md
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ resource "cockroach_cluster" "basic" {

- `backup_config` (Attributes) The backup settings for a cluster.
Each cluster has backup settings that determine if backups are enabled, how frequently they are taken, and how long they are retained for. Use this attribute to manage those settings. (see [below for nested schema](#nestedatt--backup_config))
- `cockroach_version` (String) Major version of CockroachDB running on the cluster.
- `cockroach_version` (String) Major version of CockroachDB running on the cluster. This value can be used to orchestrate version upgrades. Supported for ADVANCED and STANDARD clusters (when `serverless.upgrade_type` set to 'MANUAL').
- `dedicated` (Attributes) (see [below for nested schema](#nestedatt--dedicated))
- `delete_protection` (Boolean) Set to true to enable delete protection on the cluster. If unset, the server chooses the value on cluster creation, and preserves the value on cluster update.
- `parent_id` (String) The ID of the cluster's parent folder. 'root' is used for a cluster at the root level.
Expand Down
113 changes: 63 additions & 50 deletions internal/provider/cluster_resource.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ import (
"regexp"
"sort"
"strconv"
"strings"
"time"

"github.com/cockroachdb/cockroach-cloud-sdk-go/v5/pkg/client"
Expand Down Expand Up @@ -123,7 +122,7 @@ func (r *clusterResource) Schema(
PlanModifiers: []planmodifier.String{
stringplanmodifier.UseStateForUnknown(),
},
Description: "Major version of CockroachDB running on the cluster.",
MarkdownDescription: "Major version of CockroachDB running on the cluster. This value can be used to orchestrate version upgrades. Supported for ADVANCED and STANDARD clusters (when `serverless.upgrade_type` set to 'MANUAL').",
fantapop marked this conversation as resolved.
Show resolved Hide resolved
},
"account_id": schema.StringAttribute{
Computed: true,
Expand Down Expand Up @@ -377,6 +376,27 @@ func (r *clusterResource) ConfigValidators(_ context.Context) []resource.ConfigV
}
}

func derivePlanType(cluster *CockroachCluster) (client.PlanType, error) {
var planType client.PlanType
if IsKnown(cluster.Plan) {
planType = client.PlanType(cluster.Plan.ValueString())
} else if cluster.DedicatedConfig != nil {
planType = client.PLANTYPE_ADVANCED
} else if cluster.ServerlessConfig != nil {
if cluster.ServerlessConfig.UsageLimits != nil && IsKnown(cluster.ServerlessConfig.UsageLimits.ProvisionedVirtualCpus) {
rgcase marked this conversation as resolved.
Show resolved Hide resolved
planType = client.PLANTYPE_STANDARD
} else {
planType = client.PLANTYPE_BASIC
}
} else {
return "", fmt.Errorf("could not derive plan type, plan must contain either a ServerlessConfig or a DedicatedConfig")
}
if !planType.IsValid() {
return "", fmt.Errorf("invalid plan type %q", cluster.Plan.ValueString())
}
return planType, nil
}

func (r *clusterResource) Create(
ctx context.Context, req resource.CreateRequest, resp *resource.CreateResponse,
) {
Expand Down Expand Up @@ -439,6 +459,20 @@ func (r *clusterResource) Create(
}
}

if IsKnown(plan.CockroachVersion) {
planType, err := derivePlanType(&plan)
if err != nil {
resp.Diagnostics.AddError("Invalid Configuration", err.Error())
return
}

if planType == client.PLANTYPE_BASIC {
resp.Diagnostics.AddError("Invalid Attribute Combination", "cockroach_version is not supported for BASIC clusters.")
fantapop marked this conversation as resolved.
Show resolved Hide resolved
} else {
resp.Diagnostics.AddError("Unsupported Attribute", "cockroach_version is not supported during cluster creation for STANDARD clusters. STANDARD clusters are automatically created using the latest available version.")
}
}

clusterSpec.SetServerless(*serverless)
} else if plan.DedicatedConfig != nil {
dedicated := client.DedicatedClusterCreateSpecification{}
Expand Down Expand Up @@ -788,66 +822,45 @@ func (r *clusterResource) Update(
// CRDB Upgrades/Downgrades
if IsKnown(plan.CockroachVersion) && plan.CockroachVersion != state.CockroachVersion {
planVersion := plan.CockroachVersion.ValueString()
stateVersion := state.CockroachVersion.ValueString()
traceAPICall("ListMajorClusterVersions")
apiResp, _, err := r.provider.service.ListMajorClusterVersions(ctx, &client.ListMajorClusterVersionsOptions{})
if err != nil {
resp.Diagnostics.AddError("Couldn't retrieve CockroachDB version list", formatAPIErrorMessage(err))
return
}

// Target version must be a valid major version
var versionValid bool
for _, v := range apiResp.Versions {
if v.Version == planVersion {
versionValid = true
break
}
}
if !versionValid {
validVersions := make([]string, len(apiResp.Versions))
for i, v := range apiResp.Versions {
validVersions[i] = v.Version
// Unfortunately the update cluster api endpoint doesn't allow updating
// the version and anything else in the same call. Since version may
// rely on upgrade_type and plan, if they are updated at the same time
// we need to fail out.
// We could explore doing the version upgrade after the other updates to
// avoid this 2 step apply requirement.

if client.PlanType(state.Plan.ValueString()) == client.PLANTYPE_BASIC {
if client.PlanType(plan.Plan.ValueString()) != client.PLANTYPE_BASIC {
resp.Diagnostics.AddError("Error updating cluster", "plan and version must not be changed during the same terraform plan.")
} else {
resp.Diagnostics.AddError("Error updating cluster", "cockroach_version is not supported for BASIC clusters, please update plan before setting cockroach_version.")
}
resp.Diagnostics.AddError("Invalid CockroachDB version",
fmt.Sprintf(
"'%s' is not a valid major CockroachDB version. Valid versions include [%s]",
planVersion,
strings.Join(validVersions, "|")))
return
}

// Next, if we are upgrading, it must be a valid upgrade from the
// current version, according to the ListMajorClusterVersions API.
if upgrading, err := isUpgrade(stateVersion, planVersion); upgrading {
var currentVersionInfo client.ClusterMajorVersion
for _, v := range apiResp.Versions {
if v.Version == stateVersion {
currentVersionInfo = v
break
}
if state.ServerlessConfig != nil {
stateUpgradeTypeString := state.ServerlessConfig.UpgradeType.ValueString()
planUpgradeTypeString := plan.ServerlessConfig.UpgradeType.ValueString()
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it guaranteed that plan.ServerlessConfig will be non-nil here?

Copy link
Collaborator Author

@fantapop fantapop Dec 5, 2024

Choose a reason for hiding this comment

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

I didn't know this so I wrote a test to try it out. Apparently we're protected against this here. ModifyPlan is run before the update should it should never be the case that state.ServerlessConfig is set but plan.ServerlessConfig is not. I went ahead and left the test in.

if stateUpgradeTypeString != planUpgradeTypeString {
resp.Diagnostics.AddError("Error updating cluster", "upgrade_type and version must not be changed during the same terraform plan.")
return
}

var validUpgrade bool
for _, v := range currentVersionInfo.AllowedUpgrades {
if v == planVersion {
validUpgrade = true
break
}
stateUpgradeTypePtr, err := client.NewUpgradeTypeTypeFromValue(stateUpgradeTypeString)
if err != nil {
resp.Diagnostics.AddError("Error updating cluster", err.Error())
return
}
if !validUpgrade {
resp.Diagnostics.AddError("Invalid CockroachDB version",
fmt.Sprintf(
"Cannot change major version to '%s'. Valid upgrade versions include [%s]",
planVersion,
strings.Join(currentVersionInfo.AllowedUpgrades, "|")))
stateUpgradeType := *stateUpgradeTypePtr
if stateUpgradeType != client.UPGRADETYPETYPE_MANUAL {
resp.Diagnostics.AddError("Error updating cluster", "upgrade_type must be set to MANUAL before setting cockroach_version.")
return
}
} else if err != nil {
resp.Diagnostics.AddError("Invalid CockroachDB version", err.Error())
return
}

// We rely on the server to validate that the upgrade version is
// actually valid to update to.
traceAPICall("UpdateCluster")
clusterObj, _, err := r.provider.service.UpdateCluster(ctx, plan.ID.ValueString(), &client.UpdateClusterSpecification{
CockroachVersion: &planVersion,
Expand Down
Loading
Loading