karmada
karmada copied to clipboard
introduce factory interface for karmadactl
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:
- https://github.com/karmada-io/karmada/blob/master/pkg/karmadactl/get_flags.go
- https://github.com/karmada-io/karmada/blob/master/pkg/karmadactl/config.go
- https://github.com/karmada-io/karmada/blob/master/pkg/karmadactl/options/global.go
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
/cc @lonelyCZ @RainbowMango @prodanlabs
Looks good, I will further look it.
/assign
Hi @lonelyCZ, any suggestions here?
/cc @lonelyCZ
Sorry for delay, I will back ASAP after previous prs.
Hi @lonelyCZ, any suggestions here?
/retitle introduce factory interface for karmadactl
@lonelyCZ rename FactoryExpansion
to Factory
and move it into util
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?
@ 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.
/cc @lonelyCZ @prodanlabs
Looks good to me.
/cc @RainbowMango
Hi @RainbowMango , any suggestions here?
Please take a look @RainbowMango
OK. I'll try to finish it by the end of this week.
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?
@RainbowMango alpha
has been removed.
karmadactl logs uses factory to access member cluster is an example.
@RainbowMango this issue https://github.com/karmada-io/karmada/issues/2349 list things we need to do
I'm good with it. But I'll leave approval to the owner @lonelyCZ .
Hi @carlory, sorry for delaying too long, let's back.
@lonelyCZ updated.
Thanks @carlory , I just tested it that worked fine.
/lgtm
PTAL /assign @RainbowMango
[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
- ~~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