Skip to content

fix: issue 2614 dbcluster respect only supplying major engineVersion #237

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.

Sign up for GitHub

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

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

fix: issue 2614 dbcluster respect only supplying major engineVersion #237

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
680737a
Update hooks.go
rmullinnix461332 Aug 28, 2025
0c6c235
Update hooks.go
rmullinnix461332 Aug 28, 2025
210344a
Update delta.go
rmullinnix461332 Aug 28, 2025
95a1616
Update delta.go
rmullinnix461332 Aug 28, 2025
a84204a
Update pkg/resource/db_cluster/hooks.go
rmullinnix461332 Oct 2, 2025
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
8 changes: 7 additions & 1 deletion pkg/resource/db_cluster/delta.go
View file
Open in desktop

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

17 changes: 17 additions & 0 deletions pkg/resource/db_cluster/hooks.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import (
"context"
"errors"
"fmt"
"strings"

svcapitypes "github.com/aws-controllers-k8s/rds-controller/apis/v1alpha1"
"github.com/aws-controllers-k8s/runtime/apis/core/v1alpha1"
Expand Down Expand Up @@ -400,3 +401,19 @@ func setDeleteDBClusterInput(
input.DeleteAutomatedBackups = params.DeleteAutomatedBackup
return nil
}

// RDS will choose preferred engine minor version if only
// engine major version is provided and controller should not
// treat them as different, such as spec has 14, status has 14.1
// controller should treat them as same
func reconcileDBClusterEngineVersion(
a *resource,
b *resource,
) {
if a != nil && b != nil &&
a.ko.Spec.EngineVersion != nil && b.ko.Spec.EngineVersion != nil &&
strings.HasPrefix(*b.ko.Spec.EngineVersion, *a.ko.Spec.EngineVersion) &&
b.ko.Spec.AutoMinorVersionUpgrade != nil && *b.ko.Spec.AutoMinorVersionUpgrade {
Copy link
Contributor

@knottnt knottnt Oct 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this not be using the state of the desired AutoMinorVersionUpgrade instead of what we just read from AWS? If the latest value has drifted I don't think we'd want it to impact how the EngineVersion is handled.

Copy link
Author

@rmullinnix461332 rmullinnix461332 Oct 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

agreed

a.ko.Spec.EngineVersion = b.ko.Spec.EngineVersion
}
}
4 changes: 3 additions & 1 deletion pkg/resource/db_instance/delta.go
View file
Open in desktop

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