search

karmada-io / karmada

Open, Multi-Cloud, Multi-Cluster Kubernetes Orchestration
https://karmada.io
Apache License 2.0
4.12k stars 806 forks source link

introduce factory interface for karmadactl #2202

Closed carlory closed 1 year ago

carlory commented 1 year ago

What type of PR is this?

/kind feature

What this PR does / why we need it:

At present, there are many places in karmadactl, which are building clients and factories repeatedly, such as init, get, log, exec, etc.

The purpose of this PR is to provide a unified place to get clients accessing the cluster and factories needed to access sub-clusters.

With this change, after a little refactoring, the following files can be removed in the future:

An example of the alpha command is provided to make it easier for reviewers to understand the usage. It will be removed after feedback is collected.

(⎈ |karmada:default)➜  karmada git:(karmadactl-factory) go run cmd/karmadactl/karmadactl.go alpha
karmada: Kubernetes v1.21.7
ik8s: Kubernetes v1.21.5
ocp: Kubernetes v1.11.0+d4cacc0

(⎈ |karmada:default)➜  karmada git:(karmadactl-factory) go run cmd/karmadactl/karmadactl.go alpha -v6
I0716 15:29:22.371611   75493 loader.go:372] Config loaded from file:  /Users/kiki/.kube/config
I0716 15:29:22.414523   75493 round_trippers.go:553] GET https://10.29.0.221:5443/version 200 OK in 41 milliseconds
karmada: Kubernetes v1.21.7
I0716 15:29:22.440589   75493 round_trippers.go:553] GET https://10.29.0.221:5443/apis/cluster.karmada.io/v1alpha1/clusters 200 OK in 25 milliseconds
ik8s: I0716 15:29:22.798588   75493 round_trippers.go:553] GET https://10.29.0.221:5443/apis/cluster.karmada.io/v1alpha1/clusters/ik8s 200 OK in 24 milliseconds
I0716 15:29:22.828802   75493 loader.go:372] Config loaded from file:  /Users/kiki/.kube/config
I0716 15:29:22.887672   75493 round_trippers.go:553] GET https://10.29.0.221:5443/apis/cluster.karmada.io/v1alpha1/clusters/ik8s/proxy/version 200 OK in 57 milliseconds
Kubernetes v1.21.5
ocp: I0716 15:29:22.909929   75493 round_trippers.go:553] GET https://10.29.0.221:5443/apis/cluster.karmada.io/v1alpha1/clusters/ocp 200 OK in 21 milliseconds
I0716 15:29:22.935370   75493 loader.go:372] Config loaded from file:  /Users/kiki/.kube/config
I0716 15:29:23.024844   75493 round_trippers.go:553] GET https://10.29.0.221:5443/apis/cluster.karmada.io/v1alpha1/clusters/ocp/proxy/version 200 OK in 88 milliseconds
Kubernetes v1.11.0+d4cacc0

Does this PR introduce a user-facing change?:

NONE
carlory commented 1 year ago

/cc @lonelyCZ @RainbowMango @prodanlabs

lonelyCZ commented 1 year ago

Looks good, I will further look it.

lonelyCZ commented 1 year ago

/assign

carlory commented 1 year ago

Hi @lonelyCZ, any suggestions here?

carlory commented 1 year ago

/cc @lonelyCZ

lonelyCZ commented 1 year ago

Sorry for delay, I will back ASAP after previous prs.

carlory commented 1 year ago

Hi @lonelyCZ, any suggestions here?

carlory commented 1 year ago

/retitle introduce factory interface for karmadactl

carlory commented 1 year ago

@lonelyCZ rename FactoryExpansion to Factory and move it into util

lonelyCZ commented 1 year ago

rename FactoryExpansion to Factory and move it into util

I like it.

Could you please give a usage example for one subcommand that will be refactored by this interface?

carlory commented 1 year ago

@ lonelyCZ

karmadactl logs uses factory to access member cluster is an example.

(⎈ |karmada:default)➜  karmada git:(karmadactl-factory) go run cmd/karmadactl/karmadactl.go logs -C ocp master-etcd-nodedsp01.ocp.ats.io -n kube-system --tail 10
2022-08-09 15:21:51.800610 W | etcdserver: apply entries took too long [103.846094ms for 1 entries]
2022-08-09 15:21:51.800698 W | etcdserver: avoid queries with large range/delete range!
2022-08-09 15:24:04.745515 I | mvcc: store.index: compact 182209811
2022-08-09 15:24:04.877300 I | mvcc: finished scheduled compaction at 182209811 (took 125.525534ms)
2022-08-09 15:24:29.974311 W | etcdserver: apply entries took too long [120.414531ms for 1 entries]
2022-08-09 15:24:29.974353 W | etcdserver: avoid queries with large range/delete range!
2022-08-09 15:25:41.787238 W | wal: sync duration of 1.107194589s, expected less than 1s
2022-08-09 15:26:29.805921 W | wal: sync duration of 1.55465076s, expected less than 1s
2022-08-09 15:27:23.477009 W | etcdserver: apply entries took too long [118.29844ms for 1 entries]
2022-08-09 15:27:23.477058 W | etcdserver: avoid queries with large range/delete range!

(⎈ |karmada:default)➜  karmada git:(karmadactl-factory) go run cmd/karmadactl/karmadactl.go logs -C ocp router-1-hw9vw --tail 5
 - Health check ok : 0 retry attempt(s).
I0809 07:26:05.182271       1 router.go:481] Router reloaded:
 - Checking http://localhost:80 ...
 - Health check ok : 0 retry attempt(s).
W0809 07:27:12.762383       1 reflector.go:272] github.com/openshift/origin/pkg/router/controller/factory/factory.go:112: watch of *v1.Route ended with: The resourceVersion for the provided watch is too old.
carlory commented 1 year ago

/cc @lonelyCZ @prodanlabs

lonelyCZ commented 1 year ago

Looks good to me.

/cc @RainbowMango

carlory commented 1 year ago

Hi @RainbowMango , any suggestions here?

carlory commented 1 year ago

Please take a look @RainbowMango

RainbowMango commented 1 year ago

OK. I'll try to finish it by the end of this week.

RainbowMango commented 1 year ago

An example of the alpha command is provided to make it easier for reviewers to understand the usage. It will be removed after feedback is collected.

(⎈ |karmada:default)➜ karmada git:(karmadactl-factory) go run cmd/karmadactl/karmadactl.go alpha karmada: Kubernetes v1.21.7 ik8s: Kubernetes v1.21.5 ocp: Kubernetes v1.11.0+d4cacc0

What's the alpha thing?

carlory commented 1 year ago

@RainbowMango alpha has been removed.

karmadactl logs uses factory to access member cluster is an example.

carlory commented 1 year ago

@RainbowMango this issue https://github.com/karmada-io/karmada/issues/2349 list things we need to do

RainbowMango commented 1 year ago

I'm good with it. But I'll leave approval to the owner @lonelyCZ .

lonelyCZ commented 1 year ago

Hi @carlory, sorry for delaying too long, let's back.

carlory commented 1 year ago

@lonelyCZ updated.

lonelyCZ commented 1 year ago

Thanks @carlory , I just tested it that worked fine.

/lgtm

PTAL /assign @RainbowMango

karmada-bot commented 1 year ago

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: RainbowMango

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: - ~~[pkg/karmadactl/OWNERS](https://github.com/karmada-io/karmada/blob/master/pkg/karmadactl/OWNERS)~~ [RainbowMango] Approvers can indicate their approval by writing `/approve` in a comment Approvers can cancel approval by writing `/approve cancel` in a comment