support namespace in each service #137

196Ikuchil · 2022-03-25T15:36:38Z

What type of PR is this?

/kind bug

What this PR does / why we need it:

This PR allows us to specify namespace on manipulation of Pod and PVC.

In PR #111, we implemented the feature to import from the existing cluster.
But it is only for the default namespace default.
Therefore, we support the different namespace in import/export.

Which issue(s) this PR fixes:

Fixes #116

Special notes for your reviewer:

In PR #127
Since in the WebUI calls the apiserver directly, in this issue, I didn't change that logic. But we must specify a namespace to support this PR's changes. So, I specified a default namespace in Pod and PVC's handler.

/label tide/merge-method-squash
/assign @sanposhiho

k8s-ci-robot · 2022-03-25T15:36:47Z

Hi @196Ikuchil. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

sanposhiho

Looks good overall 👍

export/export.go

sanposhiho · 2022-03-28T16:34:08Z

server/handler/pod.go

+// In PR #137, we require the specification of namespace on pod manipulation.
+// But this handler will be deprecate in the future.
+// So, this is a temporary solution.


Suggested change

// In PR #137, we require the specification of namespace on pod manipulation.

// But this handler will be deprecate in the future.

// So, this is a temporary solution.

// Handlers support only default namespace.

// The previous web UI had supported only default namespace,

// and we deprecated all handlers to support namespaces in Web UI.

sanposhiho · 2022-03-28T16:35:15Z

/ok-to-test

sanposhiho · 2022-03-28T16:45:17Z

/retitle support namespace in each service

sanposhiho · 2022-03-28T16:46:16Z

server/di/interface.go

+	Get(ctx context.Context, name string, namespace string) (*corev1.Pod, error)
+	List(ctx context.Context, namespace string) (*corev1.PodList, error)
+	Apply(ctx context.Context, pod *configv1.PodApplyConfiguration, namespace string) (*corev1.Pod, error)
+	Delete(ctx context.Context, name string, namespace string) error
 	DeleteCollection(ctx context.Context, lopts metav1.ListOptions) error


DeleteCollection should support namespace as well.

sanposhiho · 2022-03-28T16:50:02Z

server/di/interface.go

-	Delete(ctx context.Context, name string) error
+	Get(ctx context.Context, name string, namespace string) (*corev1.Pod, error)
+	List(ctx context.Context, namespace string) (*corev1.PodList, error)
+	Apply(ctx context.Context, pod *configv1.PodApplyConfiguration, namespace string) (*corev1.Pod, error)


Could you swap the order of pod and namespace? (PVC service as well.)

Hi, @sanposhiho thanks for the reviews!
I implemented those proposals. Please re-review it.

Co-authored-by: Kensei Nakada <handbomusic@gmail.com>

sanposhiho · 2022-04-02T04:50:11Z

reset/reset.go

-	schedService   SchedulerService
+	// deleteNamespacedServices has the all services for each namespaced resource.
+	// key: service name.
+	deleteWithNamespaceService map[string]DeleteWithNamespaceService


Suggested change

deleteWithNamespaceService map[string]DeleteWithNamespaceService

deleteServicesForNamespacedResources map[string]DeleteWithNamespaceService

Please change interface name as well. DeleteWithNamespaceService sounds like it will delete resource and namespace.

That's true. Fixed👌

sanposhiho · 2022-04-05T16:07:35Z

server/di/di.go

+		"storage class":     c.storageClassService,
+		"priority class":    c.priorityClassService,
+	}
+	deleteWithNamespaceService := map[string]reset.DeleteServicesForNamespacedResources{


Rename to deleteForNamespacedResources

oops, thanks!

sanposhiho

Thanks for your huge work 👍

/lgtm
/approve

k8s-ci-robot · 2022-04-24T04:39:53Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: 196Ikuchil, sanposhiho

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [sanposhiho]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

196Ikuchil added 3 commits March 25, 2022 18:54

feat:pod operation requires namespace specification

6b8b210

feat:pvc operation requires namspace speciication

b71043d

test:namespace specification

1ad3a85

k8s-ci-robot assigned sanposhiho Mar 25, 2022

k8s-ci-robot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Mar 25, 2022

k8s-ci-robot requested a review from alculquicondor March 25, 2022 15:36

k8s-ci-robot added the sig/scheduling Categorizes an issue or PR as relevant to SIG Scheduling. label Mar 25, 2022

k8s-ci-robot requested a review from sanposhiho March 25, 2022 15:36

docs:add comment

9a4a6df

196Ikuchil force-pushed the namespace_specify branch from df3d89f to 9a4a6df Compare March 25, 2022 15:52

test:add Delete test case

70c7bfd

196Ikuchil force-pushed the namespace_specify branch from 36332bb to 70c7bfd Compare March 26, 2022 12:05

sanposhiho reviewed Mar 28, 2022

View reviewed changes

k8s-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Mar 28, 2022

k8s-ci-robot changed the title ~~Namespace specify~~ support namespace in each service Mar 28, 2022

sanposhiho reviewed Mar 28, 2022

View reviewed changes

196Ikuchil and others added 4 commits March 29, 2022 18:52

Update export/export.go

2bf419f

Co-authored-by: Kensei Nakada <handbomusic@gmail.com>

docs:fix description of defaultNamespaceName

5653861

refactor:swap the order of pod and namespace

b220394

fix:DeleteCollection support namespace

69ea0ec

sanposhiho reviewed Apr 2, 2022

View reviewed changes

refactor:rename to DeleteServicesForNamespacedResources

6aed01e

sanposhiho reviewed Apr 5, 2022

View reviewed changes

fix:replace var name to deleteForNamespacedResources

4276280

sanposhiho reviewed Apr 24, 2022

View reviewed changes

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Apr 24, 2022

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Apr 24, 2022

k8s-ci-robot merged commit e584f17 into kubernetes-sigs:master Apr 24, 2022

196Ikuchil mentioned this pull request Sep 25, 2022

Support Namespace resource #236

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

support namespace in each service #137

support namespace in each service #137

196Ikuchil commented Mar 25, 2022

k8s-ci-robot commented Mar 25, 2022

sanposhiho left a comment

sanposhiho Mar 28, 2022

sanposhiho commented Mar 28, 2022

sanposhiho commented Mar 28, 2022

sanposhiho Mar 28, 2022

sanposhiho Mar 28, 2022

196Ikuchil Mar 29, 2022

sanposhiho Apr 2, 2022 •

edited

Loading

sanposhiho Apr 2, 2022

196Ikuchil Apr 3, 2022

sanposhiho Apr 5, 2022

196Ikuchil Apr 6, 2022

sanposhiho left a comment

k8s-ci-robot commented Apr 24, 2022

	deleteWithNamespaceService map[string]DeleteWithNamespaceService
	deleteServicesForNamespacedResources map[string]DeleteWithNamespaceService

support namespace in each service #137

support namespace in each service #137

Conversation

196Ikuchil commented Mar 25, 2022

What type of PR is this?

What this PR does / why we need it:

Which issue(s) this PR fixes:

Special notes for your reviewer:

k8s-ci-robot commented Mar 25, 2022

sanposhiho left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sanposhiho commented Mar 28, 2022

sanposhiho commented Mar 28, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sanposhiho Apr 2, 2022 • edited Loading

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

Choose a reason for hiding this comment

sanposhiho left a comment

Choose a reason for hiding this comment

k8s-ci-robot commented Apr 24, 2022

sanposhiho Apr 2, 2022 •

edited

Loading