Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(docdb): allow (major) version upgrades #2073

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ GOLANGCILINT_VERSION = 1.54.2

UP_VERSION = v0.27.0
UP_CHANNEL = stable
UPTEST_VERSION = v0.12.0
UPTEST_VERSION = v0.11.0

-include build/makelib/k8s_tools.mk

Expand Down
13 changes: 13 additions & 0 deletions apis/docdb/generator-config.yaml
Original file line number Diff line number Diff line change
@@ -1,3 +1,16 @@
resources:
DBCluster:
fields:
AllowMajorVersionUpgrade:
from:
operation: ModifyDBCluster
path: AllowMajorVersionUpgrade
EngineVersion:
is_read_only: true
from:
operation: DescribeDBClusters
path: DBClusters.EngineVersion

ignore:
field_paths:
- DescribeDBClusterParameterGroupsInput.DBClusterParameterGroupName
Expand Down
2 changes: 2 additions & 0 deletions apis/docdb/v1alpha1/custom_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@ const (
DocDBInstanceStateDeleting = "deleting"
// The instance is being modified.
DocDBInstanceStateModifying = "modifying"
// The instance is being upgraded.
DocDBInstanceStateUpgrading = "upgrading"
// 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.
DocDBInstanceStateFailed = "failed"
)
Expand Down
8 changes: 8 additions & 0 deletions apis/docdb/v1alpha1/zz_db_cluster.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

10 changes: 10 additions & 0 deletions apis/docdb/v1alpha1/zz_generated.deepcopy.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

12 changes: 12 additions & 0 deletions package/crds/docdb.aws.crossplane.io_dbclusters.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,15 @@ spec:
forProvider:
description: DBClusterParameters defines the desired state of DBCluster
properties:
allowMajorVersionUpgrade:
description: |-
A value that indicates whether major version upgrades are allowed.


Constraints: You must allow major version upgrades when specifying a value
for the EngineVersion parameter that is a different major version than the
DB cluster's current version.
type: boolean
applyImmediately:
description: |-
A value that specifies whether the changes in this request and any pending
Expand Down Expand Up @@ -984,6 +993,9 @@ spec:
description: Specifies the connection endpoint for the primary
instance of the cluster.
type: string
engineVersion:
description: Indicates the database engine version.
type: string
hostedZoneID:
description: Specifies the ID that Amazon Route 53 assigns when
you create a hosted zone.
Expand Down
68 changes: 67 additions & 1 deletion pkg/controller/docdb/dbcluster/setup.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ import (
"github.com/crossplane/crossplane-runtime/pkg/password"
"github.com/crossplane/crossplane-runtime/pkg/reconciler/managed"
"github.com/crossplane/crossplane-runtime/pkg/resource"
cpresource "github.com/crossplane/crossplane-runtime/pkg/resource"
"github.com/pkg/errors"
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
Expand All @@ -40,8 +41,10 @@ import (

svcapitypes "github.com/crossplane-contrib/provider-aws/apis/docdb/v1alpha1"
"github.com/crossplane-contrib/provider-aws/apis/v1alpha1"
"github.com/crossplane-contrib/provider-aws/pkg/controller/docdb/utils"
svcutils "github.com/crossplane-contrib/provider-aws/pkg/controller/docdb/utils"
"github.com/crossplane-contrib/provider-aws/pkg/features"
errorutils "github.com/crossplane-contrib/provider-aws/pkg/utils/errors"
"github.com/crossplane-contrib/provider-aws/pkg/utils/pointer"
custommanaged "github.com/crossplane-contrib/provider-aws/pkg/utils/reconciler/managed"
)
Expand Down Expand Up @@ -121,6 +124,9 @@ func (e *hooks) postObserve(ctx context.Context, cr *svcapitypes.DBCluster, resp
return managed.ExternalObservation{}, err
}

cluster := resp.DBClusters[0]
cr.Status.AtProvider.EngineVersion = cluster.EngineVersion

obs.ConnectionDetails = getConnectionDetails(cr)

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

if pointer.StringValue(cluster.Status) == svcapitypes.DocDBInstanceStateModifying || pointer.StringValue(cluster.Status) == svcapitypes.DocDBInstanceStateUpgrading {
return true, "", nil
}

_, pwChanged, err := e.getPasswordFromRef(ctx, cr.Spec.ForProvider.MasterUserPasswordSecretRef, cr.Spec.WriteConnectionSecretToReference)
if err != nil || pwChanged {
return false, "", err
Expand All @@ -187,6 +197,7 @@ func (e *hooks) isUpToDate(ctx context.Context, cr *svcapitypes.DBCluster, resp
pointer.StringValue(cr.Spec.ForProvider.DBClusterParameterGroupName) != pointer.StringValue(cluster.DBClusterParameterGroup),
pointer.BoolValue(cr.Spec.ForProvider.DeletionProtection) != pointer.BoolValue(cluster.DeletionProtection),
!areSameElements(cr.Spec.ForProvider.EnableCloudwatchLogsExports, cluster.EnabledCloudwatchLogsExports),
!isEngineVersionUpToDate(cr, resp),
pointer.Int64Value(cr.Spec.ForProvider.Port) != pointer.Int64Value(cluster.Port),
pointer.StringValue(cr.Spec.ForProvider.PreferredBackupWindow) != pointer.StringValue(cluster.PreferredBackupWindow),
pointer.StringValue(cr.Spec.ForProvider.PreferredMaintenanceWindow) != pointer.StringValue(cluster.PreferredMaintenanceWindow):
Expand All @@ -211,14 +222,53 @@ func (e *hooks) preUpdate(ctx context.Context, cr *svcapitypes.DBCluster, obj *s
if pwchanged {
obj.MasterUserPassword = aws.String(pw)
}

// ModifyDBCluster() returns error, when trying to upgrade major (minor is fine) EngineVersion:
// "Cannot change VPC security group while doing a major version upgrade."
// therefore EngineVersion update is entirely done separately in postUpdate
obj.EngineVersion = nil
// In case of a custom DBClusterParameterGroup, AWS requires for a major version update that
// EngineVersion and DBClusterParameterGroupName are in the same ModifyDBCluster()-call
obj.DBClusterParameterGroupName = nil

return nil
}

func (e *hooks) postUpdate(_ context.Context, cr *svcapitypes.DBCluster, resp *svcsdk.ModifyDBClusterOutput, upd managed.ExternalUpdate, err error) (managed.ExternalUpdate, error) {
func (e *hooks) postUpdate(ctx context.Context, cr *svcapitypes.DBCluster, resp *svcsdk.ModifyDBClusterOutput, upd managed.ExternalUpdate, err error) (managed.ExternalUpdate, error) {
if err != nil {
return managed.ExternalUpdate{}, err
}

input := GenerateDescribeDBClustersInput(cr)
// GenerateDescribeDBClustersInput returns an empty DescribeDBClustersInput
// and the function is generated by ack-generate, so we manually need to set the
// DBClusterIdentifier
input.DBClusterIdentifier = pointer.ToOrNilIfZeroValue(meta.GetExternalName(cr))
out, err := e.client.DescribeDBClustersWithContext(ctx, input)
if err != nil {
return managed.ExternalUpdate{}, errorutils.Wrap(cpresource.Ignore(IsNotFound, err), errDescribe)
}

needsEngineVersionUpdate := !isEngineVersionUpToDate(cr, out)
needsDBClusterParamGroupUpdate := pointer.StringValue(cr.Spec.ForProvider.DBClusterParameterGroupName) != pointer.StringValue(out.DBClusters[0].DBClusterParameterGroup)
needsPostUpdate := needsEngineVersionUpdate || needsDBClusterParamGroupUpdate

if needsPostUpdate {
modifyInput := &svcsdk.ModifyDBClusterInput{
DBClusterIdentifier: pointer.ToOrNilIfZeroValue(meta.GetExternalName(cr)),
ApplyImmediately: cr.Spec.ForProvider.ApplyImmediately,
DBClusterParameterGroupName: cr.Spec.ForProvider.DBClusterParameterGroupName,
}
if needsEngineVersionUpdate {
modifyInput.EngineVersion = cr.Spec.ForProvider.EngineVersion
modifyInput.AllowMajorVersionUpgrade = cr.Spec.ForProvider.AllowMajorVersionUpgrade
}

if _, err = e.client.ModifyDBClusterWithContext(ctx, modifyInput); err != nil {
return managed.ExternalUpdate{}, err
}
}

return upd, svcutils.UpdateTagsForResource(e.client, cr.Spec.ForProvider.Tags, resp.DBCluster.DBClusterArn)
}

Expand Down Expand Up @@ -442,6 +492,22 @@ func areSameElements(a1, a2 []*string) bool {
return true
}

func isEngineVersionUpToDate(cr *svcapitypes.DBCluster, out *svcsdk.DescribeDBClustersOutput) bool {
// If EngineVersion is not set, AWS sets a default value,
// so we do not try to update in this case
if cr.Spec.ForProvider.EngineVersion != nil {
if out.DBClusters[0].EngineVersion == nil {
return false
}

// Upgrade is only necessary if the spec version is higher.
// Downgrades are not possible in pointer.
c := utils.CompareEngineVersions(*cr.Spec.ForProvider.EngineVersion, *out.DBClusters[0].EngineVersion)
return c <= 0
}
return true
}

func (e *hooks) getPasswordFromRef(ctx context.Context, in *xpv1.SecretKeySelector, out *xpv1.SecretReference) (newPwd string, changed bool, err error) {
if in == nil {
return "", false, nil
Expand Down
Loading
Loading