Skip to content

Commit

Permalink
Merge pull request #679 from MStokluska/MGDAPI-5459
Browse files Browse the repository at this point in the history 
MGDAPI-5459 remove maintenance window
  • Loading branch information
@openshift-merge-robot
openshift-merge-robot authored Apr 17, 2023
2 parents 3eca0a2 + 0cf5355 commit ab0a0bf
Show file tree
Hide file tree
Showing 4 changed files with 54 additions and 279 deletions.
50 changes: 21 additions & 29 deletions pkg/providers/gcp/provider_postgres.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"context"
"encoding/json"
"fmt"
"reflect"
"time"

metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
Expand Down Expand Up @@ -104,12 +105,6 @@ func (p *PostgresProvider) ReconcilePostgres(ctx context.Context, pg *v1alpha1.P
return nil, croType.StatusMessage(errMsg), fmt.Errorf("%s: %w", errMsg, err)
}

maintenanceWindowEnabled, err := resources.VerifyPostgresMaintenanceWindow(ctx, p.Client, pg.Namespace, pg.Name)
if err != nil {
errMsg := "failed to verify if postgres updates are allowed"
return nil, croType.StatusMessage(errMsg), errorUtil.Wrap(err, errMsg)
}

sqlClient, err := gcpiface.NewSQLAdminService(ctx, option.WithCredentialsJSON(creds.ServiceAccountJson), p.Logger)
if err != nil {
errMsg := "could not initialise new SQL Admin Service"
Expand All @@ -136,10 +131,10 @@ func (p *PostgresProvider) ReconcilePostgres(ctx context.Context, pg *v1alpha1.P
return nil, msg, err
}

return p.reconcileCloudSQLInstance(ctx, pg, sqlClient, strategyConfig, address, maintenanceWindowEnabled)
return p.reconcileCloudSQLInstance(ctx, pg, sqlClient, strategyConfig, address)
}

func (p *PostgresProvider) reconcileCloudSQLInstance(ctx context.Context, pg *v1alpha1.Postgres, sqladminService gcpiface.SQLAdminService, strategyConfig *StrategyConfig, address *computepb.Address, maintenanceWindowEnabled bool) (*providers.PostgresInstance, croType.StatusMessage, error) {
func (p *PostgresProvider) reconcileCloudSQLInstance(ctx context.Context, pg *v1alpha1.Postgres, sqladminService gcpiface.SQLAdminService, strategyConfig *StrategyConfig, address *computepb.Address) (*providers.PostgresInstance, croType.StatusMessage, error) {
logger := p.Logger.WithField("action", "reconcileCloudSQLInstance")
logger.Infof("reconciling cloudSQL instance")

Expand Down Expand Up @@ -203,27 +198,18 @@ func (p *PostgresProvider) reconcileCloudSQLInstance(ctx context.Context, pg *v1
return nil, croType.StatusMessage(msg), nil
}

if maintenanceWindowEnabled {
logger.Infof("building cloudSQL update config for: %s", foundInstance.Name)
modifiedInstance, err := p.buildCloudSQLUpdateStrategy(gcpInstanceConfig, foundInstance)
if err != nil {
msg := "error building update config for cloudsql instance"
return nil, croType.StatusMessage(msg), errorUtil.Wrap(err, msg)
}
if modifiedInstance != nil {
logger.Infof("modifying cloudSQL instance: %s", foundInstance.Name)
_, err := sqladminService.ModifyInstance(ctx, strategyConfig.ProjectID, foundInstance.Name, modifiedInstance)
if err != nil && !resources.IsConflictError(err) {
msg := fmt.Sprintf("failed to modify cloudsql instance: %s", foundInstance.Name)
return nil, croType.StatusMessage(msg), errorUtil.Wrap(err, msg)
}
}
_, err = controllerutil.CreateOrUpdate(ctx, p.Client, pg, func() error {
pg.Spec.MaintenanceWindow = false
return nil
})
if err != nil {
msg := "failed to set postgres maintenance window to false"
logger.Infof("building cloudSQL update config for: %s", foundInstance.Name)
modifiedInstance, err := p.buildCloudSQLUpdateStrategy(gcpInstanceConfig, foundInstance)
if err != nil {
msg := "error building update config for cloudsql instance"
return nil, croType.StatusMessage(msg), errorUtil.Wrap(err, msg)
}

if modifiedInstance != nil {
logger.Infof("modifying cloudSQL instance: %s", foundInstance.Name)
_, err := sqladminService.ModifyInstance(ctx, strategyConfig.ProjectID, foundInstance.Name, modifiedInstance)
if err != nil && !resources.IsConflictError(err) {
msg := fmt.Sprintf("failed to modify cloudsql instance: %s", foundInstance.Name)
return nil, croType.StatusMessage(msg), errorUtil.Wrap(err, msg)
}
}
Expand Down Expand Up @@ -565,6 +551,12 @@ func (p *PostgresProvider) buildCloudSQLUpdateStrategy(cloudSQLConfig *gcpiface.
modifiedInstance.Settings.DataDiskSizeGb = cloudSQLConfig.Settings.DataDiskSizeGb
updateFound = true
}

userLabelsMatch := reflect.DeepEqual(cloudSQLConfig.Settings.UserLabels, foundInstance.Settings.UserLabels)
if !userLabelsMatch {
modifiedInstance.Settings.UserLabels = cloudSQLConfig.Settings.UserLabels
updateFound = true
}
}

if cloudSQLConfig.Settings.BackupConfiguration != nil && foundInstance.Settings.BackupConfiguration != nil {
Expand Down
115 changes: 18 additions & 97 deletions pkg/providers/gcp/provider_postgres_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1065,11 +1065,10 @@ func TestPostgresProvider_reconcileCloudSQLInstance(t *testing.T) {
ConfigManager ConfigManager
}
type args struct {
p *v1alpha1.Postgres
sqladminService gcpiface.SQLAdminService
strategyConfig *StrategyConfig
address *computepb.Address
maintenanceWindow bool
p *v1alpha1.Postgres
sqladminService gcpiface.SQLAdminService
strategyConfig *StrategyConfig
address *computepb.Address
}
tests := []struct {
name string
Expand Down Expand Up @@ -1097,8 +1096,7 @@ func TestPostgresProvider_reconcileCloudSQLInstance(t *testing.T) {
ProjectID: gcpTestProjectId,
CreateStrategy: json.RawMessage(`{"instance":{"name":"gcptestclustertestNsgcpcloudsql","settings":{"backupConfiguration":{"backupRetentionSettings":{}}}}}`),
},
address: buildValidGcpAddressRange(gcpTestIpRangeName),
maintenanceWindow: false,
address: buildValidGcpAddressRange(gcpTestIpRangeName),
},
want: "cannot retrieve sql instance from gcp",
wantErr: true,
Expand Down Expand Up @@ -1132,8 +1130,7 @@ func TestPostgresProvider_reconcileCloudSQLInstance(t *testing.T) {
ProjectID: "sample-project-id",
CreateStrategy: json.RawMessage(`{"instance":{"settings":{"backupConfiguration":{"backupRetentionSettings":{}}}}}`),
},
address: buildValidGcpAddressRange(gcpTestIpRangeName),
maintenanceWindow: false,
address: buildValidGcpAddressRange(gcpTestIpRangeName),
},
want: "started cloudSQL provision",
wantErr: false,
Expand Down Expand Up @@ -1167,8 +1164,7 @@ func TestPostgresProvider_reconcileCloudSQLInstance(t *testing.T) {
ProjectID: "sample-project-id",
CreateStrategy: json.RawMessage(`{"instance":{}}`),
},
address: buildValidGcpAddressRange(gcpTestIpRangeName),
maintenanceWindow: false,
address: buildValidGcpAddressRange(gcpTestIpRangeName),
},
want: "started cloudSQL provision",
wantErr: false,
Expand Down Expand Up @@ -1202,8 +1198,7 @@ func TestPostgresProvider_reconcileCloudSQLInstance(t *testing.T) {
ProjectID: "sample-project-id",
CreateStrategy: json.RawMessage(`{"instance":{"settings":{}}}`),
},
address: buildValidGcpAddressRange(gcpTestIpRangeName),
maintenanceWindow: false,
address: buildValidGcpAddressRange(gcpTestIpRangeName),
},
want: "started cloudSQL provision",
wantErr: false,
Expand Down Expand Up @@ -1242,8 +1237,7 @@ func TestPostgresProvider_reconcileCloudSQLInstance(t *testing.T) {
ProjectID: "sample-project-id",
CreateStrategy: json.RawMessage(`{"instance":{"name":"gcptestclustertestNsgcpcloudsql"}}`),
},
address: buildValidGcpAddressRange(gcpTestIpRangeName),
maintenanceWindow: false,
address: buildValidGcpAddressRange(gcpTestIpRangeName),
},
want: "started cloudSQL provision",
wantErr: false,
Expand Down Expand Up @@ -1279,8 +1273,7 @@ func TestPostgresProvider_reconcileCloudSQLInstance(t *testing.T) {
ProjectID: "sample-project-id",
CreateStrategy: json.RawMessage(`{"instance":{"name":"gcptestclustertestNsgcpcloudsql"}}`),
},
address: buildValidGcpAddressRange(gcpTestIpRangeName),
maintenanceWindow: false,
address: buildValidGcpAddressRange(gcpTestIpRangeName),
},
want: "creation of " + gcpTestPostgresInstanceName + " cloudSQL instance in progress",
wantErr: false,
Expand Down Expand Up @@ -1312,8 +1305,7 @@ func TestPostgresProvider_reconcileCloudSQLInstance(t *testing.T) {
ProjectID: "sample-project-id",
CreateStrategy: json.RawMessage(`{"instance":{"name":"gcptestclustertestNsgcpcloudsql"}}`),
},
address: buildValidGcpAddressRange(gcpTestIpRangeName),
maintenanceWindow: false,
address: buildValidGcpAddressRange(gcpTestIpRangeName),
},
want: "failed to create cloudSQL instance",
wantErr: true,
Expand Down Expand Up @@ -1358,8 +1350,7 @@ func TestPostgresProvider_reconcileCloudSQLInstance(t *testing.T) {
ProjectID: "sample-project-id",
CreateStrategy: json.RawMessage(`{"instance":{}}`),
},
address: buildValidGcpAddressRange(gcpTestIpRangeName),
maintenanceWindow: false,
address: buildValidGcpAddressRange(gcpTestIpRangeName),
},
want: "failed to add annotation",
wantErr: true,
Expand Down Expand Up @@ -1429,8 +1420,7 @@ func TestPostgresProvider_reconcileCloudSQLInstance(t *testing.T) {
ProjectID: "sample-project-id",
CreateStrategy: json.RawMessage(`{"instance":{}}`),
},
address: buildValidGcpAddressRange(gcpTestIpRangeName),
maintenanceWindow: false,
address: buildValidGcpAddressRange(gcpTestIpRangeName),
},
want: "failed to add annotation to postgres cr",
wantErr: true,
Expand Down Expand Up @@ -1477,8 +1467,7 @@ func TestPostgresProvider_reconcileCloudSQLInstance(t *testing.T) {
ProjectID: "sample-project-id",
CreateStrategy: json.RawMessage(`{"instance":{"settings":{"backupConfiguration":{"backupRetentionSettings":{}}}}}`),
},
address: buildValidGcpAddressRange(gcpTestIpRangeName),
maintenanceWindow: true,
address: buildValidGcpAddressRange(gcpTestIpRangeName),
},
want: "failed to modify cloudsql instance: " + gcpTestPostgresInstanceName,
wantErr: true,
Expand Down Expand Up @@ -1530,6 +1519,7 @@ func TestPostgresProvider_reconcileCloudSQLInstance(t *testing.T) {
Day: 1,
Hour: 10,
},
UserLabels: map[string]string{},
},
}, nil
}
Expand All @@ -1539,82 +1529,13 @@ func TestPostgresProvider_reconcileCloudSQLInstance(t *testing.T) {
}),
strategyConfig: &StrategyConfig{
ProjectID: "sample-project-id",
CreateStrategy: json.RawMessage(`{"instance":{"settings":{"deletionProtectionEnabled":false,"storageAutoResize":false,"ipConfiguration":{"ipv4Enabled":true},"maintenanceWindow":{"day": 7, "hour": 0},"backupConfiguration":{"enabled":false,"pointInTimeRecoveryEnabled":false,"backupRetentionSettings":{"retentionUnit":"RETENTION_UNIT_UNSPECIFIED","retainedBackups":20}}}}}`),
CreateStrategy: json.RawMessage(`{"instance":{"settings":{"deletionProtectionEnabled":false,"storageAutoResize":false,"userLabels":{"same-label":"same-value"},"ipConfiguration":{"ipv4Enabled":true},"maintenanceWindow":{"day": 7, "hour": 0},"backupConfiguration":{"enabled":false,"pointInTimeRecoveryEnabled":false,"backupRetentionSettings":{"retentionUnit":"RETENTION_UNIT_UNSPECIFIED","retainedBackups":20}}}}}`),
},
address: buildValidGcpAddressRange(gcpTestIpRangeName),
maintenanceWindow: true,
address: buildValidGcpAddressRange(gcpTestIpRangeName),
},
want: "successfully reconciled cloudsql instance gcptestclustertestNsgcpcloudsql",
wantErr: false,
},
{
name: "error when setting postgres maintenance window to false",
fields: fields{
Client: func() client.Client {
mc := moqClient.NewSigsClientMoqWithScheme(scheme, &corev1.Secret{ObjectMeta: metav1.ObjectMeta{
Name: postgresProviderName + defaultCredSecSuffix,
Namespace: testNs,
},
Data: map[string][]byte{
defaultPostgresUserKey: []byte(testUser),
defaultPostgresPasswordKey: []byte(testPassword),
},
}, &v1alpha1.Postgres{
ObjectMeta: metav1.ObjectMeta{
Name: postgresProviderName,
Namespace: testNs,
Labels: map[string]string{
"productName": "test_product",
},
ResourceVersion: "1000",
},
Spec: types.ResourceTypeSpec{
MaintenanceWindow: true,
},
}, buildTestGcpInfrastructure(nil))
mc.UpdateFunc = func(ctx context.Context, obj client.Object, opts ...client.UpdateOption) error {
return fmt.Errorf("generic error")
}
return mc
}(),
Logger: logrus.NewEntry(logrus.StandardLogger()),
CredentialManager: NewCredentialMinterCredentialManager(nil),
ConfigManager: nil,
},
args: args{
p: buildTestPostgres(),
sqladminService: gcpiface.GetMockSQLClient(func(sqlClient *gcpiface.MockSqlClient) {
sqlClient.GetInstanceFn = func(ctx context.Context, s string, s2 string) (*sqladmin.DatabaseInstance, error) {
return &sqladmin.DatabaseInstance{
Name: gcpTestPostgresInstanceName,
State: "RUNNABLE",
DatabaseVersion: defaultGCPCLoudSQLDatabaseVersion,
IpAddresses: []*sqladmin.IpMapping{
{
IpAddress: "",
},
},
Settings: &sqladmin.Settings{
BackupConfiguration: &sqladmin.BackupConfiguration{
BackupRetentionSettings: &sqladmin.BackupRetentionSettings{
RetentionUnit: defaultBackupRetentionSettingsRetentionUnit,
RetainedBackups: defaultBackupRetentionSettingsRetainedBackups,
},
},
},
}, nil
}
}),
strategyConfig: &StrategyConfig{
ProjectID: "sample-project-id",
CreateStrategy: json.RawMessage(`{"instance":{"settings":{"backupConfiguration":{"backupRetentionSettings":{}}}}}`),
},
address: buildValidGcpAddressRange(gcpTestIpRangeName),
maintenanceWindow: true,
},
want: "failed to set postgres maintenance window to false",
wantErr: true,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
Expand All @@ -1625,7 +1546,7 @@ func TestPostgresProvider_reconcileCloudSQLInstance(t *testing.T) {
ConfigManager: tt.fields.ConfigManager,
TCPPinger: resources.BuildMockConnectionTester(),
}
_, got1, err := pp.reconcileCloudSQLInstance(context.TODO(), tt.args.p, tt.args.sqladminService, tt.args.strategyConfig, tt.args.address, tt.args.maintenanceWindow)
_, got1, err := pp.reconcileCloudSQLInstance(context.TODO(), tt.args.p, tt.args.sqladminService, tt.args.strategyConfig, tt.args.address)
if (err != nil) != tt.wantErr {
t.Errorf("reconcileCloudSQLInstance() error = %v, wantErr %v", err, tt.wantErr)
return
Expand Down
35 changes: 11 additions & 24 deletions pkg/providers/gcp/provider_redis.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -160,35 +160,22 @@ func (p *RedisProvider) createRedisInstance(ctx context.Context, networkManager
statusMessage := fmt.Sprintf("gcp redis instance %s is not ready yet, current state is %s", createInstanceRequest.Instance.Name, foundInstance.State.String())
return nil, croType.StatusMessage(statusMessage), nil
}
maintenanceWindowEnabled, err := resources.VerifyRedisMaintenanceWindow(ctx, p.Client, r.Namespace, r.Name)
if err != nil {
statusMessage := "failed to verify if redis updates are allowed"
return nil, croType.StatusMessage(statusMessage), errorUtil.Wrap(err, statusMessage)
}
if maintenanceWindowEnabled {
if updateInstanceRequest := p.buildUpdateInstanceRequest(createInstanceRequest.Instance, foundInstance); updateInstanceRequest != nil {
_, err = redisClient.UpdateInstance(ctx, updateInstanceRequest)
if err != nil {
statusMessage := fmt.Sprintf("failed to update gcp redis instance %s", createInstanceRequest.Instance.Name)
return nil, croType.StatusMessage(statusMessage), errorUtil.Wrap(err, statusMessage)
}
}
if upgradeInstanceRequest := p.buildUpgradeInstanceRequest(createInstanceRequest.Instance, foundInstance); upgradeInstanceRequest != nil {
_, err = redisClient.UpgradeInstance(ctx, upgradeInstanceRequest)
if err != nil {
statusMessage := fmt.Sprintf("failed to upgrade gcp redis instance %s", createInstanceRequest.Instance.Name)
return nil, croType.StatusMessage(statusMessage), errorUtil.Wrap(err, statusMessage)
}

if updateInstanceRequest := p.buildUpdateInstanceRequest(createInstanceRequest.Instance, foundInstance); updateInstanceRequest != nil {
_, err = redisClient.UpdateInstance(ctx, updateInstanceRequest)
if err != nil {
statusMessage := fmt.Sprintf("failed to update gcp redis instance %s", createInstanceRequest.Instance.Name)
return nil, croType.StatusMessage(statusMessage), errorUtil.Wrap(err, statusMessage)
}
_, err = controllerutil.CreateOrUpdate(ctx, p.Client, r, func() error {
r.Spec.MaintenanceWindow = false
return nil
})
}
if upgradeInstanceRequest := p.buildUpgradeInstanceRequest(createInstanceRequest.Instance, foundInstance); upgradeInstanceRequest != nil {
_, err = redisClient.UpgradeInstance(ctx, upgradeInstanceRequest)
if err != nil {
statusMessage := "failed to set redis maintenance window to false"
statusMessage := fmt.Sprintf("failed to upgrade gcp redis instance %s", createInstanceRequest.Instance.Name)
return nil, croType.StatusMessage(statusMessage), errorUtil.Wrap(err, statusMessage)
}
}

rdd := &providers.RedisDeploymentDetails{
URI: foundInstance.Host,
Port: int64(foundInstance.Port),
Expand Down
Loading

0 comments on commit ab0a0bf

Please sign in to comment.