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

rds.DBInstance - fix "up to date" when using ApplyImmediately #1196

Merged
merged 2 commits into from Mar 10, 2022

Conversation

chlunde
Copy link
Collaborator

@chlunde chlunde commented Mar 9, 2022

Description of your changes

DBInstance is in the example is never "up to date"

	2022/03/09 22:26:23   &v1alpha1.DBInstanceParameters{
		... // 4 ignored and 40 identical fields
		TDECredentialPassword: nil,
		Timezone:              nil,
		CustomDBInstanceParameters: v1alpha1.CustomDBInstanceParameters{
			... // 14 ignored and 2 identical fields
	- 		ApplyImmediately: nil,
	+ 		ApplyImmediately: &true,
		},
	  }

	2022-03-09T22:26:24.297+0100	DEBUG	provider-aws	Successfully requested update of external resource

ApplyImmediately is not an observable field, but an option for the modify API call,
so we should not compare it.

With the fix:

	2022-03-09T22:31:55.197+0100	DEBUG	provider-aws	External resource is up to date	{"controller": "managed/dbinstance.rds.aws.crossplane.io", "request": "/example-dbinstance", "uid": "b097b3ca-f29a-44e4-8cb4-1d4488a711c6", "version": "940953", "external-name": "example-dbinstance", "requeue-after": "2022-03-09T22:32:55.197+0100"}

I have:

  • Read and followed Crossplane's [contribution process].
  • Run make reviewable test to ensure this PR is ready for review.

How has this code been tested

Apply example file and check logs.

Added temporary debugging (not in commit):

diff --git a/pkg/controller/rds/dbinstance/setup.go b/pkg/controller/rds/dbinstance/setup.go
index 4f22ac8b..b8317a72 100644
--- a/pkg/controller/rds/dbinstance/setup.go
+++ b/pkg/controller/rds/dbinstance/setup.go
@@ -3,6 +3,7 @@ package dbinstance
 import (
        "context"
        "encoding/json"
+       "log"
        "strconv"
        "strings"
        "time"
@@ -329,7 +330,7 @@ func (e *custom) isUpToDate(cr *svcapitypes.DBInstance, out *svcsdk.DescribeDBIn
                return false, err
        }
 
-       return cmp.Equal(&svcapitypes.DBInstanceParameters{}, patch, cmpopts.EquateEmpty(),
+       diff := cmp.Diff(&svcapitypes.DBInstanceParameters{}, patch, cmpopts.EquateEmpty(),
                cmpopts.IgnoreTypes(&xpv1.Reference{}, &xpv1.Selector{}, []xpv1.Reference{}),
                cmpopts.IgnoreFields(svcapitypes.DBInstanceParameters{}, "Region"),
                cmpopts.IgnoreFields(svcapitypes.DBInstanceParameters{}, "Tags"),
@@ -340,7 +341,9 @@ func (e *custom) isUpToDate(cr *svcapitypes.DBInstance, out *svcsdk.DescribeDBIn
                cmpopts.IgnoreFields(svcapitypes.DBInstanceParameters{}, "PreferredMaintenanceWindow"),
                cmpopts.IgnoreFields(svcapitypes.DBInstanceParameters{}, "PreferredBackupWindow"),
                cmpopts.IgnoreFields(svcapitypes.CustomDBInstanceParameters{}, "ApplyImmediately"),
-       ) && !maintenanceWindowChanged && !backupWindowChanged && !pwChanged, nil
+       )
+       log.Println(diff)
+       return diff == "" && !maintenanceWindowChanged && !backupWindowChanged && !pwChanged, nil
 }
 
 func createPatch(out *svcsdk.DescribeDBInstancesOutput, target *svcapitypes.DBInstanceParameters) (*svcapitypes.DBInstanceParameters, error) {

    cannot create DBInstance in AWS: InvalidParameterCombination: Cannot find version 12.4 for postgres

Signed-off-by: Carl Henrik Lunde <chlunde@ifi.uio.no>
This is not an observable field, but an option for the modify API call,
so we should not compare it.

	2022/03/09 22:26:23   &v1alpha1.DBInstanceParameters{
		... // 4 ignored and 40 identical fields
		TDECredentialPassword: nil,
		Timezone:              nil,
		CustomDBInstanceParameters: v1alpha1.CustomDBInstanceParameters{
			... // 14 ignored and 2 identical fields
	- 		ApplyImmediately: nil,
	+ 		ApplyImmediately: &true,
		},
	  }

	2022-03-09T22:26:24.297+0100	DEBUG	provider-aws	Successfully requested update of external resource

with fix:

	2022-03-09T22:31:55.197+0100	DEBUG	provider-aws	External resource is up to date	{"controller": "managed/dbinstance.rds.aws.crossplane.io", "request": "/example-dbinstance", "uid": "b097b3ca-f29a-44e4-8cb4-1d4488a711c6", "version": "940953", "external-name": "example-dbinstance", "requeue-after": "2022-03-09T22:32:55.197+0100"}

Signed-off-by: Carl Henrik Lunde <chlunde@ifi.uio.no>
@chlunde chlunde requested a review from haarchri March 9, 2022 21:51
@haarchri haarchri merged commit c87e40e into crossplane-contrib:master Mar 10, 2022
tektondeploy pushed a commit to gtn3010/provider-aws that referenced this pull request Mar 12, 2024
…et-1.1

[release-1.1] Bump upjet to 1.2.2
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants