-
Notifications
You must be signed in to change notification settings - Fork 132
Add support for Orphan management policy #864
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
Open
bobh66
wants to merge
8
commits into
crossplane:main
Choose a base branch
from
nokia:add_orphan_policy
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+267
−9
Open
Changes from all commits
Commits
Show all changes
8 commits
Select commit
Hold shift + click to select a range
295bfb6
Add support for Orphan management policy
bobh66 129728a
build: remove release-1.18 from Renovate baseBranches
jbw976 5eca344
Merge pull request #865 from jbw976/renovate-bb
jbw976 445c242
fix npe
mjudeikis 5ea4f6f
Merge pull request #870 from mjudeikis/mjudeikis/npe
jbw976 a76212c
Add support for Orphan management policy
bobh66 ecd2a77
Merge branch 'add_orphan_policy' of https://github.com/nokia/crosspla…
bobh66 2caf1b9
Add Orphan to kubebuilder list
bobh66 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -86,7 +86,7 @@ func TestReconciler(t *testing.T) { | |
| }, | ||
| want: want{result: reconcile.Result{}}, | ||
| }, | ||
| "UnpublishConnectionDetailsDeletionPolicyDeleteOrpahn": { | ||
| "UnpublishConnectionDetailsDeletionPolicyDeleteOrphan": { | ||
| reason: "Errors unpublishing connection details should trigger a requeue after a short wait.", | ||
| args: args{ | ||
| m: &fake.Manager{ | ||
|
|
@@ -1735,6 +1735,45 @@ func TestReconciler(t *testing.T) { | |
| }, | ||
| want: want{result: reconcile.Result{Requeue: true}}, | ||
| }, | ||
| "ManagementPolicyOrphanCreateSuccessful": { | ||
| reason: "Successful managed resource creation using management policy Orphan should trigger a requeue after a short wait.", | ||
| args: args{ | ||
| m: &fake.Manager{ | ||
| Client: &test.MockClient{ | ||
| MockGet: test.NewMockGetFn(nil, func(obj client.Object) error { | ||
| mg := asLegacyManaged(obj, 42) | ||
| mg.SetManagementPolicies(xpv1.ManagementPolicies{xpv1.ManagementActionOrphan}) | ||
| return nil | ||
| }), | ||
| MockUpdate: test.NewMockUpdateFn(nil), | ||
| MockStatusUpdate: test.MockSubResourceUpdateFn(func(_ context.Context, obj client.Object, _ ...client.SubResourceUpdateOption) error { | ||
| want := newLegacyManaged(42) | ||
| want.SetManagementPolicies(xpv1.ManagementPolicies{xpv1.ManagementActionOrphan}) | ||
| meta.SetExternalCreatePending(want, time.Now()) | ||
| meta.SetExternalCreateSucceeded(want, time.Now()) | ||
| want.SetConditions(xpv1.ReconcileSuccess().WithObservedGeneration(42)) | ||
| want.SetConditions(xpv1.Creating().WithObservedGeneration(42)) | ||
| if diff := cmp.Diff(want, obj, test.EquateConditions(), cmpopts.EquateApproxTime(1*time.Second)); diff != "" { | ||
| reason := "Successful managed resource creation should be reported as a conditioned status." | ||
| t.Errorf("\nReason: %s\n-want, +got:\n%s", reason, diff) | ||
| } | ||
| return nil | ||
| }), | ||
| }, | ||
| Scheme: fake.SchemeWith(&fake.LegacyManaged{}), | ||
| }, | ||
| mg: resource.ManagedKind(fake.GVK(&fake.LegacyManaged{})), | ||
| o: []ReconcilerOption{ | ||
| WithInitializers(), | ||
| WithManagementPolicies(), | ||
| WithReferenceResolver(ReferenceResolverFn(func(_ context.Context, _ resource.Managed) error { return nil })), | ||
| WithExternalConnector(&NopConnector{}), | ||
| WithCriticalAnnotationUpdater(CriticalAnnotationUpdateFn(func(_ context.Context, _ client.Object) error { return nil })), | ||
| WithFinalizer(resource.FinalizerFns{AddFinalizerFn: func(_ context.Context, _ resource.Object) error { return nil }}), | ||
| }, | ||
| }, | ||
| want: want{result: reconcile.Result{Requeue: true}}, | ||
| }, | ||
| "ManagementPolicyCreateCreateSuccessful": { | ||
| reason: "Successful managed resource creation using management policy Create should trigger a requeue after a short wait.", | ||
| args: args{ | ||
|
|
@@ -1869,6 +1908,53 @@ func TestReconciler(t *testing.T) { | |
| }, | ||
| want: want{result: reconcile.Result{RequeueAfter: defaultPollInterval}}, | ||
| }, | ||
| "ManagementPolicyOrphanUpdateSuccessful": { | ||
| reason: "A successful managed resource update using management policies should trigger a requeue after a long wait.", | ||
| args: args{ | ||
| m: &fake.Manager{ | ||
| Client: &test.MockClient{ | ||
| MockGet: test.NewMockGetFn(nil, func(obj client.Object) error { | ||
| mg := asLegacyManaged(obj, 42) | ||
| mg.SetManagementPolicies(xpv1.ManagementPolicies{xpv1.ManagementActionOrphan}) | ||
| return nil | ||
| }), | ||
| MockStatusUpdate: test.MockSubResourceUpdateFn(func(_ context.Context, obj client.Object, _ ...client.SubResourceUpdateOption) error { | ||
| want := newLegacyManaged(42) | ||
| want.SetManagementPolicies(xpv1.ManagementPolicies{xpv1.ManagementActionOrphan}) | ||
| want.SetConditions(xpv1.ReconcileSuccess().WithObservedGeneration(42).WithObservedGeneration(42)) | ||
| if diff := cmp.Diff(want, obj, test.EquateConditions()); diff != "" { | ||
| reason := "A successful managed resource update should be reported as a conditioned status." | ||
| t.Errorf("\nReason: %s\n-want, +got:\n%s", reason, diff) | ||
| } | ||
| return nil | ||
| }), | ||
| }, | ||
| Scheme: fake.SchemeWith(&fake.LegacyManaged{}), | ||
| }, | ||
| mg: resource.ManagedKind(fake.GVK(&fake.LegacyManaged{})), | ||
| o: []ReconcilerOption{ | ||
| WithInitializers(), | ||
| WithManagementPolicies(), | ||
| WithReferenceResolver(ReferenceResolverFn(func(_ context.Context, _ resource.Managed) error { return nil })), | ||
| WithExternalConnector(ExternalConnectorFn(func(_ context.Context, _ resource.Managed) (ExternalClient, error) { | ||
| c := &ExternalClientFns{ | ||
| ObserveFn: func(_ context.Context, _ resource.Managed) (ExternalObservation, error) { | ||
| return ExternalObservation{ResourceExists: true, ResourceUpToDate: false}, nil | ||
| }, | ||
| UpdateFn: func(_ context.Context, _ resource.Managed) (ExternalUpdate, error) { | ||
| return ExternalUpdate{}, nil | ||
| }, | ||
| DisconnectFn: func(_ context.Context) error { | ||
| return nil | ||
| }, | ||
| } | ||
| return c, nil | ||
| })), | ||
| WithFinalizer(resource.FinalizerFns{AddFinalizerFn: func(_ context.Context, _ resource.Object) error { return nil }}), | ||
| }, | ||
| }, | ||
| want: want{result: reconcile.Result{RequeueAfter: defaultPollInterval}}, | ||
| }, | ||
| "ManagementPolicyUpdateUpdateSuccessful": { | ||
| reason: "A successful managed resource update using management policies should trigger a requeue after a long wait.", | ||
| args: args{ | ||
|
|
@@ -2175,6 +2261,14 @@ func TestLegacyManagementPoliciesResolverShouldCreate(t *testing.T) { | |
| }, | ||
| want: true, | ||
| }, | ||
| "ManagementPoliciesEnabledHasCreateOrphan": { | ||
| reason: "Should return true if management policies are enabled and managementPolicies has action Orphan", | ||
| args: args{ | ||
| managementPoliciesEnabled: true, | ||
| policy: xpv1.ManagementPolicies{xpv1.ManagementActionOrphan}, | ||
| }, | ||
| want: true, | ||
| }, | ||
| "ManagementPoliciesEnabledActionNotAllowed": { | ||
| reason: "Should return false if management policies are enabled and managementPolicies does not have Create", | ||
| args: args{ | ||
|
|
@@ -2236,6 +2330,14 @@ func TestLegacyManagementPoliciesResolverShouldUpdate(t *testing.T) { | |
| }, | ||
| want: false, | ||
| }, | ||
| "ManagementPoliciesEnabledHasUpdateOrphan": { | ||
| reason: "Should return true if management policies are enabled and managementPolicies has action Orphan", | ||
| args: args{ | ||
| managementPoliciesEnabled: true, | ||
| policy: xpv1.ManagementPolicies{xpv1.ManagementActionOrphan}, | ||
| }, | ||
| want: true, | ||
| }, | ||
| } | ||
| for name, tc := range cases { | ||
| t.Run(name, func(t *testing.T) { | ||
|
|
@@ -2289,6 +2391,14 @@ func TestLegacyManagementPoliciesResolverShouldLateInitialize(t *testing.T) { | |
| }, | ||
| want: false, | ||
| }, | ||
| "ManagementPoliciesEnabledHasLateInitializeOrphan": { | ||
| reason: "Should return true if management policies are enabled and managementPolicies has action Orphan", | ||
| args: args{ | ||
| managementPoliciesEnabled: true, | ||
| policy: xpv1.ManagementPolicies{xpv1.ManagementActionOrphan}, | ||
| }, | ||
| want: true, | ||
| }, | ||
| } | ||
| for name, tc := range cases { | ||
| t.Run(name, func(t *testing.T) { | ||
|
|
@@ -2459,6 +2569,33 @@ func TestLegacyShouldDelete(t *testing.T) { | |
| }, | ||
| want: want{delete: false}, | ||
| }, | ||
| "DeletionDeleteManagementActionOrphan": { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. what do you think about another test case for |
||
| reason: "Should orphan if management policies are enabled and deletion policy is set to Delete and management policy does not have action Delete.", | ||
| args: args{ | ||
| managementPoliciesEnabled: true, | ||
| managed: &fake.LegacyManaged{ | ||
| Orphanable: fake.Orphanable{ | ||
| Policy: xpv1.DeletionDelete, | ||
| }, | ||
| Manageable: fake.Manageable{ | ||
| Policy: xpv1.ManagementPolicies{xpv1.ManagementActionOrphan}, | ||
| }, | ||
| }, | ||
| }, | ||
| want: want{delete: false}, | ||
| }, | ||
| "DeletionManagementActionOrphan": { | ||
| reason: "Should orphan if management policies are enabled and deletion policy is not set and management policy does not have action Delete.", | ||
| args: args{ | ||
| managementPoliciesEnabled: true, | ||
| managed: &fake.LegacyManaged{ | ||
| Manageable: fake.Manageable{ | ||
| Policy: xpv1.ManagementPolicies{xpv1.ManagementActionOrphan}, | ||
| }, | ||
| }, | ||
| }, | ||
| want: want{delete: false}, | ||
| }, | ||
| } | ||
| for name, tc := range cases { | ||
| t.Run(name, func(t *testing.T) { | ||
|
|
||
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this was already done in #865, so it's odd to see here again 🤔