Use resource package for k8s values and add code for conversion for v…

…alues accepted/returned by bridge API.

Co-authored-by: Chris Bandy <[email protected]>

config/crd/bases/postgres-operator.crunchydata.com_crunchybridgeclusters.yaml

@@ -278,9 +278,12 @@ spec:
                 description: State of cluster in Bridge.
                 type: string
               storage:
-                description: The amount of storage available to the cluster in gigabytes.
-                format: int64
-                type: integer
+                anyOf:
+                - type: integer
+                - type: string
+                description: The amount of storage available to the cluster.
+                pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$
+                x-kubernetes-int-or-string: true
             type: object
         type: object
     served: true

examples/crunchybridgecluster/crunchybridgecluster.yaml

@@ -10,4 +10,4 @@ spec:
   provider: aws
   region: us-east-2
   secret: crunchy-bridge-api-key
-  storage: 10G
+  storage: 10Gi

internal/bridge/client.go

@@ -88,7 +88,7 @@ func (c *ClusterApiResource) AddDataToClusterStatus(cluster *v1beta1.CrunchyBrid
 	cluster.Status.IsProtected = c.IsProtected
 	cluster.Status.MajorVersion = c.MajorVersion
 	cluster.Status.Plan = c.Plan
-	cluster.Status.Storage = c.Storage
+	cluster.Status.Storage = FromGibibytes(c.Storage)
 	cluster.Status.Responses.Cluster = c.ResponsePayload
 }
 

internal/bridge/crunchybridgecluster/crunchybridgecluster_controller.go

@@ -24,7 +24,6 @@ import (
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/equality"
 	"k8s.io/apimachinery/pkg/api/meta"
-	"k8s.io/apimachinery/pkg/api/resource"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/util/intstr"
@@ -274,14 +273,6 @@ func (r *CrunchyBridgeClusterReconciler) Reconcile(ctx context.Context, req ctrl
 			v1beta1.ConditionCreating)
 	}
 
-	storageVal, err := handleStorage(crunchybridgecluster.Spec.Storage)
-	if err != nil {
-		log.Error(err, "issue handling storage value")
-		// TODO(crunchybridgecluster)
-		// lint:ignore nilerr no requeue needed
-		return ctrl.Result{}, nil
-	}
-
 	// We should only be missing the ID if no create has been issued
 	// or the create was interrupted and we haven't received the ID.
 	if crunchybridgecluster.Status.ID == "" {
@@ -336,7 +327,7 @@ func (r *CrunchyBridgeClusterReconciler) Reconcile(ctx context.Context, req ctrl
 			PostgresVersion: intstr.FromInt(crunchybridgecluster.Spec.PostgresVersion),
 			Provider:        crunchybridgecluster.Spec.Provider,
 			Region:          crunchybridgecluster.Spec.Region,
-			Storage:         storageVal,
+			Storage:         bridge.ToGibibytes(crunchybridgecluster.Spec.Storage),
 			Team:            team,
 		}
 		cluster, err := r.NewClient().CreateCluster(ctx, key, createClusterRequestPayload)
@@ -415,10 +406,10 @@ func (r *CrunchyBridgeClusterReconciler) Reconcile(ctx context.Context, req ctrl
 	// Check if there's an upgrade difference for the three upgradeable fields that hit the upgrade endpoint
 	// Why PostgresVersion and MajorVersion? Because MajorVersion in the Status is sure to be
 	// an int of the major version, whereas Status.Responses.Cluster.PostgresVersion might be the ID
-	if (storageVal != crunchybridgecluster.Status.Storage) ||
+	if (crunchybridgecluster.Spec.Storage != *crunchybridgecluster.Status.Storage) ||
 		crunchybridgecluster.Spec.Plan != crunchybridgecluster.Status.Plan ||
 		crunchybridgecluster.Spec.PostgresVersion != crunchybridgecluster.Status.MajorVersion {
-		return r.handleUpgrade(ctx, key, crunchybridgecluster, storageVal)
+		return r.handleUpgrade(ctx, key, crunchybridgecluster)
 	}
 
 	// Are there diffs between the cluster response from the Bridge API and the spec?
@@ -467,23 +458,10 @@ func (r *CrunchyBridgeClusterReconciler) reconcileBridgeConnectionSecret(
 	return key, team, err
 }
 
-// handleStorage returns a usable int in G (rounded up if the original storage was in Gi).
-// Returns an error if the int is outside the range for Bridge min (10) or max (65535).
-func handleStorage(storageSpec resource.Quantity) (int64, error) {
-	scaledValue := storageSpec.ScaledValue(resource.Giga)
-
-	if scaledValue < 10 || scaledValue > 65535 {
-		return 0, fmt.Errorf("storage value must be between 10 and 65535")
-	}
-
-	return scaledValue, nil
-}
-
 // handleUpgrade handles upgrades that hit the "POST /clusters/<id>/upgrade" endpoint
 func (r *CrunchyBridgeClusterReconciler) handleUpgrade(ctx context.Context,
 	apiKey string,
 	crunchybridgecluster *v1beta1.CrunchyBridgeCluster,
-	storageVal int64,
 ) (ctrl.Result, error) {
 	log := ctrl.LoggerFrom(ctx)
 
@@ -492,7 +470,7 @@ func (r *CrunchyBridgeClusterReconciler) handleUpgrade(ctx context.Context,
 	upgradeRequest := &bridge.PostClustersUpgradeRequestPayload{
 		Plan:            crunchybridgecluster.Spec.Plan,
 		PostgresVersion: intstr.FromInt(crunchybridgecluster.Spec.PostgresVersion),
-		Storage:         storageVal,
+		Storage:         bridge.ToGibibytes(crunchybridgecluster.Spec.Storage),
 	}
 
 	clusterUpgrade, err := r.NewClient().UpgradeCluster(ctx, apiKey,

internal/bridge/quantity.go

@@ -0,0 +1,53 @@
+/*
+ Copyright 2024 Crunchy Data Solutions, Inc.
+ Licensed under the Apache License, Version 2.0 (the "License");
+ you may not use this file except in compliance with the License.
+ You may obtain a copy of the License at
+ http://www.apache.org/licenses/LICENSE-2.0
+ Unless required by applicable law or agreed to in writing, software
+ distributed under the License is distributed on an "AS IS" BASIS,
+ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ See the License for the specific language governing permissions and
+ limitations under the License.
+*/
+
+package bridge
+
+import (
+	"fmt"
+
+	"k8s.io/apimachinery/pkg/api/resource"
+)
+
+func FromCPU(n int64) *resource.Quantity {
+	// Assume the Bridge API returns numbers that can be parsed by the
+	// [resource] package.
+	if q, err := resource.ParseQuantity(fmt.Sprint(n)); err == nil {
+		return &q
+	}
+
+	return resource.NewQuantity(0, resource.DecimalSI)
+}
+
+// FromGibibytes returns n gibibytes as a [resource.Quantity].
+func FromGibibytes(n int64) *resource.Quantity {
+	// Assume the Bridge API returns numbers that can be parsed by the
+	// [resource] package.
+	if q, err := resource.ParseQuantity(fmt.Sprint(n) + "Gi"); err == nil {
+		return &q
+	}
+
+	return resource.NewQuantity(0, resource.BinarySI)
+}
+
+// ToGibibytes returns q rounded up to a non-negative gibibyte.
+func ToGibibytes(q resource.Quantity) int64 {
+	v := q.Value()
+
+	if v <= 0 {
+		return 0
+	}
+
+	// https://stackoverflow.com/a/2745086
+	return 1 + ((v - 1) >> 30)
+}

internal/bridge/quantity_test.go

@@ -0,0 +1,68 @@
+/*
+ Copyright 2024 Crunchy Data Solutions, Inc.
+ Licensed under the Apache License, Version 2.0 (the "License");
+ you may not use this file except in compliance with the License.
+ You may obtain a copy of the License at
+ http://www.apache.org/licenses/LICENSE-2.0
+ Unless required by applicable law or agreed to in writing, software
+ distributed under the License is distributed on an "AS IS" BASIS,
+ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ See the License for the specific language governing permissions and
+ limitations under the License.
+*/
+
+package bridge
+
+import (
+	"testing"
+
+	"gotest.tools/v3/assert"
+	"k8s.io/apimachinery/pkg/api/resource"
+)
+
+func TestFromCPU(t *testing.T) {
+	zero := FromCPU(0)
+	assert.Assert(t, zero.IsZero())
+	assert.Equal(t, zero.String(), "0")
+
+	one := FromCPU(1)
+	assert.Equal(t, one.String(), "1")
+
+	negative := FromCPU(-2)
+	assert.Equal(t, negative.String(), "-2")
+}
+
+func TestFromGibibytes(t *testing.T) {
+	zero := FromGibibytes(0)
+	assert.Assert(t, zero.IsZero())
+	assert.Equal(t, zero.String(), "0")
+
+	one := FromGibibytes(1)
+	assert.Equal(t, one.String(), "1Gi")
+
+	negative := FromGibibytes(-2)
+	assert.Equal(t, negative.String(), "-2Gi")
+}
+
+func TestToGibibytes(t *testing.T) {
+	zero := resource.MustParse("0")
+	assert.Equal(t, ToGibibytes(zero), int64(0))
+
+	// Negative quantities become zero.
+	negative := resource.MustParse("-4G")
+	assert.Equal(t, ToGibibytes(negative), int64(0))
+
+	// Decimal quantities round up.
+	decimal := resource.MustParse("9000M")
+	assert.Equal(t, ToGibibytes(decimal), int64(9))
+
+	// Binary quantities round up.
+	binary := resource.MustParse("8000Mi")
+	assert.Equal(t, ToGibibytes(binary), int64(8))
+
+	fourGi := resource.MustParse("4096Mi")
+	assert.Equal(t, ToGibibytes(fourGi), int64(4))
+
+	moreThanFourGi := resource.MustParse("4097Mi")
+	assert.Equal(t, ToGibibytes(moreThanFourGi), int64(5))
+}

pkg/apis/postgres-operator.crunchydata.com/v1beta1/crunchy_bridgecluster_types.go

@@ -158,9 +158,9 @@ type CrunchyBridgeClusterStatus struct {
 	// +optional
 	State string `json:"state,omitempty"`
 
-	// The amount of storage available to the cluster in gigabytes.
+	// The amount of storage available to the cluster.
 	// +optional
-	Storage int64 `json:"storage"`
+	Storage *resource.Quantity `json:"storage"`
 }
 
 type APIResponses struct {