add kumactl delete command #343

pradeepmurugesan · 2019-10-15T03:42:51Z

Summary

Implemented the support for delete command. The delete command takes the mesh name as a parameter.
Check if the the given mesh name exists and then delete.

returns an error in case of no mesh with the given name.

Full changelog

Implemented the delete command in kumactl

Issues resolved

Fix #279

yskopets

@pradeepmurugesan Hi!

Thanks a lot for your first contribution to Kuma!

I've left some comments to give you an idea what to expect from code review.

Looking forward to the next step!

app/kumactl/cmd/root.go

app/kumactl/cmd/delete/delete.go

app/kumactl/cmd/delete/say_hello.go

app/kumactl/cmd/delete/delete_meshes.go

app/kumactl/cmd/delete/say_hello.go

app/kumactl/cmd/delete/delete_meshes.go

yskopets

@pradeepmurugesan I like the progress!

Let's fix these minor comments first.

Next, I'd like us to try to have a single delete command for resources of all types.

app/kumactl/cmd/delete/delete_meshes.go

app/kumactl/cmd/delete/delete_meshes_test.go

pradeepmurugesan · 2019-10-16T17:02:34Z

@yskopets thanks for reviewing.. Fixed the comments. 👍 Kindly review..

yskopets

A few minor changes

app/kumactl/cmd/delete/delete_meshes_test.go

app/kumactl/cmd/delete/delete_meshes.go

app/kumactl/cmd/delete/delete_meshes_test.go

yskopets

@pradeepmurugesan Perfect!

It's a great milestone!

Next, let's try to implement a single kumactl delete command for all resource types.

We will reuse the logic you've already implemented and parametrise it with a resource type.

let's have kubectl delete to accept 2 args: <type> and <name>
let's validate <type> against a well-known list (mesh, dataplane, proxytemplate, traffic-log, traffic-permission)
next, we need to map <type> (from command line) onto a respective Kuma Resource (i.e., MeshResource, TrafficLogResource, etc)
- the simplest solution would be to have a switch statement (i.e. case "mesh": return &mesh.MeshResource{} ) - it's ok to start from it
- better approach, is to reuse registry if all supported Resources (pkg/core/resources/registry.Global())
after that, we can pass Resource instance as a parameter to delete logic

I hope this clarifies what kind of solution we would like to have in the end.

Thanks again for your time and willingness to contribute to Kuma!

pradeepmurugesan · 2019-10-17T07:16:03Z

@yskopets refactored for the existing functionality. deleting the mesh.

I feel like implementing the other options like dataplane, proxytemplate, traffic-log, traffic-permission once we sort out the design with mesh. Let me know if this is what you had in mind

yskopets

@pradeepmurugesan Great!

Please go ahead with the remaining resource types.

app/kumactl/cmd/delete/delete.go

yskopets

👍Almost there!

Just a few final changes.

app/kumactl/cmd/delete/delete.go

app/kumactl/cmd/delete/delete_dataplane_test.go

Implemented the support for delete command. The delete command takes the mesh name as a parameter. Check if the the given mesh name exists and then delete. * returns an error in case of no mesh with the given name. Fix kumahq#279

…all resource types Right now the design is to handle the resource types via subcommands. Refactored so that the delete command accepts a resourceType arg. Based on the passed resource type arg the corresponding resource will be deleted. * returns an error in case of no mesh with the given name. Fix kumahq#279

Implemented the support for deleting dataplane. Check if the the given dataplane exists and thdataplane. * returns an error in case of no dataplane with the given name. Fix kumahq#279

Implemented the support for deleting proxytemplate. Check if the the given proxytemplate exists and then delete. * returns an error in case of no proxytemplate with the given name. Fix kumahq#279

Implemented the support for deleting traffic-log. Check if the the given traffic-log exists and then delete. * returns an error in case of no traffic-log with the given name. Fix kumahq#279

Implemented the support for deleting trafficpermission. Check if the the given trafficpermission exists and then delete. * returns an error in case of no trafficpermission with the given name. Fix kumahq#279

yskopets

Perfect! 👏

I'm asking for 2 more minor changes, but mostly to trigger CI build on your next commit. After that, it's ready to be merged!

Thanks a lot for your contribution!

Expect a nice Kuma T-shirt shortly! 🎉

yskopets · 2019-10-22T21:25:57Z

app/kumactl/cmd/delete/delete.go

+
+func NewDeleteCmd(pctx *kumactl_cmd.RootContext) *cobra.Command {
+	cmd := &cobra.Command{
+		Use:   "delete",


Let's change Use: "delete" to Use: "delete TYPE NAME"

yskopets · 2019-10-22T21:28:13Z

app/kumactl/cmd/delete/delete.go

+				resourceType = mesh.TrafficPermissionType
+
+			default:
+				return errors.Errorf("unknown resource type: %s. Allowed types: mesh, dataplane, proxytemplate, traffic-log, traffic-permission", resourceTypeArg)


Let's slightly change this error message to keep it in sync with Use, namely

unknown TYPE: %s. Allowed values: mesh, dataplane, proxytemplate, traffic-log, traffic-permission

pradeepmurugesan mentioned this pull request Oct 15, 2019

Add kumactl delete command #279

Closed

yskopets suggested changes Oct 15, 2019

View reviewed changes

pradeepmurugesan force-pushed the kuma-delete branch from 4f5d20c to 473234d Compare October 15, 2019 17:21

pradeepmurugesan changed the title ~~initial commit~~ add kumactl delete command Oct 15, 2019

pradeepmurugesan force-pushed the kuma-delete branch from 473234d to fefcbf1 Compare October 16, 2019 06:33

pradeepmurugesan marked this pull request as ready for review October 16, 2019 06:33

pradeepmurugesan requested a review from yskopets October 16, 2019 06:33

pradeepmurugesan force-pushed the kuma-delete branch from fefcbf1 to 387ff9a Compare October 16, 2019 06:38

yskopets reviewed Oct 16, 2019

View reviewed changes

pradeepmurugesan force-pushed the kuma-delete branch 2 times, most recently from a123085 to 5f2cad4 Compare October 16, 2019 16:57

pradeepmurugesan requested a review from yskopets October 16, 2019 17:01

yskopets reviewed Oct 16, 2019

View reviewed changes

pradeepmurugesan force-pushed the kuma-delete branch from 5f2cad4 to ae78bee Compare October 16, 2019 17:36

pradeepmurugesan requested a review from yskopets October 16, 2019 17:37

yskopets reviewed Oct 16, 2019

View reviewed changes

yskopets assigned pradeepmurugesan Oct 16, 2019

yskopets added the kumactl label Oct 16, 2019

yskopets added this to the 0.3.0 milestone Oct 16, 2019

pradeepmurugesan requested a review from yskopets October 17, 2019 07:09

yskopets reviewed Oct 17, 2019

View reviewed changes

app/kumactl/cmd/delete/delete.go Outdated Show resolved Hide resolved

app/kumactl/cmd/delete/delete.go Outdated Show resolved Hide resolved

pradeepmurugesan force-pushed the kuma-delete branch from 41ba22a to f505a47 Compare October 17, 2019 17:39

pradeepmurugesan requested a review from yskopets October 17, 2019 18:45

yskopets reviewed Oct 17, 2019

View reviewed changes

pradeepmurugesan requested a review from yskopets October 18, 2019 03:22

pradeepmurugesan added 3 commits October 22, 2019 08:13

feat(kumactl) add kumactl delete command

ff3dc9f

Implemented the support for delete command. The delete command takes the mesh name as a parameter. Check if the the given mesh name exists and then delete. * returns an error in case of no mesh with the given name. Fix kumahq#279

feat(kumactl) add support for deleting dataplane

64bac30

Implemented the support for deleting dataplane. Check if the the given dataplane exists and thdataplane. * returns an error in case of no dataplane with the given name. Fix kumahq#279

pradeepmurugesan added 5 commits October 22, 2019 08:13

feat(kumactl) add support for deleting proxytemplate

8c2a042

Implemented the support for deleting proxytemplate. Check if the the given proxytemplate exists and then delete. * returns an error in case of no proxytemplate with the given name. Fix kumahq#279

feat(kumactl) add support for deleting traffic-log

e8d570b

Implemented the support for deleting traffic-log. Check if the the given traffic-log exists and then delete. * returns an error in case of no traffic-log with the given name. Fix kumahq#279

feat(kumactl) add support for deleting trafficpermission

39548e1

Implemented the support for deleting trafficpermission. Check if the the given trafficpermission exists and then delete. * returns an error in case of no trafficpermission with the given name. Fix kumahq#279

fix(kumactl) fix formatting issues

2468a3c

fix(kumactl) extracted the current mesh from --mesh option

491d978

pradeepmurugesan force-pushed the kuma-delete branch from 0d6d7f2 to 491d978 Compare October 22, 2019 02:43

yskopets reviewed Oct 22, 2019

View reviewed changes

fix(kumactl): changed the usage and error message

edbe0ad

yskopets approved these changes Oct 23, 2019

View reviewed changes

yskopets merged commit 4798a1f into kumahq:master Oct 23, 2019

pradeepmurugesan deleted the kuma-delete branch October 23, 2019 05:52

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add kumactl delete command #343

add kumactl delete command #343

pradeepmurugesan commented Oct 15, 2019 •

edited

Loading

yskopets left a comment

yskopets left a comment •

edited

Loading

pradeepmurugesan commented Oct 16, 2019

yskopets left a comment

yskopets left a comment •

edited

Loading

pradeepmurugesan commented Oct 17, 2019

yskopets left a comment •

edited

Loading

yskopets left a comment

yskopets left a comment

yskopets Oct 22, 2019

yskopets Oct 22, 2019

add kumactl delete command #343

add kumactl delete command #343

Conversation

pradeepmurugesan commented Oct 15, 2019 • edited Loading

Summary

Full changelog

Issues resolved

yskopets left a comment

Choose a reason for hiding this comment

yskopets left a comment • edited Loading

Choose a reason for hiding this comment

pradeepmurugesan commented Oct 16, 2019

yskopets left a comment

Choose a reason for hiding this comment

yskopets left a comment • edited Loading

Choose a reason for hiding this comment

pradeepmurugesan commented Oct 17, 2019

yskopets left a comment • edited Loading

Choose a reason for hiding this comment

yskopets left a comment

Choose a reason for hiding this comment

yskopets left a comment

Choose a reason for hiding this comment

yskopets Oct 22, 2019

Choose a reason for hiding this comment

yskopets Oct 22, 2019

Choose a reason for hiding this comment

pradeepmurugesan commented Oct 15, 2019 •

edited

Loading

yskopets left a comment •

edited

Loading

yskopets left a comment •

edited

Loading

yskopets left a comment •

edited

Loading