Skip to content

Commit f8e71c8

Browse files
Merge branch 'main' into refractor-crp-controller
2 parents e42a6c5 + 4221b88 commit f8e71c8

File tree

7 files changed

+427
-4
lines changed

7 files changed

+427
-4
lines changed
Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,77 @@
1+
# CRP Delete Policy Implementation
2+
3+
## Problem Analysis
4+
5+
The issue requests adding API options to allow customers to choose whether to delete placed resources when a CRP (ClusterResourcePlacement) is deleted.
6+
7+
### Current Deletion Behavior
8+
1. When a CRP is deleted, it has two finalizers:
9+
- `ClusterResourcePlacementCleanupFinalizer` - handled by CRP controller to delete snapshots
10+
- `SchedulerCleanupFinalizer` - handled by scheduler to delete bindings
11+
12+
2. The deletion flow:
13+
- CRP controller removes snapshots (ClusterSchedulingPolicySnapshot, ClusterResourceSnapshot)
14+
- Scheduler removes bindings (ClusterResourceBinding) which triggers cleanup of placed resources
15+
- Currently there's no option for users to control whether placed resources are deleted
16+
17+
### References
18+
- Kubernetes DeleteOptions has `PropagationPolicy` with values: `Orphan`, `Background`, `Foreground`
19+
- AKS API has `DeletePolicy` pattern (couldn't fetch exact details but following similar pattern)
20+
21+
## Implementation Plan
22+
23+
### Phase 1: API Design
24+
- [x] Add `DeleteStrategy` struct to `RolloutStrategy` in beta API
25+
- [x] Define `PropagationPolicy` field with enum values: `Delete` (default), `Abandon`
26+
- [x] Update API documentation and validation
27+
- [x] Move DeleteStrategy inside RolloutStrategy after ApplyStrategy for consistency
28+
29+
### Phase 2: Implementation Details
30+
TODO: @Arvindthiru to fill out the details for controller logic implementation.
31+
32+
### Phase 3: Testing
33+
- [ ] Add unit tests for new deletion policy options
34+
- [ ] Add integration tests to verify behavior
35+
- [ ] Test both `Delete` and `Abandon` scenarios
36+
37+
### Phase 4: Documentation & Examples
38+
- [ ] Update CRD documentation
39+
- [ ] Add example configurations
40+
- [ ] Update any user-facing documentation
41+
42+
## Success Criteria
43+
- [x] CRP API has `deleteStrategy` field with `Delete`/`Abandon` options inside RolloutStrategy
44+
- [x] Default behavior (`Delete`) preserves current functionality
45+
- [ ] `Abandon` policy leaves placed resources intact when CRP is deleted
46+
- [ ] All tests pass including new deletion policy tests
47+
- [x] Changes are minimal and backwards compatible
48+
49+
## Current API Structure
50+
51+
The DeleteStrategy is now part of RolloutStrategy:
52+
53+
```go
54+
type RolloutStrategy struct {
55+
// ... other fields ...
56+
ApplyStrategy *ApplyStrategy `json:"applyStrategy,omitempty"`
57+
DeleteStrategy *DeleteStrategy `json:"deleteStrategy,omitempty"`
58+
}
59+
60+
type DeleteStrategy struct {
61+
PropagationPolicy DeletePropagationPolicy `json:"propagationPolicy,omitempty"`
62+
}
63+
64+
type DeletePropagationPolicy string
65+
66+
const (
67+
DeletePropagationPolicyDelete DeletePropagationPolicy = "Delete" // default
68+
DeletePropagationPolicyAbandon DeletePropagationPolicy = "Abandon"
69+
)
70+
```
71+
72+
## Behavior Summary
73+
74+
- **Default (`Delete`)**: When CRP is deleted, all placed resources are removed from member clusters (current behavior)
75+
- **Abandon**: When CRP is deleted, placed resources remain on member clusters but are no longer managed by Fleet
76+
77+
This provides customers with the flexibility to choose between complete cleanup or leaving resources in place when deleting a CRP.

.github/copilot-instructions.md

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ The main idea is that we are creating a multi-cluster application management sol
1111
- Follow the [Uber Go Style Guide](https://github.com/uber-go/guide/blob/master/style.md) if possible.
1212
- Favor using the standard library over third-party libraries.
1313
- Run "make reviewable" before submitting a pull request to ensure the code is formatted correctly and all dependencies are up to date.
14+
- The title of a PR must use one of the following prefixes: "[WIP] ", "feat: ", "test: ", "fix: ", "docs: ", "style: ", "interface: ", "util: ", "chore: ", "ci: ", "perf: ", "refactor: ", "revert: ". Please pick one that matches the PR content the most.
1415

1516
## Terminology
1617
- **Fleet**: A conceptual term referring to a collection of clusters.
@@ -216,4 +217,4 @@ This practice creates a trail of decision points that document our thought proce
216217

217218
**User**: This looks good.
218219

219-
**Agent**: I've updated the breadcrumb with the latest understanding.
220+
**Agent**: I've updated the breadcrumb with the latest understanding.

apis/placement/v1beta1/clusterresourceplacement_types.go

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -487,6 +487,10 @@ type RolloutStrategy struct {
487487
// ApplyStrategy describes when and how to apply the selected resources to the target cluster.
488488
// +kubebuilder:validation:Optional
489489
ApplyStrategy *ApplyStrategy `json:"applyStrategy,omitempty"`
490+
491+
// DeleteStrategy configures the deletion behavior when the ClusterResourcePlacement is deleted.
492+
// +kubebuilder:validation:Optional
493+
DeleteStrategy *DeleteStrategy `json:"deleteStrategy,omitempty"`
490494
}
491495

492496
// ApplyStrategy describes when and how to apply the selected resource to the target cluster.
@@ -1318,6 +1322,38 @@ const (
13181322
PickFixedPlacementType PlacementType = "PickFixed"
13191323
)
13201324

1325+
// DeleteStrategy configures the deletion behavior when a placement is deleted.
1326+
type DeleteStrategy struct {
1327+
// PropagationPolicy controls whether to delete placed resources when placement is deleted.
1328+
//
1329+
// Available options:
1330+
//
1331+
// * Delete: all placed resources on member clusters will be deleted when
1332+
// the placement is deleted. This is the default behavior.
1333+
//
1334+
// * Abandon: all placed resources on member clusters will be left intact (abandoned)
1335+
// when the placement is deleted.
1336+
//
1337+
// +kubebuilder:validation:Enum=Abandon;Delete
1338+
// +kubebuilder:default=Delete
1339+
// +kubebuilder:validation:Optional
1340+
PropagationPolicy DeletePropagationPolicy `json:"propagationPolicy,omitempty"`
1341+
}
1342+
1343+
// DeletePropagationPolicy identifies the propagation policy when a placement is deleted.
1344+
// +enum
1345+
type DeletePropagationPolicy string
1346+
1347+
const (
1348+
// DeletePropagationPolicyAbandon instructs Fleet to leave (abandon) all placed resources on member
1349+
// clusters when the placement is deleted.
1350+
DeletePropagationPolicyAbandon DeletePropagationPolicy = "Abandon"
1351+
1352+
// DeletePropagationPolicyDelete instructs Fleet to delete all placed resources on member clusters
1353+
// when the placement is deleted. This is the default behavior.
1354+
DeletePropagationPolicyDelete DeletePropagationPolicy = "Delete"
1355+
)
1356+
13211357
// ClusterResourcePlacementList contains a list of ClusterResourcePlacement.
13221358
// +kubebuilder:resource:scope="Cluster"
13231359
// +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object

apis/placement/v1beta1/zz_generated.deepcopy.go

Lines changed: 20 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

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

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1905,6 +1905,28 @@ spec:
19051905
- Never
19061906
type: string
19071907
type: object
1908+
deleteStrategy:
1909+
description: DeleteStrategy configures the deletion behavior when
1910+
the ClusterResourcePlacement is deleted.
1911+
properties:
1912+
propagationPolicy:
1913+
default: Delete
1914+
description: |-
1915+
PropagationPolicy controls how the deletion is propagated to placed resources.
1916+
Defaults to "Delete".
1917+
1918+
Available options:
1919+
1920+
* Delete: all placed resources on member clusters will be deleted when the
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.
1925+
enum:
1926+
- Abandon
1927+
- Delete
1928+
type: string
1929+
type: object
19081930
rollingUpdate:
19091931
description: Rolling update config params. Present only if RolloutStrategyType
19101932
= RollingUpdate.

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

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -840,6 +840,28 @@ spec:
840840
- Never
841841
type: string
842842
type: object
843+
deleteStrategy:
844+
description: DeleteStrategy configures the deletion behavior when
845+
the ClusterResourcePlacement is deleted.
846+
properties:
847+
propagationPolicy:
848+
default: Delete
849+
description: |-
850+
PropagationPolicy controls how the deletion is propagated to placed resources.
851+
Defaults to "Delete".
852+
853+
Available options:
854+
855+
* Delete: all placed resources on member clusters will be deleted when the
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.
860+
enum:
861+
- Abandon
862+
- Delete
863+
type: string
864+
type: object
843865
rollingUpdate:
844866
description: Rolling update config params. Present only if RolloutStrategyType
845867
= RollingUpdate.

0 commit comments

Comments
 (0)