feat: add CompareResourceForPreDelete and DeltaForPreDelete support#686
feat: add CompareResourceForPreDelete and DeltaForPreDelete support#686sapphirew wants to merge 1 commit intoaws-controllers-k8s:mainfrom
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: sapphirew The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
michaelhtm
left a comment
michaelhtm
left a comment
There was a problem hiding this comment.
Thanks @sapphirew
left one comment below
pkg/generate/code/compare.go
Outdated
| // CompareResourceForPreDelete returns the Go code that traverses a set of two | ||
| // Resources, adding differences between the two Resources to an | ||
| // `ackcompare.Delta`. Unlike CompareResource, this function does NOT skip | ||
| // fields where compareConfig.IsIgnored is true. This is used for pre-delete | ||
| // sync to ensure fields like DeletionProtectionEnabled are included in the | ||
| // delta comparison. |
There was a problem hiding this comment.
should we make this delta comparison opt-in? Only compute delta on specific fields defined in generator.yaml?
In dsql example, we could just compare DeletionProtection field only
There was a problem hiding this comment.
good call, that would provide fine-control on specific fields for a controller.
The current delta comparison can be enabled by a flag like this instead:
resources:
Cluster:
pre_delete_sync:
compare_all: true
…Delete support Add pre-delete delta comparison support to the code generator. Controllers can opt specific fields into pre-delete comparison via compare.pre_delete_include: true in generator.yaml, so that fields like DeletionProtectionEnabled are synced before deletion even when they use compare.is_ignored for normal reconciliation. Config changes: - Add PreDeleteInclude bool to CompareFieldConfig (field.go) - Add PreDeleteSyncConfig with CompareAll to ResourceConfig (resource.go) Code generation: - Add CompareResourceForPreDelete function that emits comparison code only for opted-in fields (or all fields when compare_all is true) - Register GoCodeCompareForPreDelete template function (controller.go) - Add newResourceDeltaForPreDelete in delta.go.tpl - Add DeltaForPreDelete method on resourceDescriptor in descriptor.go.tpl Tests: - Four tests covering no-opt-in (empty), explicit opt-in, compare_all, and empty-without-opt-in scenarios - Two new S3 test fixture generator YAMLs
sapphirew
force-pushed
the
pre-delete-sync
branch
from
028a963 to
b3f5d67
Compare
|
/test s3-controller-test |
|
/test lambda-controller-test |
|
/retest |
2 similar comments
|
/retest |
|
/retest |
3 participants
Add pre-delete delta comparison support to the code generator. Controllers can opt specific fields into pre-delete comparison via
compare.pre_delete_include: truein generator.yaml, so that fields likeDeletionProtectionEnabledare synced before deletion even when they usecompare.is_ignoredfor normal reconciliation.Config changes:
PreDeleteIncludebool toCompareFieldConfig(field.go)PreDeleteSyncConfigwithCompareAll to ResourceConfig(resource.go)Code generation:
CompareResourceForPreDeletefunction that emits comparison codeonly for opted-in fields (or all fields when compare_all is true)
GoCodeCompareForPreDeletetemplate function (controller.go)newResourceDeltaForPreDeletein delta.go.tplDeltaForPreDeletemethod onresourceDescriptorin descriptor.go.tplTests:
and empty-without-opt-in scenarios
runtime changes: aws-controllers-k8s/runtime#234
Issue #, if available:
Description of changes:
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.