karmadactl apply uses factory to access member cluster

[helen-frank]

What type of PR is this? /kind feature

Which issue(s) this PR fixes: Fixes # Part of#2349 Special notes for your reviewer: Test

go run ./cmd/karmadactl/karmadactl.go --kubeconfig /etc/karmada/karmada-apiserver.config apply -f ./_tmp/nginx.yaml
namespace/test1 created
deployment.apps/nginx1 created

Does this PR introduce a user-facing change?:

NONE

[RainbowMango]

@XiShanYongYe-Chang Seems this change has nothing to do with E2E.

[XiShanYongYe-Chang]

@XiShanYongYe-Chang Seems this change has nothing to do with E2E.

Let me take a look.

[carlory]

I'll review ASAP.

[lonelyCZ]

Hi, @helen-frank , It seems a problem.

[root@master67 karmada]# ./karmadactl apply -f deployment.yaml --kubeconfig /etc/karmada/karmada-apiserver.config -C member68 -v=4
deployment.apps/nginx created
error: no kind "PropagationPolicy" is registered for version "policy.karmada.io/v1alpha1" in scheme "k8s.io/kubectl/pkg/scheme/scheme.go:28"

But the pp has been created.

[root@master67 karmada]# kapi get pp
NAME               AGE
nginx-6d7f8d5f5b   3m45s

[helen-frank]

@lonelyCZ fixed

[codecov-commenter]

Codecov Report

:exclamation: No coverage uploaded for pull request base (master@2a0610e). Click here to learn what that means. The diff coverage is n/a.

:exclamation: Current head 8f8f173 differs from pull request most recent head 97a1cae. Consider uploading reports for the commit 97a1cae to get more accurate results

@@            Coverage Diff            @@
##             master    #2570   +/-   ##
=========================================
  Coverage          ?   21.88%           
=========================================
  Files             ?      183           
  Lines             ?    18381           
  Branches          ?        0           
=========================================
  Hits              ?     4023           
  Misses            ?    14048           
  Partials          ?      310           

Flag	Coverage Δ
unittests	21.88% <0.00%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

[lonelyCZ]

Hi, @helen-frank , have you tested it in your env? It seems still the problem in my env.

[helen-frank]

Hi, @helen-frank , have you tested it in your env? It seems still the problem in my env.

Yes, I have tested it with pp.yaml below samples, but I am trying to solve this problem by modifying Scheme, please wait a moment

[helen-frank]

@lonelyCZ I added the karmada scheme to k8s.io/kubectl/pkg/scheme, karmadactl apply the create capability of apply is complete, but patch will have problems “Error from server (UnsupportedMediaType): the body of the request was in an unknown format - accepted media types include: application/json-patch+json, application/merge-patch+json, application/apply-patch+yaml”

vendor/k8s.io/kubectl/pkg/cmd/apply/patcher.go

// Create the versioned struct from the type defined in the restmapping
	// (which is the API version we'll be submitting the patch to)
	versionedObject, err := scheme.Scheme.New(p.Mapping.GroupVersionKind)
	switch {
	case runtime.IsNotRegisteredError(err):
		// fall back to generic JSON merge patch
		patchType = types.MergePatchType
		...
	case err != nil:
		return nil, nil, cmdutil.AddSourceToErr(fmt.Sprintf("getting instance of versioned object for %v:", p.Mapping.GroupVersionKind), source, err)
	case err == nil:
		// Compute a three way strategic merge patch to send to server.
		patchType = types.StrategicMergePatchType
                ...
	}

When the karmada scheme is not added to' k8s.io/kubectl/pkg/scheme', use

// Complete completes all the required options
func (o *CommandApplyOptions) Complete(f util.Factory, cmd *cobra.Command, parentCommand string, args []string) error {
	karmadaClient, err := f.KarmadaClientSet()
	if err != nil {
		return err
	}
	o.karmadaClient = karmadaClient

	restConfig, err := f.ToRESTConfig()
	if err != nil {
		return err
	}
	defaultConfigFlags.WrapConfigFn = func(config *rest.Config) *rest.Config { return restConfig }

	o.KubectlApplyFlags.Factory = cmdutil.NewFactory(defaultConfigFlags)
        ...
}

It can solve the problem, but I don't quite understand this logic in the code.

And I think these codes are unreasonable, can you give me some tips about the solution here?

[helen-frank]

I tried to solve this problem by FactoryForKarmada()

[lonelyCZ]

And I think these codes are unreasonable, can you give me some tips about the solution here?

I guess the reason could be that the Factory is not instantiated.

[lonelyCZ]

I just tested it that worked also fine in my env.

Thanks for your hard work! @helen-frank

/lgtm

/cc @carlory

[carlory]

sorry for too late reply, I will review it tomorrow.

[carlory]

@helen-frank

we don't need define the FactoryForKarmada func because the factoryImpl has implemented the cmdutil.Factory interface.

The root cause is https://github.com/karmada-io/karmada/blob/d9e3bc860629202aa3c8629f743080d61afc2297/pkg/karmadactl/apply.go#L177

we should use UnstructuredClientForMapping instead of ClientForMapping.

[helen-frank]

@helen-frank

we don't need define the FactoryForKarmada func because the factoryImpl has implemented the cmdutil.Factory interface.

The root cause is

https://github.com/karmada-io/karmada/blob/d9e3bc860629202aa3c8629f743080d61afc2297/pkg/karmadactl/apply.go#L177

we should use UnstructuredClientForMapping instead of ClientForMapping.

OK, let me try

[helen-frank]

I have just tested it and can work normally. Thank you @carlory

[helen-frank]

Look at my other pr if you have time.

[carlory]

/lgtm

/cc @lonelyCZ

[carlory]

Look at my other pr if you have time.

I will review it this week.

[lonelyCZ]

Thanks!

/approve

[karmada-bot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: lonelyCZ

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~~ [lonelyCZ]

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