Skip to content

Commit

Permalink
Don't delete resources unless field managers match
Browse files Browse the repository at this point in the history
  • Loading branch information
blampe committed Jan 3, 2025
1 parent b0ab343 commit 0a9b5d5
Show file tree
Hide file tree
Showing 9 changed files with 287 additions and 0 deletions.
25 changes: 25 additions & 0 deletions provider/pkg/await/await.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ import (
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
"k8s.io/apimachinery/pkg/runtime/schema"
"k8s.io/apimachinery/pkg/types"
"k8s.io/apimachinery/pkg/util/sets"
"k8s.io/apimachinery/pkg/watch"
"k8s.io/apiserver/pkg/storage/names"
"k8s.io/client-go/dynamic"
Expand Down Expand Up @@ -834,6 +835,30 @@ func Deletion(c DeleteConfig) error {
PropagationPolicy: &deletePolicy,
}

live, err := client.Get(c.Context, c.Name, metav1.GetOptions{})
if err != nil {
return nilIfGVKDeleted(err)
}

actualSSAManagers := sets.Set[string]{}
for _, f := range live.GetManagedFields() {
// Ignore fields not managed by pulumi SSA.
if !strings.HasPrefix(f.Manager, "pulumi-kubernetes-") {
continue
}
actualSSAManagers = actualSSAManagers.Insert(f.Manager)
}
if actualSSAManagers.Len() > 0 {
fmt.Println("OVERLAP")
}
if c.ServerSideApply && !actualSSAManagers.Has(c.FieldManager) {
// Didn't find our expected manager on the object. Assume it was
// upserted by another manager and refuse to delete it. For the sake of
// the program's state, report that it has been deleted since we are no
// longer managing it.
return nil
}

err = client.Delete(c.Context, c.Name, deleteOpts)
if err != nil {
return nilIfGVKDeleted(err)
Expand Down
41 changes: 41 additions & 0 deletions provider/pkg/await/await_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -787,6 +787,34 @@ func TestDeletion(t *testing.T) {
}
}

field := []byte(`{
"f:metadata": {
"f:generateName": {},
"f:labels": {},
"f:ownerReferences": {}
}
}`)

original := validPodUnstructured.DeepCopy()
original.SetManagedFields([]metav1.ManagedFieldsEntry{{
Manager: "pulumi-kubernetes-123",
Operation: "Update",
FieldsType: "FieldsV1",
FieldsV1: &metav1.FieldsV1{
Raw: field,
},
}})

upserted := validPodUnstructured.DeepCopy()
upserted.SetManagedFields([]metav1.ManagedFieldsEntry{{
Manager: "pulumi-kubernetes-XYZ",
Operation: "Update",
FieldsType: "FieldsV1",
FieldsV1: &metav1.FieldsV1{
Raw: field,
},
}})

tests := []struct {
name string
client client
Expand Down Expand Up @@ -909,6 +937,19 @@ func TestDeletion(t *testing.T) {
condition: awaitNoop,
expect: []expectF{succeeded()},
},
{
name: "Field manager mismatch",
args: args{
resType: tokens.Type("kubernetes:core/v1:Pod"),
name: "foo",
objects: []runtime.Object{original},
inputs: original,
outputs: original,
serverSideApply: true,
},
condition: awaitNoop,
expect: []expectF{succeeded()},
},
}

for _, tt := range tests {
Expand Down
1 change: 1 addition & 0 deletions provider/pkg/await/condition/deleted.go
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,7 @@ func (dc *Deleted) Satisfied() (bool, error) {
}

uns := dc.Object()

r, _ := status.Compute(uns)
if r.Message != "" {
dc.logger.LogStatus(diag.Info, r.Message)
Expand Down
42 changes: 42 additions & 0 deletions provider/pkg/await/condition/deleted_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -191,4 +191,46 @@ func TestDeleted(t *testing.T) {
assert.False(t, done)
assert.Contains(t, buf.String(), "boom")
})

t.Run("fieldManager mismatch", func(t *testing.T) {
field := []byte(`{
"f:metadata": {
"f:generateName": {},
"f:labels": {},
"f:ownerReferences": {}
}
}`)

original := pod.DeepCopy()
original.SetManagedFields([]metav1.ManagedFieldsEntry{{
Manager: "pulumi-kubernetes-123",
Operation: "Update",
FieldsType: "FieldsV1",
FieldsV1: &metav1.FieldsV1{
Raw: field,
},
}})

upserted := pod.DeepCopy()
upserted.SetManagedFields([]metav1.ManagedFieldsEntry{{
Manager: "pulumi-kubernetes-XYZ",
Operation: "Update",
FieldsType: "FieldsV1",
FieldsV1: &metav1.FieldsV1{
Raw: field,
},
}})

ctx := context.Background()
getter := get404{}

cond, err := NewDeleted(ctx, Static(nil), getter, stdout, pod)
assert.NoError(t, err)

cond.Range(nil)

done, err := cond.Satisfied()
assert.NoError(t, err)
assert.True(t, done)
})
}
67 changes: 67 additions & 0 deletions tests/sdk/java/delete_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
package provider

import (
"context"
"testing"
"time"

"github.com/pulumi/providertest/pulumitest"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)

func TestDeleteDueToRename(t *testing.T) {
t.Parallel()
ctx := context.Background()
test := pulumitest.NewPulumiTest(t, "delete/rename")
t.Cleanup(func() {
test.Destroy()
})

test.Up()

// Change our Pod's resource name
test.UpdateSource("testdata/delete/rename/step2")
test.Up()

// Renaming the namespace should not have deleted it. Perform a refresh and
// make sure our pod is still running -- if it's not, Pulumi will have
// deleted it from our state.
refresh, err := test.CurrentStack().Refresh(ctx)
assert.NoError(t, err)
assert.NotContains(t, refresh.StdOut, "deleted", refresh.StdOut)
}

func TestDeletePatchResource(t *testing.T) {
t.Parallel()
ctx := context.Background()
test := pulumitest.NewPulumiTest(t, "testdata/delete/patch")
t.Cleanup(func() {
test.Destroy()
})

test.Up()

time.Sleep(60 * time.Second)

outputs, err := test.CurrentStack().Outputs(ctx)
require.NoError(t, err)

// The ConfigMap should have 3 managed fields -- one for the URN annotation
// and 2 for the patches.
mf, ok := outputs["managedFields"]
require.True(t, ok)
assert.Len(t, mf.Value, 3)

// Delete a patch.
test.UpdateSource("testdata/delete/patch/step2")
test.Up()

// One ConfigMapPatch should still be applied, plus a manager for our URN
// annotation.
outputs, err = test.CurrentStack().Outputs(ctx)
require.NoError(t, err)
mf, ok = outputs["managedFields"]
require.True(t, ok)
assert.Len(t, mf.Value, 2)
}
37 changes: 37 additions & 0 deletions tests/sdk/java/testdata/delete/patch/Pulumi.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
name: delete-patch-resource
runtime: yaml
description: |
Deleting a logical patch resource should not delete the underlying physical
resource.
outputs:
managedFields: ${patch2.metadata.managedFields}

resources:
configmap:
type: kubernetes:core/v1:ConfigMap
properties:
metadata:
name: patched-configmap

patch1:
type: kubernetes:core/v1:ConfigMapPatch
properties:
metadata:
name: patched-configmap
data:
foo: bar
options:
dependsOn:
- ${configmap}

patch2:
type: kubernetes:core/v1:ConfigMapPatch
properties:
metadata:
name: patched-configmap
data:
boo: baz
options:
dependsOn:
- ${patch1}
40 changes: 40 additions & 0 deletions tests/sdk/java/testdata/delete/patch/step2/Pulumi.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
name: delete-patch-resource
runtime: yaml
description: |
Deleting a logical patch resource should not delete the underlying physical
resource.
outputs:
managedFields: ${patch1.metadata.managedFields}

resources:
configmap:
type: kubernetes:core/v1:ConfigMap
properties:
metadata:
name: patched-configmap

patch1:
type: kubernetes:core/v1:ConfigMapPatch
properties:
metadata:
name: patched-configmap
data:
foo: bar
options:
dependsOn:
- ${configmap}

# Delete patch2 - the underlying ConfigMap should not be deleted, and patch1
# should still be applied.
#
# patch2:
# type: kubernetes:core/v1:ConfigMapPatch
# properties:
# metadata:
# name: patched-configmap
# data:
# boo: baz
# options:
# dependsOn:
# - ${patch1}
16 changes: 16 additions & 0 deletions tests/sdk/java/testdata/delete/rename/Pulumi.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
name: delete-with-rename
runtime: yaml
description: |
Changing a resource's name, but leaving .metadata untouched, should not
result in a deletion from the cluster.
resources:
pod:
type: kubernetes:core/v1:Pod
properties:
spec:
containers:
- image: nginx:1.14.2
name: nginx
ports:
- containerPort: 80
18 changes: 18 additions & 0 deletions tests/sdk/java/testdata/delete/rename/step2/Pulumi.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
name: delete-with-rename
runtime: yaml
description: |
Changing a resource's name, but leaving .metadata untouched, should not
result in a deletion from the cluster.
resources:
# Change the resource's name from "pod" to "mypod" but leave everything
# else the same.
mypod:
type: kubernetes:core/v1:Pod
properties:
spec:
containers:
- image: nginx:1.14.2
name: nginx
ports:
- containerPort: 80

0 comments on commit 0a9b5d5

Please sign in to comment.