Merge pull request #2823 from jasonvigil/fix-sqlinstance-mappings

Tidy up SQLInstance mappings and ref resolving

pkg/controller/direct/sql/sqlinstance_controller.go

@@ -77,7 +77,6 @@ type sqlInstanceAdapter struct {
 	resourceID string
 
 	desired *krm.SQLInstance
-	refs    *SQLInstanceInternalRefs
 	actual  *api.DatabaseInstance
 
 	sqlOperationsClient *api.OperationsService
@@ -109,16 +108,14 @@ func (m *sqlInstanceModel) AdapterForObject(ctx context.Context, kube client.Rea
 		return nil, fmt.Errorf("building gcp client: %w", err)
 	}
 
-	refs, err := NormalizeSQLInstance(ctx, kube, obj)
-	if err != nil {
+	if err := NormalizeSQLInstance(ctx, kube, obj); err != nil {
 		return nil, err
 	}
 
 	return &sqlInstanceAdapter{
 		projectID:           projectID,
 		resourceID:          resourceID,
 		desired:             obj.DeepCopy(),
-		refs:                refs,
 		sqlOperationsClient: gcpClient.sqlOperationsClient(),
 		sqlInstancesClient:  gcpClient.sqlInstancesClient(),
 		sqlUsersClient:      gcpClient.sqlUsersClient(),
@@ -169,12 +166,13 @@ func (a *sqlInstanceAdapter) Create(ctx context.Context, createOp *directbase.Cr
 }
 
 func (a *sqlInstanceAdapter) cloneInstance(ctx context.Context, u *unstructured.Unstructured, log klog.Logger) error {
-	desiredGCP, err := SQLInstanceKRMToGCPCloneRequest(a.desired, a.refs)
+	desiredGCP, err := SQLInstanceCloneKRMToGCP(a.desired)
 	if err != nil {
 		return err
 	}
 
-	op, err := a.sqlInstancesClient.Clone(a.projectID, a.refs.sourceSQLInstance, desiredGCP).Context(ctx).Do()
+	sourceInstance := a.desired.Spec.CloneSource.SQLInstanceRef.External
+	op, err := a.sqlInstancesClient.Clone(a.projectID, sourceInstance, desiredGCP).Context(ctx).Do()
 	if err != nil {
 		return fmt.Errorf("cloning SQLInstance %s failed: %w", a.desired.Name, err)
 	}
@@ -206,15 +204,15 @@ func (a *sqlInstanceAdapter) cloneInstance(ctx context.Context, u *unstructured.
 
 	log.V(2).Info("instance cloned", "op", op, "instance", created)
 
-	status := &krm.SQLInstanceStatus{}
-	if err := Convert_SQLInstance_API_v1_To_KRM_status(created, status); err != nil {
+	status, err := SQLInstanceStatusGCPToKRM(created)
+	if err != nil {
 		return fmt.Errorf("updating SQLInstance status failed: %w", err)
 	}
 	return setStatus(u, status)
 }
 
 func (a *sqlInstanceAdapter) insertInstance(ctx context.Context, u *unstructured.Unstructured, log klog.Logger) error {
-	desiredGCP, err := SQLInstanceKRMToGCP(a.desired, a.refs)
+	desiredGCP, err := SQLInstanceKRMToGCP(a.desired)
 	if err != nil {
 		return err
 	}
@@ -283,8 +281,8 @@ func (a *sqlInstanceAdapter) insertInstance(ctx context.Context, u *unstructured
 
 	log.V(2).Info("instance created", "op", op, "instance", created)
 
-	status := &krm.SQLInstanceStatus{}
-	if err := Convert_SQLInstance_API_v1_To_KRM_status(created, status); err != nil {
+	status, err := SQLInstanceStatusGCPToKRM(created)
+	if err != nil {
 		return fmt.Errorf("updating SQLInstance status failed: %w", err)
 	}
 	return setStatus(u, status)
@@ -383,15 +381,18 @@ func (a *sqlInstanceAdapter) Update(ctx context.Context, updateOp *directbase.Up
 	}
 
 	// Finally, update rest of the fields
-	merged, diffDetected, err := MergeDesiredSQLInstanceWithActual(a.desired, a.refs, a.actual)
+	desiredGCP, err := SQLInstanceKRMToGCP(a.desired)
 	if err != nil {
-		return fmt.Errorf("diffing SQLInstances failed: %w", err)
+		return err
 	}
 
-	if diffDetected {
-		op, err := a.sqlInstancesClient.Update(a.projectID, merged.Name, merged).Context(ctx).Do()
+	if !InstancesMatch(desiredGCP, a.actual) {
+		// GCP API requires we set the current settings version, otherwise update will fail.
+		desiredGCP.Settings.SettingsVersion = a.actual.Settings.SettingsVersion
+
+		op, err := a.sqlInstancesClient.Update(a.projectID, desiredGCP.Name, desiredGCP).Context(ctx).Do()
 		if err != nil {
-			return fmt.Errorf("updating SQLInstance %s failed: %w", merged.Name, err)
+			return fmt.Errorf("updating SQLInstance %s failed: %w", desiredGCP.Name, err)
 		}
 
 		pollingBackoff := gax.Backoff{
@@ -406,23 +407,23 @@ func (a *sqlInstanceAdapter) Update(ctx context.Context, updateOp *directbase.Up
 				break
 			}
 			if err := gax.Sleep(ctx, pollingBackoff.Pause()); err != nil {
-				return fmt.Errorf("waiting for SQLInstance %s update failed: %w", merged.Name, err)
+				return fmt.Errorf("waiting for SQLInstance %s update failed: %w", desiredGCP.Name, err)
 			}
 			op, err = a.sqlOperationsClient.Get(a.projectID, op.Name).Do()
 			if err != nil {
-				return fmt.Errorf("getting SQLInstance %s update operation %s failed: %w", merged.Name, op.Name, err)
+				return fmt.Errorf("getting SQLInstance %s update operation %s failed: %w", desiredGCP.Name, op.Name, err)
 			}
 		}
 
 		updated, err := a.sqlInstancesClient.Get(a.projectID, a.resourceID).Context(ctx).Do()
 		if err != nil {
-			return fmt.Errorf("getting SQLInstance %s failed: %w", merged.Name, err)
+			return fmt.Errorf("getting SQLInstance %s failed: %w", desiredGCP.Name, err)
 		}
 
 		log.V(2).Info("instance updated", "op", op, "instance", updated)
 
-		status := &krm.SQLInstanceStatus{}
-		if err := Convert_SQLInstance_API_v1_To_KRM_status(updated, status); err != nil {
+		status, err := SQLInstanceStatusGCPToKRM(updated)
+		if err != nil {
 			return fmt.Errorf("updating SQLInstance status failed: %w", err)
 		}
 		return setStatus(u, status)