Skip to content

feat(recipe): delete all resources belonging to a gvk from a particular namespace#101

Open
amitbhatt818 wants to merge 4 commits intomayadata-io:masterfrom
amitbhatt818:delete
Open

feat(recipe): delete all resources belonging to a gvk from a particular namespace#101
amitbhatt818 wants to merge 4 commits intomayadata-io:masterfrom
amitbhatt818:delete

Conversation

@amitbhatt818
Copy link
Contributor

@amitbhatt818 amitbhatt818 commented Jul 22, 2020 •
edited
Loading

Why We Need This PR?

  • We have feature to delete a particular Kubernetes Native resources.
  • But sometimes there are requirements to delete all the resources present in that particular namespace.
  • So this PR helps user to delete all the resources present in that particular namespaces.
  • This PR helps up to achieve this.

Some more Information:

Experiment YAML

apiVersion: dope.metacontroller.io/v1
kind: Recipe
metadata:
  name: delete-all-3
  namespace: d-testing
  labels:
    d-testing.dope.metacontroller.io/enabled: "true"
spec:
  tasks:
  - name: delete-all
    deleteAll:
      state:
        kind: Pod
        apiVersion: v1
        metadata:
          namespace: test
---

Result

Name:         delete-all-3
Namespace:    d-testing
Labels:       d-testing.dope.metacontroller.io/enabled=true
              recipe.dope.metacontroller.io/phase=Completed
Annotations:  <none>
API Version:  dope.metacontroller.io/v1
Kind:         Recipe
Metadata:
  Creation Timestamp:  2020-07-24T14:17:49Z
  Generation:          1
  Resource Version:    33969192
  Self Link:           /apis/metacontroller.app/v1/namespaces/d-testing/jobs/delete-all-3
  UID:                 12902e00-7132-4064-8cc4-70bb3646d591
Spec:
  Tasks:
    Delete All:
      State:
        API Version:  v1
        Kind:         Pod
        Metadata:
          Namespace:  test
    Name:             delete-all
Status:
  Failed Task Count:  0
  Message:            
  Phase:              Completed
  Reason:             
  Task Count:         1
  Task List Status:
    Delete - All:
      Message:  DeleteAll: Resource test v1: GVK Pod
      Phase:    Passed
      Step:     1
    delete-all-3-lock:
      Internal:  true
      Message:   Create: Lock d-testing delete-all-3-lock: GVK /v1, Kind=ConfigMap
      Phase:     Passed
      Step:      0
    delete-all-3-unlock:
      Internal:  true
      Message:   Locked forever
      Phase:     Passed
      Step:      3
    Job - Elapsed - Time:
      Elapsed Time In Seconds:  0.03936893
      Internal:                 true
      Phase:                    Passed
      Step:                     2
Events:                         <none>

Signed-off-by: Amit Bhatt amitbhatt818@gmail.com

@amitbhatt818 amitbhatt818 self-assigned this Jul 22, 2020
AmitKumarDas
AmitKumarDas reviewed Jul 23, 2020
View reviewed changes
pkg/job/delete_all.go Outdated
@@ -0,0 +1,49 @@
package job
Copy link

@AmitKumarDas AmitKumarDas Jul 23, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

@amitbhatt818 One of the first things that I would do is to explain what this PR is all about in the PR body.
It should explain everything to a reviewer to let this reviewer help as much as possible.
Note that a reviewer may be a maintainer or a new contributor to this project. The levels of knowledge & expertise will vary. We should put all effort to explain things in a manner it can be understood by all these personas.

Can you rebase from master?

Copy link
Contributor Author

@amitbhatt818 amitbhatt818 Jul 25, 2020

Choose a reason for hiding this comment

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

Added description in the PR body. And rebased the branch.

AmitKumarDas
AmitKumarDas reviewed Jul 23, 2020
View reviewed changes
pkg/job/delete_all.go Outdated

func (r *TaskRunner) deleteAll() (*types.TaskStatus, error) {
var message = fmt.Sprintf(
"Delete: Resource %s %s: GVK %s",
Copy link

@AmitKumarDas AmitKumarDas Jul 23, 2020

Choose a reason for hiding this comment

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

The message should start as DeleteAll: ...

Copy link
Contributor Author

@amitbhatt818 amitbhatt818 Jul 25, 2020

Choose a reason for hiding this comment

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

Done

AmitKumarDas
AmitKumarDas reviewed Jul 23, 2020
View reviewed changes
pkg/job/delete_all.go
"openebs.io/metac/dynamic/clientset"
)

func (r *TaskRunner) deleteAll() (*types.TaskStatus, error) {
Copy link

@AmitKumarDas AmitKumarDas Jul 23, 2020

Choose a reason for hiding this comment

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

Do you think this should have a dedicated struct. For example:

type Deleting struct {}
func NewDeleter(config DeletingConfig) *Deleting {}
func (d *Deleting) DeleteAll() {}
func (d *Deleting) Delete() {}

@AmitKumarDas AmitKumarDas changed the title (feat):Delete all resources present in particular namespace feat: delete all resources belonging to a gvk from a particular namespace Jul 23, 2020
AmitKumarDas
AmitKumarDas reviewed Jul 23, 2020
View reviewed changes
pkg/job/delete_all.go Outdated
err = client.
Namespace(r.Task.DeleteAll.State.GetNamespace()).
Delete(
r.Task.DeleteAll.State.GetKind(),
Copy link

@AmitKumarDas AmitKumarDas Jul 23, 2020

Choose a reason for hiding this comment

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

Delete does not take kind as an argument. It takes Name.
In this case we should use DeleteAll or DeleteCollection method.

Copy link
Contributor Author

@amitbhatt818 amitbhatt818 Jul 25, 2020

Choose a reason for hiding this comment

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

Thanks 👍

AmitKumarDas
AmitKumarDas reviewed Jul 23, 2020
View reviewed changes
pkg/job/task.go
r.tryRunAssert,
r.tryRunDelete,
r.tryRunApply,
r.tryRunDeleteAll,
Copy link

@AmitKumarDas AmitKumarDas Jul 23, 2020

Choose a reason for hiding this comment

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

can you rearrange tryRunDeleteAll just after tryRunDelete

Copy link
Contributor Author

@amitbhatt818 amitbhatt818 Jul 25, 2020

Choose a reason for hiding this comment

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

Resolved this comment

AmitKumarDas
AmitKumarDas reviewed Jul 23, 2020
View reviewed changes
types/job/delete_all.go Outdated
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
)

// DeleteAll deletes the state found in the cluster
Copy link

@AmitKumarDas AmitKumarDas Jul 23, 2020

Choose a reason for hiding this comment

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

can you explain this better

Copy link
Contributor Author

@amitbhatt818 amitbhatt818 Jul 25, 2020

Choose a reason for hiding this comment

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

Changed the comment

AmitKumarDas
AmitKumarDas reviewed Jul 23, 2020
View reviewed changes
pkg/job/delete_all.go
"openebs.io/metac/dynamic/clientset"
)

func (r *TaskRunner) deleteAll() (*types.TaskStatus, error) {
Copy link

@AmitKumarDas AmitKumarDas Jul 23, 2020

Choose a reason for hiding this comment

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

Please add Unit Tests for all the changes or additions.

Copy link
Contributor Author

@amitbhatt818 amitbhatt818 Jul 25, 2020

Choose a reason for hiding this comment

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

I will add unit test for these all changes In my coming PR @AmitKumarDas

@amitbhatt818
Delete all resources present in particular namespace
37fc661 
Signed-off-by: Amit Bhatt <amitbhatt818@gmail.com>
@amitbhatt818
Changed methord name
64cbb60 
Signed-off-by: Amit Bhatt <amitbhatt818@gmail.com>
@amitbhatt818
Changed job to recipe
c393864 
Signed-off-by: Amit Bhatt <amitbhatt818@gmail.com>
@amitbhatt818
Added comments
c13b5ac 
Signed-off-by: Amit Bhatt <amitbhatt818@gmail.com>
@amitbhatt818 amitbhatt818 marked this pull request as ready for review July 25, 2020 14:05
@AmitKumarDas AmitKumarDas changed the title feat: delete all resources belonging to a gvk from a particular namespace feat(recipe): delete all resources belonging to a gvk from a particular namespace Aug 5, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@harshshekhar15 harshshekhar15 Awaiting requested review from harshshekhar15

1 more reviewer

@AmitKumarDas AmitKumarDas AmitKumarDas left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

@amitbhatt818 amitbhatt818

Labels

delete feature flag recipe use case

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@amitbhatt818 @AmitKumarDas