Skip to content
This repository was archived by the owner on Oct 17, 2024. It is now read-only.

support run work agent outside of managed cluster#104

Merged
openshift-merge-robot merged 2 commits intoopen-cluster-management-io:mainfrom
zhujian7:agent-outside
Dec 3, 2021
Merged

support run work agent outside of managed cluster#104
openshift-merge-robot merged 2 commits intoopen-cluster-management-io:mainfrom
zhujian7:agent-outside

Conversation

@zhujian7
Copy link
Copy Markdown
Member

@zhujian7 zhujian7 commented Nov 4, 2021

Signed-off-by: zhujian jiazhu@redhat.com

related issue: open-cluster-management-io/registration-operator#158


todo:

  • add description and script to deploy work agent in Hosted mode. (not contained in this PR)

Comment thread pkg/spoke/spokeagent.go Outdated
flags := cmd.Flags()
// This command only supports reading from config
flags.StringVar(&o.HubKubeconfigFile, "hub-kubeconfig", o.HubKubeconfigFile, "Location of kubeconfig file to connect to hub cluster.")
flags.StringVar(&o.SpokeKubeconfigFile, "spoke-kubeconfig", o.SpokeKubeconfigFile, "Location of kubeconfig file to connect to spoke cluster.")
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can't we use the built-in --kubeconfig flag here?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it is better not, the built-in --kubeconfig in our case represents the cluster where the work-agent pod run.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this should be explicit callout that if this is not set, but --kubeconfig is set. kubeconfig will be used to connect to spoke.

@zhujian7 zhujian7 force-pushed the agent-outside branch 3 times, most recently from e29befa to 9a6686d Compare December 2, 2021 07:49
@zhujian7 zhujian7 changed the title WIP:support run work agent outside of managed cluster support run work agent outside of managed cluster Dec 2, 2021
Comment thread pkg/spoke/spokeagent.go Outdated
Burst int
HubKubeconfigFile string
SpokeKubeconfigFile string
SpokeKubeconfig *rest.Config
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why do we need this?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was originally used for integration test, but now I think it can be deleted

Comment thread pkg/spoke/spokeagent.go Outdated
spokeDynamicClient, err := dynamic.NewForConfig(spokeRestConfig)
// load spoke client config and create spoke clients,
// the work agent may running not in the spoke/managed cluster.
if o.SpokeKubeconfig == nil {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do not see how it can be not nil

Comment thread pkg/spoke/spokeagent.go Outdated
flags := cmd.Flags()
// This command only supports reading from config
flags.StringVar(&o.HubKubeconfigFile, "hub-kubeconfig", o.HubKubeconfigFile, "Location of kubeconfig file to connect to hub cluster.")
flags.StringVar(&o.SpokeKubeconfigFile, "spoke-kubeconfig", o.SpokeKubeconfigFile, "Location of kubeconfig file to connect to spoke cluster.")
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this should be explicit callout that if this is not set, but --kubeconfig is set. kubeconfig will be used to connect to spoke.

Comment thread pkg/spoke/spokeagent.go Outdated
if o.SpokeKubeconfigFile != "" {
o.SpokeKubeconfig, err = clientcmd.BuildConfigFromFlags("" /* leave masterurl as empty */, o.SpokeKubeconfigFile)
if err != nil {
return fmt.Errorf("unable to load spoke kubeconfig from file %q: %w", o.SpokeKubeconfigFile, err)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would make it a func

 func (o *WorkloadAgentOptions) spokeKubeConfig(controllerContext) *rest.Config, error {
    if o.SpokeKubeconfigFile != "" {
        return clientcmd.BuildConfigFromFlags("" /* leave masterurl as empty */, o.SpokeKubeconfigFile)
    }

   return controllerContext.KubeConfig
}

add a flag spoke-kubeconfig to specify the kubeconfig of the apoke/managed cluster

Signed-off-by: zhujian <jiazhu@redhat.com>
@zhujian7
Copy link
Copy Markdown
Member Author

zhujian7 commented Dec 2, 2021

/retest

Signed-off-by: zhujian <jiazhu@redhat.com>
Copy link
Copy Markdown
Member

@qiujian16 qiujian16 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/approve
/lgtm

@openshift-ci
Copy link
Copy Markdown
Contributor

openshift-ci Bot commented Dec 3, 2021

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: qiujian16, zhujian7

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

The pull request process is described here

Details Needs approval from an approver in each of these files:

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

@openshift-ci openshift-ci Bot added the approved label Dec 3, 2021
@openshift-merge-robot openshift-merge-robot merged commit 44e5c78 into open-cluster-management-io:main Dec 3, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants