Skip to content

feat: Add delete override implementation #977

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.

Sign up for GitHub

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

Merged
zhiying-lin merged 4 commits into from
Dec 5, 2024
Merged

feat: Add delete override implementation #977

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
3a41176
add the delete implementation
Dec 3, 2024
c3bb485
Merge branch 'main' into add-delete-override
ryanzhang-oss Dec 3, 2024
998d32c
add more tests
Dec 4, 2024
a1fd408
add one more test
Dec 4, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion apis/placement/v1alpha1/override_types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ type OverrideRule struct {

// OverrideType defines the type of the override rules.
// +kubebuilder:validation:Enum=JSONPatch;Delete
// +kubebuilder:default:JSONPatch
// +kubebuilder:default=JSONPatch
// +optional
OverrideType OverrideType `json:"overrideType,omitempty"`

Expand Down
15 changes: 11 additions & 4 deletions config/crd/bases/placement.kubernetes-fleet.io_clusterresourceoverrides.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -303,8 +303,9 @@ spec:
- clusterSelectorTerms
type: object
jsonPatchOverrides:
description: JSONPatchOverrides defines a list of JSON patch
override rules.
description: |-
JSONPatchOverrides defines a list of JSON patch override rules.
This field is only allowed when OverrideType is JSONPatch.
items:
description: JSONPatchOverride applies a JSON patch on
the selected resources following [RFC 6902](https://datatracker.ietf.org/doc/html/rfc6902).
Expand Down Expand Up @@ -334,8 +335,14 @@ spec:
maxItems: 20
minItems: 1
type: array
required:
- jsonPatchOverrides
overrideType:
default: JSONPatch
description: OverrideType defines the type of the override
rules.
enum:
- JSONPatch
- Delete
type: string
type: object
maxItems: 20
minItems: 1
Expand Down
15 changes: 11 additions & 4 deletions config/crd/bases/placement.kubernetes-fleet.io_clusterresourceoverridesnapshots.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -317,8 +317,9 @@ spec:
- clusterSelectorTerms
type: object
jsonPatchOverrides:
description: JSONPatchOverrides defines a list of JSON
patch override rules.
description: |-
JSONPatchOverrides defines a list of JSON patch override rules.
This field is only allowed when OverrideType is JSONPatch.
items:
description: JSONPatchOverride applies a JSON patch
on the selected resources following [RFC 6902](https://datatracker.ietf.org/doc/html/rfc6902).
Expand Down Expand Up @@ -348,8 +349,14 @@ spec:
maxItems: 20
minItems: 1
type: array
required:
- jsonPatchOverrides
overrideType:
default: JSONPatch
description: OverrideType defines the type of the override
rules.
enum:
- JSONPatch
- Delete
type: string
type: object
maxItems: 20
minItems: 1
Expand Down
15 changes: 11 additions & 4 deletions config/crd/bases/placement.kubernetes-fleet.io_resourceoverrides.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -218,8 +218,9 @@ spec:
- clusterSelectorTerms
type: object
jsonPatchOverrides:
description: JSONPatchOverrides defines a list of JSON patch
override rules.
description: |-
JSONPatchOverrides defines a list of JSON patch override rules.
This field is only allowed when OverrideType is JSONPatch.
items:
description: JSONPatchOverride applies a JSON patch on
the selected resources following [RFC 6902](https://datatracker.ietf.org/doc/html/rfc6902).
Expand Down Expand Up @@ -249,8 +250,14 @@ spec:
maxItems: 20
minItems: 1
type: array
required:
- jsonPatchOverrides
overrideType:
default: JSONPatch
description: OverrideType defines the type of the override
rules.
enum:
- JSONPatch
- Delete
type: string
type: object
maxItems: 20
minItems: 1
Expand Down
15 changes: 11 additions & 4 deletions config/crd/bases/placement.kubernetes-fleet.io_resourceoverridesnapshots.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -232,8 +232,9 @@ spec:
- clusterSelectorTerms
type: object
jsonPatchOverrides:
description: JSONPatchOverrides defines a list of JSON
patch override rules.
description: |-
JSONPatchOverrides defines a list of JSON patch override rules.
This field is only allowed when OverrideType is JSONPatch.
items:
description: JSONPatchOverride applies a JSON patch
on the selected resources following [RFC 6902](https://datatracker.ietf.org/doc/html/rfc6902).
Expand Down Expand Up @@ -263,8 +264,14 @@ spec:
maxItems: 20
minItems: 1
type: array
required:
- jsonPatchOverrides
overrideType:
default: JSONPatch
description: OverrideType defines the type of the override
rules.
enum:
- JSONPatch
- Delete
type: string
type: object
maxItems: 20
minItems: 1
Expand Down
23 changes: 14 additions & 9 deletions pkg/controllers/workgenerator/controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -425,17 +425,22 @@ func (r *Reconciler) syncAllWork(ctx context.Context, resourceBinding *fleetv1be
}
var simpleManifests []fleetv1beta1.Manifest
for j := range snapshot.Spec.SelectedResources {
selectedResource := snapshot.Spec.SelectedResources[j]
if err := r.applyOverrides(&selectedResource, cluster, croMap, roMap); err != nil {
return false, false, err
selectedResource := snapshot.Spec.SelectedResources[j].DeepCopy()
// TODO: override the content of the wrapped resource instead of the envelope itself
resourceDeleted, overrideErr := r.applyOverrides(selectedResource, cluster, croMap, roMap)
if overrideErr != nil {
return false, false, overrideErr
}
if resourceDeleted {
klog.V(2).InfoS("The resource is deleted by the override rules", "snapshot", klog.KObj(snapshot), "selectedResource", snapshot.Spec.SelectedResources[j])
continue
}

// we need to special treat configMap with envelopeConfigMapAnnotation annotation,
// so we need to check the GVK and annotation of the selected resource
var uResource unstructured.Unstructured
if err := uResource.UnmarshalJSON(selectedResource.Raw); err != nil {
klog.ErrorS(err, "work has invalid content", "snapshot", klog.KObj(snapshot), "selectedResource", selectedResource.Raw)
return true, false, controller.NewUnexpectedBehaviorError(err)
if unMarshallErr := uResource.UnmarshalJSON(selectedResource.Raw); unMarshallErr != nil {
klog.ErrorS(unMarshallErr, "work has invalid content", "snapshot", klog.KObj(snapshot), "selectedResource", selectedResource.Raw)
return true, false, controller.NewUnexpectedBehaviorError(unMarshallErr)
}
if uResource.GetObjectKind().GroupVersionKind() == utils.ConfigMapGVK &&
len(uResource.GetAnnotations()[fleetv1beta1.EnvelopeConfigMapAnnotation]) != 0 {
Expand All @@ -447,11 +452,11 @@ func (r *Reconciler) syncAllWork(ctx context.Context, resourceBinding *fleetv1be
activeWork[work.Name] = work
newWork = append(newWork, work)
} else {
simpleManifests = append(simpleManifests, fleetv1beta1.Manifest(selectedResource))
simpleManifests = append(simpleManifests, fleetv1beta1.Manifest(*selectedResource))
}
}
if len(simpleManifests) == 0 {
klog.V(2).InfoS("the snapshot contains enveloped resource only", "snapshot", klog.KObj(snapshot))
klog.V(2).InfoS("the snapshot contains no resource to apply either because of override or enveloped resources", "snapshot", klog.KObj(snapshot))
}
// generate a work object for the manifests even if there is nothing to place
// to allow CRP to collect the status of the placement
Expand Down
28 changes: 19 additions & 9 deletions pkg/controllers/workgenerator/override.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,15 +92,21 @@ func (r *Reconciler) fetchResourceOverrideSnapshots(ctx context.Context, resourc
return roMap, nil
}

func (r *Reconciler) applyOverrides(resource *placementv1beta1.ResourceContent, cluster clusterv1beta1.MemberCluster, croMap map[placementv1beta1.ResourceIdentifier][]*placementv1alpha1.ClusterResourceOverrideSnapshot, roMap map[placementv1beta1.ResourceIdentifier][]*placementv1alpha1.ResourceOverrideSnapshot) error {
// applyOverrides applies the overrides on the selected resources.
// The resource could be selected by both ClusterResourceOverride and ResourceOverride.
// It returns
// - true if the resource is deleted by the overrides.
// - an error if the override rules are invalid.
func (r *Reconciler) applyOverrides(resource *placementv1beta1.ResourceContent, cluster clusterv1beta1.MemberCluster,
croMap map[placementv1beta1.ResourceIdentifier][]*placementv1alpha1.ClusterResourceOverrideSnapshot, roMap map[placementv1beta1.ResourceIdentifier][]*placementv1alpha1.ResourceOverrideSnapshot) (bool, error) {
if len(croMap) == 0 && len(roMap) == 0 {
return nil
return false, nil
}

var uResource unstructured.Unstructured
if err := uResource.UnmarshalJSON(resource.Raw); err != nil {
klog.ErrorS(err, "Work has invalid content", "selectedResource", resource.Raw)
return controller.NewUnexpectedBehaviorError(err)
return false, controller.NewUnexpectedBehaviorError(err)
}
gvk := uResource.GetObjectKind().GroupVersionKind()
key := placementv1beta1.ResourceIdentifier{
Expand Down Expand Up @@ -131,13 +137,12 @@ func (r *Reconciler) applyOverrides(resource *placementv1beta1.ResourceContent,
}
if err := applyOverrideRules(resource, cluster, snapshot.Spec.OverrideSpec.Policy.OverrideRules); err != nil {
klog.ErrorS(err, "Failed to apply the override rules", "clusterResourceOverrideSnapshot", klog.KObj(snapshot))
return err
return false, err
}
}
klog.V(2).InfoS("Applied clusterResourceOverrideSnapshots", "resource", klog.KObj(&uResource), "numberOfOverrides", len(croMap[key]))

// If the resource is selected by both ClusterResourceOverride and ResourceOverride, ResourceOverride will win when
// resolving conflicts.
// If the resource is selected by both ClusterResourceOverride and ResourceOverride, ResourceOverride will win when resolving conflicts.
Copy link
Contributor

@zhiying-lin zhiying-lin Dec 3, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what happens for a namespace scope resource which is selected by both cro and ro?

for example, cro deletes the resource, and ro overrides the resource.

Copy link
Contributor Author

@ryanzhang-oss ryanzhang-oss Dec 4, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we actually have the same problem now when a CRO deletes a field and a ro wants to replace it. This can happen even between any CRO or RO.

// Apply ResourceOverrideSnapshots.
if !isClusterScopeResource {
key = placementv1beta1.ResourceIdentifier{
Expand All @@ -155,12 +160,12 @@ func (r *Reconciler) applyOverrides(resource *placementv1beta1.ResourceContent,
}
if err := applyOverrideRules(resource, cluster, snapshot.Spec.OverrideSpec.Policy.OverrideRules); err != nil {
klog.ErrorS(err, "Failed to apply the override rules", "resourceOverrideSnapshot", klog.KObj(snapshot))
return err
return false, err
}
}
klog.V(2).InfoS("Applied resourceOverrideSnapshots", "resource", klog.KObj(&uResource), "numberOfOverrides", len(roMap[key]))
}
return nil
return resource.Raw == nil, nil
}

func applyOverrideRules(resource *placementv1beta1.ResourceContent, cluster clusterv1beta1.MemberCluster, rules []placementv1alpha1.OverrideRule) error {
Expand All @@ -173,7 +178,12 @@ func applyOverrideRules(resource *placementv1beta1.ResourceContent, cluster clus
if !matched {
continue
}

if rule.OverrideType == placementv1alpha1.DeleteOverrideType {
// Delete the resource
resource.Raw = nil
return nil
}
// Apply JSONPatchOverrides by default
if err := applyJSONPatchOverride(resource, rule.JSONPatchOverrides); err != nil {
klog.ErrorS(err, "Failed to apply JSON patch override")
return controller.NewUserError(err)
Expand Down
Loading
Loading