Skip to content

Commit 02bbff9

Browse files
Copilotcircy9
andcommitted
Change default DeleteStrategy propagationPolicy to Delete and update documentation
Co-authored-by: circy9 <[email protected]>
1 parent 27771e4 commit 02bbff9

File tree

4 files changed

+51
-53
lines changed

4 files changed

+51
-53
lines changed

.github/.copilot/breadcrumbs/2025-01-15-1400-crp-delete-policy.md

Lines changed: 31 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -21,59 +21,57 @@ The issue requests adding API options to allow customers to choose whether to de
2121
## Implementation Plan
2222

2323
### Phase 1: API Design
24-
- [x] Add `DeletionPolicy` field to `PlacementSpec` in beta API
25-
- [x] Define enum values: `Delete` (default), `Orphan`
24+
- [x] Add `DeleteStrategy` struct to `RolloutStrategy` in beta API
25+
- [x] Define `PropagationPolicy` field with enum values: `Delete` (default), `Abandon`
2626
- [x] Update API documentation and validation
27+
- [x] Move DeleteStrategy inside RolloutStrategy after ApplyStrategy for consistency
2728

28-
### Phase 2: Controller Logic Updates
29-
- [x] Update scheduler to respect deletion policy when cleaning up bindings
30-
- [x] Add logic to skip binding deletion when policy is `Orphan`
31-
- [x] Update CRP controller to handle policy appropriately
29+
### Phase 2: Implementation Details
30+
TODO: @Arvindthiru to fill out the details for controller logic implementation.
3231

3332
### Phase 3: Testing
34-
- [x] Add unit tests for new deletion policy options
35-
- [x] Add integration tests to verify behavior
36-
- [x] Test both `Delete` and `Orphan` scenarios
33+
- [ ] Add unit tests for new deletion policy options
34+
- [ ] Add integration tests to verify behavior
35+
- [ ] Test both `Delete` and `Abandon` scenarios
3736

3837
### Phase 4: Documentation & Examples
39-
- [x] Update CRD documentation
38+
- [ ] Update CRD documentation
4039
- [ ] Add example configurations
41-
- [x] Update any user-facing documentation
40+
- [ ] Update any user-facing documentation
4241

4342
## Success Criteria
44-
- [x] CRP API has `deletionPolicy` field with `Delete`/`Orphan` options
43+
- [x] CRP API has `deleteStrategy` field with `Delete`/`Abandon` options inside RolloutStrategy
4544
- [x] Default behavior (`Delete`) preserves current functionality
46-
- [x] `Orphan` policy leaves placed resources intact when CRP is deleted
47-
- [x] All tests pass including new deletion policy tests
45+
- [ ] `Abandon` policy leaves placed resources intact when CRP is deleted
46+
- [ ] All tests pass including new deletion policy tests
4847
- [x] Changes are minimal and backwards compatible
4948

50-
## Implementation Details
49+
## Current API Structure
5150

52-
The key insight is that the scheduler's `cleanUpAllBindingsFor` method is what triggers the deletion of placed resources by deleting the bindings. We need to modify this to respect a deletion policy.
51+
The DeleteStrategy is now part of RolloutStrategy:
5352

54-
### Changes Made:
53+
```go
54+
type RolloutStrategy struct {
55+
// ... other fields ...
56+
ApplyStrategy *ApplyStrategy `json:"applyStrategy,omitempty"`
57+
DeleteStrategy *DeleteStrategy `json:"deleteStrategy,omitempty"`
58+
}
5559

56-
1. **API Changes**: Added `DeletionPolicy` enum and field to `PlacementSpec` in `apis/placement/v1beta1/clusterresourceplacement_types.go`
57-
- Enum with values `Delete` (default) and `Orphan`
58-
- Field with proper validation and documentation
60+
type DeleteStrategy struct {
61+
PropagationPolicy DeletePropagationPolicy `json:"propagationPolicy,omitempty"`
62+
}
5963

60-
2. **Scheduler Logic Changes**: Modified `cleanUpAllBindingsFor` method in `pkg/scheduler/scheduler.go`
61-
- Check deletion policy from placement spec
62-
- For `Delete` policy: maintain current behavior (delete bindings)
63-
- For `Orphan` policy: remove finalizers but don't delete bindings (leaves placed resources intact)
64+
type DeletePropagationPolicy string
6465

65-
3. **Tests Added**: Extended existing test suite in `pkg/scheduler/scheduler_test.go`
66-
- Test cases for both `Delete` and `Orphan` policies
67-
- Verified that bindings are deleted with `Delete` policy
68-
- Verified that bindings are orphaned (finalizers removed but not deleted) with `Orphan` policy
69-
70-
4. **Generated Artifacts**:
71-
- Updated CRDs with proper enum validation and default value
72-
- Regenerated Go code for API types
66+
const (
67+
DeletePropagationPolicyDelete DeletePropagationPolicy = "Delete" // default
68+
DeletePropagationPolicyAbandon DeletePropagationPolicy = "Abandon"
69+
)
70+
```
7371

7472
## Behavior Summary
7573

7674
- **Default (`Delete`)**: When CRP is deleted, all placed resources are removed from member clusters (current behavior)
77-
- **Orphan**: When CRP is deleted, placed resources remain on member clusters but are no longer managed by Fleet
75+
- **Abandon**: When CRP is deleted, placed resources remain on member clusters but are no longer managed by Fleet
7876

7977
This provides customers with the flexibility to choose between complete cleanup or leaving resources in place when deleting a CRP.

apis/placement/v1beta1/clusterresourceplacement_types.go

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1326,18 +1326,18 @@ const (
13261326
// DeleteStrategy configures the deletion behavior when a placement is deleted.
13271327
type DeleteStrategy struct {
13281328
// PropagationPolicy controls how the deletion is propagated to placed resources.
1329-
// Defaults to "Abandon".
1329+
// Defaults to "Delete".
13301330
//
13311331
// Available options:
13321332
//
1333-
// * Abandon: all placed resources on member clusters will be left intact (abandoned)
1334-
// when the ClusterResourcePlacement is deleted. This is the default behavior.
1335-
//
13361333
// * Delete: all placed resources on member clusters will be deleted when the
1337-
// ClusterResourcePlacement is deleted.
1334+
// ClusterResourcePlacement is deleted. This is the default behavior.
1335+
//
1336+
// * Abandon: all placed resources on member clusters will be left intact (abandoned)
1337+
// when the ClusterResourcePlacement is deleted.
13381338
//
13391339
// +kubebuilder:validation:Enum=Abandon;Delete
1340-
// +kubebuilder:default=Abandon
1340+
// +kubebuilder:default=Delete
13411341
// +kubebuilder:validation:Optional
13421342
PropagationPolicy DeletePropagationPolicy `json:"propagationPolicy,omitempty"`
13431343
}
@@ -1348,11 +1348,11 @@ type DeletePropagationPolicy string
13481348

13491349
const (
13501350
// DeletePropagationPolicyAbandon instructs Fleet to leave (abandon) all placed resources on member
1351-
// clusters when the ClusterResourcePlacement is deleted. This is the default behavior.
1351+
// clusters when the ClusterResourcePlacement is deleted.
13521352
DeletePropagationPolicyAbandon DeletePropagationPolicy = "Abandon"
13531353

13541354
// DeletePropagationPolicyDelete instructs Fleet to delete all placed resources on member clusters
1355-
// when the ClusterResourcePlacement is deleted.
1355+
// when the ClusterResourcePlacement is deleted. This is the default behavior.
13561356
DeletePropagationPolicyDelete DeletePropagationPolicy = "Delete"
13571357
)
13581358

config/crd/bases/placement.kubernetes-fleet.io_clusterresourceplacements.yaml

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1910,18 +1910,18 @@ spec:
19101910
the ClusterResourcePlacement is deleted.
19111911
properties:
19121912
propagationPolicy:
1913-
default: Abandon
1913+
default: Delete
19141914
description: |-
19151915
PropagationPolicy controls how the deletion is propagated to placed resources.
1916-
Defaults to "Abandon".
1916+
Defaults to "Delete".
19171917
19181918
Available options:
19191919
1920-
* Abandon: all placed resources on member clusters will be left intact (abandoned)
1921-
when the ClusterResourcePlacement is deleted. This is the default behavior.
1922-
19231920
* Delete: all placed resources on member clusters will be deleted when the
1924-
ClusterResourcePlacement is deleted.
1921+
ClusterResourcePlacement is deleted. This is the default behavior.
1922+
1923+
* Abandon: all placed resources on member clusters will be left intact (abandoned)
1924+
when the ClusterResourcePlacement is deleted.
19251925
enum:
19261926
- Abandon
19271927
- Delete

config/crd/bases/placement.kubernetes-fleet.io_resourceplacements.yaml

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -845,18 +845,18 @@ spec:
845845
the ClusterResourcePlacement is deleted.
846846
properties:
847847
propagationPolicy:
848-
default: Abandon
848+
default: Delete
849849
description: |-
850850
PropagationPolicy controls how the deletion is propagated to placed resources.
851-
Defaults to "Abandon".
851+
Defaults to "Delete".
852852
853853
Available options:
854854
855-
* Abandon: all placed resources on member clusters will be left intact (abandoned)
856-
when the ClusterResourcePlacement is deleted. This is the default behavior.
857-
858855
* Delete: all placed resources on member clusters will be deleted when the
859-
ClusterResourcePlacement is deleted.
856+
ClusterResourcePlacement is deleted. This is the default behavior.
857+
858+
* Abandon: all placed resources on member clusters will be left intact (abandoned)
859+
when the ClusterResourcePlacement is deleted.
860860
enum:
861861
- Abandon
862862
- Delete

0 commit comments

Comments
 (0)