Skip to content

Commit 01a66db

Browse files
author
Charel Baum (external expert on behalf of DB InfraGO AG)
committed
feat(docdb): allow (major) version upgrades + observe engineVersion
Signed-off-by: Charel Baum (external expert on behalf of DB InfraGO AG) <charel.baum-extern@deutschebahn.com>
1 parent 5eb25dc commit 01a66db

10 files changed

Lines changed: 656 additions & 7 deletions

File tree

‎apis/docdb/generator-config.yaml‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,16 @@
1+
resources:
2+
DBCluster:
3+
fields:
4+
AllowMajorVersionUpgrade:
5+
from:
6+
operation: ModifyDBCluster
7+
path: AllowMajorVersionUpgrade
8+
EngineVersion:
9+
is_read_only: true
10+
from:
11+
operation: DescribeDBClusters
12+
path: DBClusters.EngineVersion
13+
114
ignore:
215
field_paths:
316
- DescribeDBClusterParameterGroupsInput.DBClusterParameterGroupName

‎apis/docdb/v1alpha1/custom_types.go‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,8 @@ const (
3131
DocDBInstanceStateDeleting = "deleting"
3232
// The instance is being modified.
3333
DocDBInstanceStateModifying = "modifying"
34+
// The instance is being upgraded.
35+
DocDBInstanceStateUpgrading = "upgrading"
3436
// The instance has failed and Amazon RDS can't recover it. Perform a point-in-time restore to the latest restorable time of the instance to recover the data.
3537
DocDBInstanceStateFailed = "failed"
3638
)

‎apis/docdb/v1alpha1/zz_db_cluster.go‎

Lines changed: 8 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎apis/docdb/v1alpha1/zz_generated.deepcopy.go‎

Lines changed: 10 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎package/crds/docdb.aws.crossplane.io_dbclusters.yaml‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,15 @@ spec:
7373
forProvider:
7474
description: DBClusterParameters defines the desired state of DBCluster
7575
properties:
76+
allowMajorVersionUpgrade:
77+
description: |-
78+
A value that indicates whether major version upgrades are allowed.
79+
80+
81+
Constraints: You must allow major version upgrades when specifying a value
82+
for the EngineVersion parameter that is a different major version than the
83+
DB cluster's current version.
84+
type: boolean
7685
applyImmediately:
7786
description: |-
7887
A value that specifies whether the changes in this request and any pending
@@ -984,6 +993,9 @@ spec:
984993
description: Specifies the connection endpoint for the primary
985994
instance of the cluster.
986995
type: string
996+
engineVersion:
997+
description: Indicates the database engine version.
998+
type: string
987999
hostedZoneID:
9881000
description: Specifies the ID that Amazon Route 53 assigns when
9891001
you create a hosted zone.

‎pkg/controller/docdb/dbcluster/setup.go‎

Lines changed: 67 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ import (
3131
"github.com/crossplane/crossplane-runtime/pkg/password"
3232
"github.com/crossplane/crossplane-runtime/pkg/reconciler/managed"
3333
"github.com/crossplane/crossplane-runtime/pkg/resource"
34+
cpresource "github.com/crossplane/crossplane-runtime/pkg/resource"
3435
"github.com/pkg/errors"
3536
corev1 "k8s.io/api/core/v1"
3637
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -40,8 +41,10 @@ import (
4041

4142
svcapitypes "github.com/crossplane-contrib/provider-aws/apis/docdb/v1alpha1"
4243
"github.com/crossplane-contrib/provider-aws/apis/v1alpha1"
44+
"github.com/crossplane-contrib/provider-aws/pkg/controller/docdb/utils"
4345
svcutils "github.com/crossplane-contrib/provider-aws/pkg/controller/docdb/utils"
4446
"github.com/crossplane-contrib/provider-aws/pkg/features"
47+
errorutils "github.com/crossplane-contrib/provider-aws/pkg/utils/errors"
4548
"github.com/crossplane-contrib/provider-aws/pkg/utils/pointer"
4649
custommanaged "github.com/crossplane-contrib/provider-aws/pkg/utils/reconciler/managed"
4750
)
@@ -121,6 +124,9 @@ func (e *hooks) postObserve(ctx context.Context, cr *svcapitypes.DBCluster, resp
121124
return managed.ExternalObservation{}, err
122125
}
123126

127+
cluster := resp.DBClusters[0]
128+
cr.Status.AtProvider.EngineVersion = cluster.EngineVersion
129+
124130
obs.ConnectionDetails = getConnectionDetails(cr)
125131

126132
if !meta.WasDeleted(cr) {
@@ -177,6 +183,10 @@ func lateInitialize(cr *svcapitypes.DBClusterParameters, resp *svcsdk.DescribeDB
177183
func (e *hooks) isUpToDate(ctx context.Context, cr *svcapitypes.DBCluster, resp *svcsdk.DescribeDBClustersOutput) (bool, string, error) {
178184
cluster := resp.DBClusters[0]
179185

186+
if pointer.StringValue(cluster.Status) == svcapitypes.DocDBInstanceStateModifying || pointer.StringValue(cluster.Status) == svcapitypes.DocDBInstanceStateUpgrading {
187+
return true, "", nil
188+
}
189+
180190
_, pwChanged, err := e.getPasswordFromRef(ctx, cr.Spec.ForProvider.MasterUserPasswordSecretRef, cr.Spec.WriteConnectionSecretToReference)
181191
if err != nil || pwChanged {
182192
return false, "", err
@@ -187,6 +197,7 @@ func (e *hooks) isUpToDate(ctx context.Context, cr *svcapitypes.DBCluster, resp
187197
pointer.StringValue(cr.Spec.ForProvider.DBClusterParameterGroupName) != pointer.StringValue(cluster.DBClusterParameterGroup),
188198
pointer.BoolValue(cr.Spec.ForProvider.DeletionProtection) != pointer.BoolValue(cluster.DeletionProtection),
189199
!areSameElements(cr.Spec.ForProvider.EnableCloudwatchLogsExports, cluster.EnabledCloudwatchLogsExports),
200+
!isEngineVersionUpToDate(cr, resp),
190201
pointer.Int64Value(cr.Spec.ForProvider.Port) != pointer.Int64Value(cluster.Port),
191202
pointer.StringValue(cr.Spec.ForProvider.PreferredBackupWindow) != pointer.StringValue(cluster.PreferredBackupWindow),
192203
pointer.StringValue(cr.Spec.ForProvider.PreferredMaintenanceWindow) != pointer.StringValue(cluster.PreferredMaintenanceWindow):
@@ -211,14 +222,53 @@ func (e *hooks) preUpdate(ctx context.Context, cr *svcapitypes.DBCluster, obj *s
211222
if pwchanged {
212223
obj.MasterUserPassword = aws.String(pw)
213224
}
225+
226+
// ModifyDBCluster() returns error, when trying to upgrade major (minor is fine) EngineVersion:
227+
// "Cannot change VPC security group while doing a major version upgrade."
228+
// therefore EngineVersion update is entirely done separately in postUpdate
229+
obj.EngineVersion = nil
230+
// In case of a custom DBClusterParameterGroup, AWS requires for a major version update that
231+
// EngineVersion and DBClusterParameterGroupName are in the same ModifyDBCluster()-call
232+
obj.DBClusterParameterGroupName = nil
233+
214234
return nil
215235
}
216236

217-
func (e *hooks) postUpdate(_ context.Context, cr *svcapitypes.DBCluster, resp *svcsdk.ModifyDBClusterOutput, upd managed.ExternalUpdate, err error) (managed.ExternalUpdate, error) {
237+
func (e *hooks) postUpdate(ctx context.Context, cr *svcapitypes.DBCluster, resp *svcsdk.ModifyDBClusterOutput, upd managed.ExternalUpdate, err error) (managed.ExternalUpdate, error) {
218238
if err != nil {
219239
return managed.ExternalUpdate{}, err
220240
}
221241

242+
input := GenerateDescribeDBClustersInput(cr)
243+
// GenerateDescribeDBClustersInput returns an empty DescribeDBClustersInput
244+
// and the function is generated by ack-generate, so we manually need to set the
245+
// DBClusterIdentifier
246+
input.DBClusterIdentifier = pointer.ToOrNilIfZeroValue(meta.GetExternalName(cr))
247+
out, err := e.client.DescribeDBClustersWithContext(ctx, input)
248+
if err != nil {
249+
return managed.ExternalUpdate{}, errorutils.Wrap(cpresource.Ignore(IsNotFound, err), errDescribe)
250+
}
251+
252+
needsEngineVersionUpdate := !isEngineVersionUpToDate(cr, out)
253+
needsDBClusterParamGroupUpdate := pointer.StringValue(cr.Spec.ForProvider.DBClusterParameterGroupName) != pointer.StringValue(out.DBClusters[0].DBClusterParameterGroup)
254+
needsPostUpdate := needsEngineVersionUpdate || needsDBClusterParamGroupUpdate
255+
256+
if needsPostUpdate {
257+
modifyInput := &svcsdk.ModifyDBClusterInput{
258+
DBClusterIdentifier: pointer.ToOrNilIfZeroValue(meta.GetExternalName(cr)),
259+
ApplyImmediately: cr.Spec.ForProvider.ApplyImmediately,
260+
DBClusterParameterGroupName: cr.Spec.ForProvider.DBClusterParameterGroupName,
261+
}
262+
if needsEngineVersionUpdate {
263+
modifyInput.EngineVersion = cr.Spec.ForProvider.EngineVersion
264+
modifyInput.AllowMajorVersionUpgrade = cr.Spec.ForProvider.AllowMajorVersionUpgrade
265+
}
266+
267+
if _, err = e.client.ModifyDBClusterWithContext(ctx, modifyInput); err != nil {
268+
return managed.ExternalUpdate{}, err
269+
}
270+
}
271+
222272
return upd, svcutils.UpdateTagsForResource(e.client, cr.Spec.ForProvider.Tags, resp.DBCluster.DBClusterArn)
223273
}
224274

@@ -442,6 +492,22 @@ func areSameElements(a1, a2 []*string) bool {
442492
return true
443493
}
444494

495+
func isEngineVersionUpToDate(cr *svcapitypes.DBCluster, out *svcsdk.DescribeDBClustersOutput) bool {
496+
// If EngineVersion is not set, AWS sets a default value,
497+
// so we do not try to update in this case
498+
if cr.Spec.ForProvider.EngineVersion != nil {
499+
if out.DBClusters[0].EngineVersion == nil {
500+
return false
501+
}
502+
503+
// Upgrade is only necessary if the spec version is higher.
504+
// Downgrades are not possible in pointer.
505+
c := utils.CompareEngineVersions(*cr.Spec.ForProvider.EngineVersion, *out.DBClusters[0].EngineVersion)
506+
return c <= 0
507+
}
508+
return true
509+
}
510+
445511
func (e *hooks) getPasswordFromRef(ctx context.Context, in *xpv1.SecretKeySelector, out *xpv1.SecretReference) (newPwd string, changed bool, err error) {
446512
if in == nil {
447513
return "", false, nil

0 commit comments

Comments
 (0)