Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[WIP] yurtcl join use node-servant #630

Closed
wants to merge 1 commit into from

Conversation

adamzhoul
Copy link
Member

@adamzhoul adamzhoul commented Nov 25, 2021

What type of PR is this?

Uncomment only one /kind <> line, hit enter to put that in a new line, and remove leading whitespace from that line:

/kind enhancement

Which issue(s) this PR fixes:

#597 [feature request] Add an option to "yurtctl join" to enable "node autonomy" automatically.

#545 [feature request] yurtctl join use node-servant to finish some job
#516 add node-servant

@openyurt-bot openyurt-bot added the do-not-merge/work-in-progress do-not-merge/work-in-progress label Nov 25, 2021
@openyurt-bot
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: adamzhoul
To complete the pull request process, please assign kadisi
You can assign the PR to them by writing /assign @kadisi in a comment when ready.

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

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

@openyurt-bot openyurt-bot added the size/L size/L: 100-499 label Nov 25, 2021
@adamzhoul adamzhoul marked this pull request as draft November 25, 2021 15:41
@adamzhoul adamzhoul force-pushed the yurtcl_join branch 3 times, most recently from cf515a2 to e99ebb2 Compare November 25, 2021 16:29
return err
}
}

if err := apiclient.PatchNode(client, cfg.NodeRegistration.Name, func(n *v1.Node) {
n.Labels[projectinfo.GetEdgeWorkerLabelKey()] = "true"
Copy link
Member

Choose a reason for hiding this comment

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

how about move makeAutonomous at here?

Copy link
Member Author

Choose a reason for hiding this comment

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

Do you mean using MarkAutonomousOptions int pkg/yurtctl/cmd/markautonomous/markautonomous.go

	co := NewMarkAutonomousOptions()
        o.RunMarkAutonomous()

it's definitely better using this.

But, it has serval questions we have to answer:

  1. a kubeconf file is required, and we may use kubelet.conf. Is this ok?
  2. join package import markautonomous package is ok but not advised I think.

Copy link
Member

Choose a reason for hiding this comment

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

@adamzhoul I'm sorry that the comment is not clear. i mean that move the code line 114~120 after line 122, so reduce one node patch request.

@adamzhoul
Copy link
Member Author

don't have enough time to have a full test.
besides, the community is going to remove k8s.io/kubernetes.
this will be affected.

so close this pr for now.

@adamzhoul adamzhoul closed this Dec 20, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
do-not-merge/work-in-progress do-not-merge/work-in-progress size/L size/L: 100-499
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants