Skip to content

Commit

Permalink
fix: report NotFound status for deleted KCC resources (#2689)
Browse files Browse the repository at this point in the history
* fix: report NotFound status for deleted KCC resources

The current custom StatusReader for Config Connector resources will
report Unknown status when a Config Connector resource is not found (aka
deleted). This causes the `kpt live destroy` reconcile loop to run
forever since it expects a NotFound status to end. This commit ensures
that deleted resources report a NotFound status instead.

* refactor: Fix linting issue for unkeyed fields in composite literal
  • Loading branch information
rquitales committed Jan 31, 2022
1 parent ef1a189 commit 0f3db0c
Show file tree
Hide file tree
Showing 2 changed files with 28 additions and 0 deletions.
4 changes: 4 additions & 0 deletions pkg/status/configconnector.go
Expand Up @@ -20,6 +20,7 @@ import (
"strings"

v1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apimachinery/pkg/api/meta"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
"k8s.io/apimachinery/pkg/runtime/schema"
Expand Down Expand Up @@ -58,6 +59,9 @@ func (c *ConfigConnectorStatusReader) ReadStatus(ctx context.Context, reader eng
u.SetGroupVersionKind(gvk)
err = reader.Get(ctx, key, &u)
if err != nil {
if errors.IsNotFound(err) {
return newResourceStatus(id, status.NotFoundStatus, &u, "Resource not found")
}
return newUnknownResourceStatus(id, nil, err)
}

Expand Down
24 changes: 24 additions & 0 deletions pkg/status/configconnector_test.go
Expand Up @@ -5,6 +5,7 @@ import (
"testing"

"github.com/stretchr/testify/assert"
"k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apimachinery/pkg/runtime/schema"
"sigs.k8s.io/cli-utils/pkg/kstatus/polling/testutil"
"sigs.k8s.io/cli-utils/pkg/kstatus/status"
Expand Down Expand Up @@ -53,6 +54,7 @@ func TestReadStatus(t *testing.T) {
resource string
gvk schema.GroupVersionKind
expectedStatus status.Status
deleted bool
}{
"Resource with deletionTimestap is Terminating": {
resource: `
Expand Down Expand Up @@ -117,6 +119,22 @@ status:
},
expectedStatus: status.FailedStatus,
},

"Resource has been deleted": {
resource: `
apiVersion: storage.cnrm.cloud.google.com/v1beta1
kind: StorageBucket
metadata:
name: fake-bucket
`,
gvk: schema.GroupVersionKind{
Group: "storage.cnrm.cloud.google.com",
Version: "v1beta1",
Kind: "StorageBucket",
},
expectedStatus: status.NotFoundStatus,
deleted: true,
},
}

for tn, tc := range testCases {
Expand All @@ -126,6 +144,12 @@ status:
fakeClusterReader := &fakeClusterReader{
getResource: obj,
}
// Return not found error if we want the resource to be deleted.
if tc.deleted {
fakeClusterReader.getResource = nil
fakeClusterReader.getErr = errors.NewNotFound(schema.GroupResource{Group: tc.gvk.Group, Resource: tc.gvk.Kind}, "fake-name")
}

fakeMapper := fakemapper.NewFakeRESTMapper(tc.gvk)
statusReader := &ConfigConnectorStatusReader{
Mapper: fakeMapper,
Expand Down

0 comments on commit 0f3db0c

Please sign in to comment.