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

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

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

amitbhatt818
Copy link
Contributor

@amitbhatt818 amitbhatt818 commented Jul 22, 2020

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
@@ -0,0 +1,49 @@
package job
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.

@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

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.


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

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

Choose a reason for hiding this comment

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

Done

"openebs.io/metac/dynamic/clientset"
)

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

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
err = client.
Namespace(r.Task.DeleteAll.State.GetNamespace()).
Delete(
r.Task.DeleteAll.State.GetKind(),

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

Choose a reason for hiding this comment

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

Thanks 👍

pkg/job/task.go Outdated
@@ -256,6 +265,7 @@ func (r *TaskRunner) Run() (types.TaskStatus, error) {
r.tryRunAssert,
r.tryRunDelete,
r.tryRunApply,
r.tryRunDeleteAll,

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

Choose a reason for hiding this comment

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

Resolved this comment

"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
)

// DeleteAll deletes the state found in the cluster

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

Choose a reason for hiding this comment

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

Changed the comment

"openebs.io/metac/dynamic/clientset"
)

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

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

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

Signed-off-by: Amit Bhatt <amitbhatt818@gmail.com>
Signed-off-by: Amit Bhatt <amitbhatt818@gmail.com>
Signed-off-by: Amit Bhatt <amitbhatt818@gmail.com>
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants