Skip to content
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.

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

Checking the UID to confirm the deletion of resource #296

Merged
merged 10 commits into from
Sep 8, 2021
4 changes: 3 additions & 1 deletion pkg/kapp/clusterapply/delete_change.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,9 @@ func (c DeleteChange) IsDoneApplying() (ctlresm.DoneApplyState, []string, error)

// it should not matter if change is ignored or not
// because it should be deleted eventually anyway (thru GC)
exists, err := c.identifiedResources.Exists(res)
// We should check for the UID check because of the following bug:
// https://github.com/vmware-tanzu/carvel-kapp/issues/229
exists, err := c.identifiedResources.Exists(res, "SameUID")
if err != nil {
return ctlresm.DoneApplyState{}, nil, err
}
Expand Down
4 changes: 2 additions & 2 deletions pkg/kapp/resources/identified_resources.go
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ func (r IdentifiedResources) Get(resource Resource) (Resource, error) {
return resource, nil
}

func (r IdentifiedResources) Exists(resource Resource) (bool, error) {
func (r IdentifiedResources) Exists(resource Resource, existsOpts ...string) (bool, error) {
defer r.logger.DebugFunc(fmt.Sprintf("Exists(%s)", resource.Description())).Finish()
return r.resources.Exists(resource)
return r.resources.Exists(resource, existsOpts...)
}
6 changes: 3 additions & 3 deletions pkg/kapp/resources/identified_resources_list_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,9 +56,9 @@ metadata:

return []ctlres.Resource{antreaRes, deploymentRes}, nil
}
func (r *FakeResources) Delete(ctlres.Resource) error { return nil }
func (r *FakeResources) Exists(ctlres.Resource) (bool, error) { return true, nil }
func (r *FakeResources) Get(ctlres.Resource) (ctlres.Resource, error) { return nil, nil }
func (r *FakeResources) Delete(ctlres.Resource) error { return nil }
func (r *FakeResources) Exists(ctlres.Resource, ...string) (bool, error) { return true, nil }
func (r *FakeResources) Get(ctlres.Resource) (ctlres.Resource, error) { return nil, nil }
func (r *FakeResources) Patch(ctlres.Resource, types.PatchType, []byte) (ctlres.Resource, error) {
return nil, nil
}
Expand Down
24 changes: 21 additions & 3 deletions pkg/kapp/resources/resources.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ const (
type Resources interface {
All([]ResourceType, AllOpts) ([]Resource, error)
Delete(Resource) error
Exists(Resource) (bool, error)
Exists(Resource, ...string) (bool, error)
Get(Resource) (Resource, error)
Patch(Resource, types.PatchType, []byte) (Resource, error)
Update(Resource) (Resource, error)
Expand Down Expand Up @@ -362,7 +362,7 @@ func (c *ResourcesImpl) Get(resource Resource) (Resource, error) {
return NewResourceUnstructured(*item, resType), nil
}

func (c *ResourcesImpl) Exists(resource Resource) (bool, error) {
func (c *ResourcesImpl) Exists(resource Resource, existsOpts ...string) (bool, error) {
rohitagg2020 marked this conversation as resolved.
Show resolved Hide resolved
if resourcesDebug {
t1 := time.Now().UTC()
defer func() { c.logger.Debug("exists %s", time.Now().UTC().Sub(t1)) }()
Expand All @@ -381,7 +381,7 @@ func (c *ResourcesImpl) Exists(resource Resource) (bool, error) {
var found bool
rohitagg2020 marked this conversation as resolved.
Show resolved Hide resolved

err = util.Retry(time.Second, time.Minute, func() (bool, error) {
_, err = resClient.Get(context.TODO(), resource.Name(), metav1.GetOptions{})
fetchedRes, err := resClient.Get(context.TODO(), resource.Name(), metav1.GetOptions{})
if err != nil {
if errors.IsNotFound(err) {
found = false
Expand All @@ -401,6 +401,24 @@ func (c *ResourcesImpl) Exists(resource Resource) (bool, error) {
return isDone, c.resourceErr(err, "Checking existence of", resource)
}

if len(existsOpts) != 0 {
for _, existsOpt := range existsOpts {
switch existsOpt {
case "SameUID":
// If fetchedRes(i.e. resource from K8s) is not null and its UID didn't match with the
// UID of resource we are trying to delete, then it means resource has been deleted
// successfully.
if fetchedRes != nil && resource.UID() != "" {
if string(fetchedRes.GetUID()) != resource.UID() {
found = false
return true, nil
}
}
}
rohitagg2020 marked this conversation as resolved.
Show resolved Hide resolved

}
}

found = true
return true, nil
})
Expand Down