[Task] Add namespace crud #227

sufuf3 · 2018-08-08T03:05:09Z

I finished Namespace CRUD, please help me review this PR.
Thank you.

codecov-io · 2018-08-08T09:30:16Z

Codecov Report

Merging #227 into develop will decrease coverage by 0.25%.
The diff coverage is 73.75%.

@@             Coverage Diff             @@
##           develop     #227      +/-   ##
===========================================
- Coverage    78.46%   78.21%   -0.26%     
===========================================
  Files           50       53       +3     
  Lines         2498     2639     +141     
===========================================
+ Hits          1960     2064     +104     
- Misses         407      437      +30     
- Partials       131      138       +7

Impacted Files	Coverage Δ
src/namespace/namespace.go	`100% <100%> (ø)`
src/server/handler_namespace.go	`68.51% <68.51%> (ø)`
src/kubernetes/namespaces.go	`86.66% <86.66%> (ø)`
src/server/route.go	`83.67% <88.88%> (+0.52%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 08742c4...6936c7d. Read the comment docs.

John-Lin · 2018-08-08T10:09:43Z

src/entity/namespace.go

+
+// the const for NamespaceCollectionName
+const (
+	NamespaceCollectionName string = "namespace"


namespaces

John-Lin · 2018-08-08T10:41:24Z

src/entity/namespace.go

+type Namespace struct {
+	ID        bson.ObjectId     `bson:"_id,omitempty" json:"id" validate:"-"`
+	Name      string            `bson:"name" json:"name" validate:"required,k8sname"`
+	Labels    map[string]string `bson:"labels,omitempty" json:"labels" validate:"required,dive,keys,alphanum,endkeys,required,alphanum"`


I think we don't need labels here

John-Lin · 2018-08-08T10:46:32Z

src/kubernetes/namespaces.go

+
+// DeleteNamespace will delete the namespace by the namespace name
+func (kc *KubeCtl) DeleteNamespace(name string) error {
+	deletePolicy := metav1.DeletePropagationBackground


Do we need DeletePropagationBackground option here?

I referred to https://github.com/kubernetes/kubernetes/blob/master/pkg/controller/namespace/deletion/namespaced_resources_deleter.go#L340-L344
IMO, maybe it's good to need?
What do you think?

I think we should use Foreground waiting for all resource to be killed. see https://godoc.org/k8s.io/apimachinery/pkg/apis/meta/v1#DeletionPropagation

The object exists in the key-value store until the garbage collector deletes all the dependents whose ownerReference.blockOwnerDeletion=true from the key-value store. API sever will put the "foregroundDeletion" finalizer on the object, and sets its deletionTimestamp. This policy is cascading, i.e., the dependents will be deleted with Foreground.

John-Lin · 2018-08-09T18:13:01Z

src/kubernetes/namespaces.go

+
+// DeleteNamespace will delete the namespace by the namespace name
+func (kc *KubeCtl) DeleteNamespace(name string) error {
+	deletePolicy := metav1.DeletePropagationBackground


I think we should use Foreground waiting for all resource to be killed. see https://godoc.org/k8s.io/apimachinery/pkg/apis/meta/v1#DeletionPropagation

The object exists in the key-value store until the garbage collector deletes all the dependents whose ownerReference.blockOwnerDeletion=true from the key-value store. API sever will put the "foregroundDeletion" finalizer on the object, and sets its deletionTimestamp. This policy is cascading, i.e., the dependents will be deleted with Foreground.

John-Lin · 2018-08-09T18:16:50Z

src/server/handler_namespace.go

+
+	if err := namespace.CreateNamespace(sp, &n); err != nil {
+		if errors.IsAlreadyExists(err) {
+			response.Conflict(req.Request, resp.ResponseWriter, fmt.Errorf("Namespace Name: %s already existed", n.Name))


Namespace: %s already existed

John-Lin · 2018-08-09T18:17:51Z

src/server/handler_namespace.go

+	}
+
+	if err := namespace.DeleteNamespace(sp, &n); err != nil {
+		if errors.IsAlreadyExists(err) {


if errors.IsNotFound(err) {

John-Lin · 2018-08-13T06:09:00Z

@sufuf3 Please add namespace api usage in API.md

To support CRUD of namespace, update src/server/route.go & add newNamespaceService.

Ref: https://raw.githubusercontent.com/kubernetes/website/master/content/en/examples/admin/namespace-dev.json ``` { "kind": "Namespace", "apiVersion": "v1", "metadata": { "name": "development", "labels": { "name": "development" } } } ```

Use Foreground waiting for all resource to be killed. Ref: - https://kubernetes.io/docs/concepts/workloads/controllers/garbage-collection/#controlling-how-the-garbage-collector-deletes-dependents - https://godoc.org/k8s.io/apimachinery/pkg/apis/meta/v1#DeletionPropagation

sufuf3 · 2018-08-13T14:37:57Z

@John-Lin I added namespace api usage in API.md and modified the files.
Please help me review this PR. Thank you.

John-Lin · 2018-08-13T15:00:52Z

API.md

+}
+```
+
+Response Data:


response data should be the namespace entity not create success message

I copied from service, sorry that I forgot to change the response data.
Updated it.

WakeupTsai · 2018-08-14T03:01:22Z

src/namespace/namespace.go

+	return err
+}
+
+// DeleteNamespace willl delete namespace


小錯字 will

sufuf3 force-pushed the sufuf3/namespace-crud branch 4 times, most recently from 0e15586 to 647745b Compare August 8, 2018 09:20

sufuf3 changed the title ~~[WIP] Add namespace crud~~ Add namespace crud Aug 8, 2018

sufuf3 force-pushed the sufuf3/namespace-crud branch from 647745b to b14d505 Compare August 8, 2018 09:30

sufuf3 requested review from hwchiu, John-Lin, chenyunchen and WakeupTsai August 8, 2018 09:41

John-Lin suggested changes Aug 8, 2018

View reviewed changes

John-Lin reviewed Aug 8, 2018

View reviewed changes

John-Lin suggested changes Aug 9, 2018

View reviewed changes

sufuf3 force-pushed the sufuf3/namespace-crud branch 10 times, most recently from 7fe5b98 to b934625 Compare August 13, 2018 05:05

sufuf3 added 3 commits August 13, 2018 22:15

Add newNamespaceService in server/route

2c23149

To support CRUD of namespace, update src/server/route.go & add newNamespaceService.

Add namespace entity for server handler

2d2a80a

Ref: https://raw.githubusercontent.com/kubernetes/website/master/content/en/examples/admin/namespace-dev.json ``` { "kind": "Namespace", "apiVersion": "v1", "metadata": { "name": "development", "labels": { "name": "development" } } } ```

Add src/kubernetes/namespaces for kubeCtl

33829fc

sufuf3 added 5 commits August 13, 2018 22:15

Add src/namespace/ for server handler package

d82acfc

Add namespace server handler

3b59ace

Remove labels of namespace

020e3f3

Fix CI test fail

760aee7

sufuf3 force-pushed the sufuf3/namespace-crud branch from b934625 to 77e6bf7 Compare August 13, 2018 14:25

sufuf3 requested a review from John-Lin August 13, 2018 14:35

John-Lin reviewed Aug 13, 2018

View reviewed changes

Add Namespace CRUD in API.md

76d3196

sufuf3 force-pushed the sufuf3/namespace-crud branch from 77e6bf7 to 76d3196 Compare August 13, 2018 16:19

John-Lin approved these changes Aug 14, 2018

View reviewed changes

WakeupTsai reviewed Aug 14, 2018

View reviewed changes

WakeupTsai approved these changes Aug 14, 2018

View reviewed changes

Update namespace.go

6936c7d

John-Lin changed the title ~~Add namespace crud~~ [Task] Add namespace crud Aug 14, 2018

John-Lin merged commit 0342e92 into develop Aug 14, 2018

sufuf3 deleted the sufuf3/namespace-crud branch August 14, 2018 07:40

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Task] Add namespace crud #227

[Task] Add namespace crud #227

sufuf3 commented Aug 8, 2018 •

edited

Loading

codecov-io commented Aug 8, 2018 •

edited

Loading

John-Lin Aug 8, 2018

John-Lin Aug 8, 2018

John-Lin Aug 8, 2018

sufuf3 Aug 8, 2018

John-Lin Aug 9, 2018 •

edited

Loading

John-Lin Aug 9, 2018 •

edited

Loading

John-Lin Aug 9, 2018

John-Lin Aug 9, 2018

John-Lin commented Aug 13, 2018

sufuf3 commented Aug 13, 2018

John-Lin Aug 13, 2018

sufuf3 Aug 13, 2018

WakeupTsai Aug 14, 2018

John-Lin Aug 14, 2018

[Task] Add namespace crud #227

[Task] Add namespace crud #227

Conversation

sufuf3 commented Aug 8, 2018 • edited Loading

codecov-io commented Aug 8, 2018 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

John-Lin Aug 9, 2018 • edited Loading

Choose a reason for hiding this comment

John-Lin Aug 9, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

John-Lin commented Aug 13, 2018

sufuf3 commented Aug 13, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sufuf3 commented Aug 8, 2018 •

edited

Loading

codecov-io commented Aug 8, 2018 •

edited

Loading

John-Lin Aug 9, 2018 •

edited

Loading

John-Lin Aug 9, 2018 •

edited

Loading