Skip to content

Commit

Permalink
Clarify patch deletion logic
Browse files Browse the repository at this point in the history
  • Loading branch information
Jordan Olshevski committed Apr 29, 2024
1 parent 159e0c6 commit f82fcb6
Show file tree
Hide file tree
Showing 3 changed files with 7 additions and 5 deletions.
2 changes: 1 addition & 1 deletion internal/controllers/reconciliation/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,7 @@ func (c *Controller) reconcileResource(ctx context.Context, comp *apiv1.Composit
reconciliationLatency.Observe(time.Since(start).Seconds())
}()

if resource.Deleted() || (resource.Patch != nil && resource.PatchDeletes()) {
if resource.Deleted() || (resource.Patch != nil && resource.PatchSetsDeletionTimestamp()) {
if current == nil || current.GetDeletionTimestamp() != nil {
return false, nil // already deleted - nothing to do
}
Expand Down
6 changes: 4 additions & 2 deletions internal/resource/resource.go
Original file line number Diff line number Diff line change
Expand Up @@ -75,12 +75,14 @@ func (r *Resource) NeedsToBePatched(current *unstructured.Unstructured) bool {
return !equality.Semantic.DeepEqual(current, patched)
}

func (r *Resource) PatchDeletes() bool {
func (r *Resource) PatchSetsDeletionTimestamp() bool {
if r.Patch == nil {
return false
}

patchedjson, err := r.Patch.Apply([]byte(`{"apiVersion": "anything/v1", "kind":"Anything", "metadata":{}}`))
// Apply the patch to a minimally-viable unstructured resource.
// This is needed to satisfy the validation logic of the unstructured json parser, which requires a kind/apiVersion.
patchedjson, err := r.Patch.Apply([]byte(`{"apiVersion": "eno.azure.io/v1", "kind":"PatchPlaceholder", "metadata":{}}`))
if err != nil {
return false
}
Expand Down
4 changes: 2 additions & 2 deletions internal/resource/resource_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ var newResourceTests = []struct {
"kind": "ConfigMap",
"data": map[string]any{},
}}
assert.False(t, r.PatchDeletes())
assert.False(t, r.PatchSetsDeletionTimestamp())
assert.True(t, r.NeedsToBePatched(cm))
assert.True(t, r.NeedsToBePatched(cm))

Expand Down Expand Up @@ -93,7 +93,7 @@ var newResourceTests = []struct {
Assert: func(t *testing.T, r *Resource) {
assert.Equal(t, schema.GroupVersionKind{Version: "v1", Kind: "ConfigMap"}, r.GVK)
assert.Len(t, r.Patch, 1)
assert.True(t, r.PatchDeletes())
assert.True(t, r.PatchSetsDeletionTimestamp())
},
},
}
Expand Down

0 comments on commit f82fcb6

Please sign in to comment.