Merge pull request #43 from vshn/pg-update

Implement Updating of PostgreSQL instances

operator/mapper/alias.go

@@ -16,15 +16,24 @@ type BackupSchedule = struct {
 	BackupMinute *int64 `json:"backup-minute,omitempty"`
 }
 
-// MaintenanceSchedule is a type alias for the embedded struct in opai.CreateDbaasServicePgJSONRequestBody.
-type MaintenanceSchedule = struct {
+// MaintenanceScheduleCreateRequest is a type alias for the embedded struct in opai.CreateDbaasServicePgJSONRequestBody.
+type MaintenanceScheduleCreateRequest = struct {
 	// Day of week for installing updates
 	Dow oapi.CreateDbaasServicePgJSONBodyMaintenanceDow `json:"dow"`
 
 	// Time for installing updates, UTC
 	Time string `json:"time"`
 }
 
+// MaintenanceScheduleUpdateRequest is a type alias for the embedded struct in opai.UpdateDbaasServicePgJSONRequestBody.
+type MaintenanceScheduleUpdateRequest = struct {
+	// Day of week for installing updates
+	Dow oapi.UpdateDbaasServicePgJSONBodyMaintenanceDow `json:"dow"`
+
+	// Time for installing updates, UTC
+	Time string `json:"time"`
+}
+
 func toBackupSchedule(day exoscalev1.TimeOfDay) (BackupSchedule, error) {
 	backupHour, backupMin, _, err := day.Parse()
 	return BackupSchedule{
@@ -33,13 +42,20 @@ func toBackupSchedule(day exoscalev1.TimeOfDay) (BackupSchedule, error) {
 	}, err
 }
 
-func toMaintenanceSchedule(spec exoscalev1.MaintenanceSpec) MaintenanceSchedule {
-	return MaintenanceSchedule{
+func toMaintenanceScheduleCreateRequest(spec exoscalev1.MaintenanceSpec) MaintenanceScheduleCreateRequest {
+	return MaintenanceScheduleCreateRequest{
 		Dow:  oapi.CreateDbaasServicePgJSONBodyMaintenanceDow(spec.DayOfWeek),
 		Time: spec.TimeOfDay.String(),
 	}
 }
 
+func toMaintenanceScheduleUpdateRequest(spec exoscalev1.MaintenanceSpec) MaintenanceScheduleUpdateRequest {
+	return MaintenanceScheduleUpdateRequest{
+		Dow:  oapi.UpdateDbaasServicePgJSONBodyMaintenanceDow(spec.DayOfWeek),
+		Time: spec.TimeOfDay.String(),
+	}
+}
+
 func toSlicePtr(arr []string) *[]string {
 	return &arr
 }

operator/mapper/alias_test.go

@@ -64,24 +64,24 @@ func Test_toBackupSchedule(t *testing.T) {
 func Test_toMaintenanceSchedule(t *testing.T) {
 	tests := map[string]struct {
 		givenSpec      exoscalev1.MaintenanceSpec
-		expectedResult MaintenanceSchedule
+		expectedResult MaintenanceScheduleCreateRequest
 	}{
 		"Disabled": {
 			givenSpec:      exoscalev1.MaintenanceSpec{TimeOfDay: "0:00:00", DayOfWeek: "never"},
-			expectedResult: MaintenanceSchedule{Time: "0:00:00", Dow: "never"},
+			expectedResult: MaintenanceScheduleCreateRequest{Time: "0:00:00", Dow: "never"},
 		},
 		"SameWeekDay": {
 			givenSpec:      exoscalev1.MaintenanceSpec{TimeOfDay: "0:00:00", DayOfWeek: "monday"},
-			expectedResult: MaintenanceSchedule{Time: "0:00:00", Dow: "monday"},
+			expectedResult: MaintenanceScheduleCreateRequest{Time: "0:00:00", Dow: "monday"},
 		},
 		"SameTime": {
 			givenSpec:      exoscalev1.MaintenanceSpec{TimeOfDay: "12:34:56", DayOfWeek: "monday"},
-			expectedResult: MaintenanceSchedule{Time: "12:34:56", Dow: "monday"},
+			expectedResult: MaintenanceScheduleCreateRequest{Time: "12:34:56", Dow: "monday"},
 		},
 	}
 	for name, tc := range tests {
 		t.Run(name, func(t *testing.T) {
-			result := toMaintenanceSchedule(tc.givenSpec)
+			result := toMaintenanceScheduleCreateRequest(tc.givenSpec)
 			assert.Equal(t, tc.expectedResult, result)
 		})
 	}

operator/mapper/conditions.go

@@ -0,0 +1,16 @@
+package mapper
+
+import (
+	xpv1 "github.com/crossplane/crossplane-runtime/apis/common/v1"
+)
+
+// FindStatusCondition returns the condition by the given type if present.
+// If not found, it returns nil.
+func FindStatusCondition(c []xpv1.Condition, conditionType xpv1.ConditionType) *xpv1.Condition {
+	for _, condition := range c {
+		if condition.Type == conditionType {
+			return &condition
+		}
+	}
+	return nil
+}

operator/mapper/postgresql.go

@@ -17,10 +17,14 @@ import (
 // PostgreSQLMapper is a mapper to convert exoscale's SDK resources to Kubernetes API specs.
 type PostgreSQLMapper struct{}
 
-// FromSpec places the given spec into the request body.
-func (PostgreSQLMapper) FromSpec(spec exoscalev1.PostgreSQLParameters, into *oapi.CreateDbaasServicePgJSONBody) error {
+// FromSpecToCreateBody places the given spec into the request body.
+func (PostgreSQLMapper) FromSpecToCreateBody(spec exoscalev1.PostgreSQLParameters, into *oapi.CreateDbaasServicePgJSONBody) error {
+	/**
+	NOTE: If you change anything below, also update FromSpecToUpdateBody().
+	Unfortunately the generated openapi-types in exoscale are unusable for reusing same properties.
+	*/
 	backupSchedule, backupErr := toBackupSchedule(spec.Backup.TimeOfDay)
-	maintenanceSchedule := toMaintenanceSchedule(spec.Maintenance)
+	maintenanceSchedule := toMaintenanceScheduleCreateRequest(spec.Maintenance)
 	pgSettings, parseErr := ToMap(spec.PGSettings)
 	if err := firstOf(backupErr, parseErr); err != nil {
 		return err
@@ -37,6 +41,30 @@ func (PostgreSQLMapper) FromSpec(spec exoscalev1.PostgreSQLParameters, into *oap
 	return nil
 }
 
+// FromSpecToUpdateBody places the given spec into the request body.
+func (PostgreSQLMapper) FromSpecToUpdateBody(spec exoscalev1.PostgreSQLParameters, into *oapi.UpdateDbaasServicePgJSONRequestBody) error {
+	/**
+	NOTE: If you change anything below, also update FromSpecToCreateBody().
+	Unfortunately the generated openapi-types in exoscale are unusable for reusing same properties.
+	*/
+	backupSchedule, backupErr := toBackupSchedule(spec.Backup.TimeOfDay)
+	maintenanceSchedule := toMaintenanceScheduleUpdateRequest(spec.Maintenance)
+	pgSettings, parseErr := ToMap(spec.PGSettings)
+	if err := firstOf(backupErr, parseErr); err != nil {
+		return err
+	}
+
+	into.Plan = pointer.String(spec.Size.Plan)
+	into.BackupSchedule = &backupSchedule
+	into.Variant = &variantAiven
+	// into.Version = pointer.String(spec.Version) -> Version cannot be changed
+	into.TerminationProtection = pointer.Bool(spec.TerminationProtection)
+	into.Maintenance = &maintenanceSchedule
+	into.IpFilter = toSlicePtr(spec.IPFilter)
+	into.PgSettings = &pgSettings
+	return nil
+}
+
 // ToStatus fills the status fields from the given response body.
 func (PostgreSQLMapper) ToStatus(pg *oapi.DbaasServicePg, into *exoscalev1.PostgreSQLObservation) error {
 	into.NoteStates = toNodeStates(pg.NodeStates)
@@ -45,6 +73,7 @@ func (PostgreSQLMapper) ToStatus(pg *oapi.DbaasServicePg, into *exoscalev1.Postg
 		DayOfWeek: pg.Maintenance.Dow,
 		TimeOfDay: exoscalev1.TimeOfDay(pg.Maintenance.Time),
 	}
+	into.Size.Plan = pg.Plan
 	into.IPFilter = *pg.IpFilter
 	into.Backup = toBackupSpec(pg.BackupSchedule)
 	parsed, err := ToRawExtension(pg.PgSettings)

operator/postgresqlcontroller/create.go

@@ -21,7 +21,7 @@ func (p *Pipeline) Create(ctx context.Context, mg resource.Managed) (managed.Ext
 	spec := pgInstance.Spec.ForProvider
 	mpr := &mapper.PostgreSQLMapper{}
 	body := oapi.CreateDbaasServicePgJSONBody{}
-	err := mpr.FromSpec(spec, &body)
+	err := mpr.FromSpecToCreateBody(spec, &body)
 	if err != nil {
 		return managed.ExternalCreation{}, errors.Wrap(err, "cannot map spec to API request")
 	}

operator/postgresqlcontroller/update.go

@@ -3,14 +3,32 @@ package postgresqlcontroller
 import (
 	"context"
 
+	"github.com/crossplane/crossplane-runtime/pkg/errors"
 	"github.com/crossplane/crossplane-runtime/pkg/reconciler/managed"
 	"github.com/crossplane/crossplane-runtime/pkg/resource"
+	"github.com/exoscale/egoscale/v2/oapi"
+	"github.com/vshn/provider-exoscale/operator/mapper"
 	controllerruntime "sigs.k8s.io/controller-runtime"
 )
 
 // Update implements managed.ExternalClient.
 func (p *Pipeline) Update(ctx context.Context, mg resource.Managed) (managed.ExternalUpdate, error) {
 	log := controllerruntime.LoggerFrom(ctx)
-	log.V(1).Info("Updating resource (noop)")
+	log.V(1).Info("Updating resource")
+
+	pgInstance := fromManaged(mg)
+
+	spec := pgInstance.Spec.ForProvider
+	mpr := &mapper.PostgreSQLMapper{}
+	body := oapi.UpdateDbaasServicePgJSONRequestBody{}
+	err := mpr.FromSpecToUpdateBody(spec, &body)
+	if err != nil {
+		return managed.ExternalUpdate{}, errors.Wrap(err, "cannot map spec to API request")
+	}
+	resp, err := p.exo.UpdateDbaasServicePgWithResponse(ctx, oapi.DbaasServiceName(pgInstance.Name), body)
+	if err != nil {
+		return managed.ExternalUpdate{}, errors.Wrap(err, "cannot update instance")
+	}
+	log.V(1).Info("Response", "json", resp.JSON200)
 	return managed.ExternalUpdate{}, nil
 }

operator/postgresqlcontroller/webhook.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"fmt"
 
+	xpv1 "github.com/crossplane/crossplane-runtime/apis/common/v1"
 	"github.com/go-logr/logr"
 	exoscalev1 "github.com/vshn/provider-exoscale/apis/exoscale/v1"
 	"github.com/vshn/provider-exoscale/operator/mapper"
@@ -26,10 +27,14 @@ func (v *Validator) ValidateCreate(_ context.Context, obj runtime.Object) error
 // ValidateUpdate implements admission.CustomValidator.
 func (v *Validator) ValidateUpdate(_ context.Context, oldObj, newObj runtime.Object) error {
 	newInstance := newObj.(*exoscalev1.PostgreSQL)
-	//	oldInstance := oldObj.(*exoscalev1.PostgreSQL)
+	oldInstance := oldObj.(*exoscalev1.PostgreSQL)
 	v.log.V(1).Info("Validate update")
 
-	return v.validateSpec(newInstance)
+	err := v.validateSpec(newInstance)
+	if err != nil {
+		return err
+	}
+	return v.compare(oldInstance, newInstance)
 }
 
 // ValidateDelete implements admission.CustomValidator.
@@ -84,3 +89,46 @@ func (v *Validator) validatePGSettings(obj exoscalev1.PostgreSQLParameters) erro
 	}
 	return nil
 }
+
+func (v *Validator) compare(old, new *exoscalev1.PostgreSQL) error {
+	if !v.isCreated(old) {
+		// comparing immutable fields is only necessary after creation.
+		return nil
+	}
+	for _, compareFn := range []func(_, _ *exoscalev1.PostgreSQL) error{
+		v.compareZone,
+		v.compareVersion,
+	} {
+		if err := compareFn(old, new); err != nil {
+			return err
+		}
+	}
+	return nil
+}
+
+func (v *Validator) compareZone(old, new *exoscalev1.PostgreSQL) error {
+	if old.Spec.ForProvider.Zone != new.Spec.ForProvider.Zone {
+		return fmt.Errorf("field is immutable after creation: %s (old), %s (changed)", old.Spec.ForProvider.Zone, new.Spec.ForProvider.Zone)
+	}
+	return nil
+}
+
+func (v *Validator) compareVersion(old, new *exoscalev1.PostgreSQL) error {
+	oldObserved := old.Status.AtProvider.Version
+	oldDesired := old.Spec.ForProvider.Version
+	newDesired := new.Spec.ForProvider.Version
+
+	if oldDesired != newDesired && oldObserved != newDesired {
+		// TODO: Better comparison of versions.
+		// For example, currently the check allows to update "14" -> "15.1" if the observed version is "15.1" , but not "14" -> "15" if observed version is "15.2".
+		// However, "14" -> "15.3" should also not be allowed if observed version is < "15.3".
+		return fmt.Errorf("field is immutable after creation: %s (old), %s (changed)", oldDesired, newDesired)
+	}
+	// we only allow version change if it matches the observed version
+	return nil
+}
+
+func (v *Validator) isCreated(obj *exoscalev1.PostgreSQL) bool {
+	cond := mapper.FindStatusCondition(obj.Status.Conditions, xpv1.Available().Type)
+	return cond != nil
+}

operator/postgresqlcontroller/webhook_test.go

@@ -44,3 +44,52 @@ func TestValidator_validatePGSettings(t *testing.T) {
 		})
 	}
 }
+func TestValidator_compareVersion(t *testing.T) {
+	tests := map[string]struct {
+		oldObservedVersion string
+		oldSpecVersion     string
+		newDesiredVersion  string
+		expectedError      string
+	}{
+		"NoChange_SameVersion": {
+			oldSpecVersion:     "14",
+			newDesiredVersion:  "14",
+			oldObservedVersion: "14.5",
+			expectedError:      "",
+		},
+		"NoChange_DifferentObservedVersion": {
+			oldSpecVersion:     "14",
+			newDesiredVersion:  "14",
+			oldObservedVersion: "15.1",
+			expectedError:      "",
+		},
+		"Change_ToSameObservedVersion": {
+			oldSpecVersion:     "14",
+			newDesiredVersion:  "15.1",
+			oldObservedVersion: "15.1",
+			expectedError:      "",
+		},
+		"Change_ToOtherValue": {
+			oldSpecVersion:     "14",
+			newDesiredVersion:  "15.2",
+			oldObservedVersion: "15.1",
+			expectedError:      "field is immutable after creation: 14 (old), 15.2 (changed)",
+		},
+	}
+	for name, tc := range tests {
+		t.Run(name, func(t *testing.T) {
+			v := Validator{log: logr.Discard()}
+			oldObj := &exoscalev1.PostgreSQL{}
+			newObj := &exoscalev1.PostgreSQL{}
+			oldObj.Status.AtProvider.Version = tc.oldObservedVersion
+			oldObj.Spec.ForProvider.Version = tc.oldSpecVersion
+			newObj.Spec.ForProvider.Version = tc.newDesiredVersion
+			err := v.compareVersion(oldObj, newObj)
+			if tc.expectedError != "" {
+				assert.EqualError(t, err, tc.expectedError)
+			} else {
+				assert.NoError(t, err)
+			}
+		})
+	}
+}