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

Auto register edge node #1184

Closed
wants to merge 5 commits into from
Closed

Auto register edge node #1184

wants to merge 5 commits into from

Conversation

sids-b
Copy link
Member

@sids-b sids-b commented Oct 10, 2019

What type of PR is this?
Uncomment only one /kind <> line, hit enter to put that in a new line, and remove leading whitespaces from that line:

/kind feature

What this PR does / why we need it:
Support auto-register edge nodes. We can still allow manual addition of nodes for the administrators to additionally configure nodes by adding labels and other fields.

Which issue(s) this PR fixes:
Fixes #881

Special notes for your reviewer:
#1045 will be fixed in next PR

Does this PR introduce a user-facing change?:


@kubeedge-bot kubeedge-bot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Oct 10, 2019
@sids-b
Copy link
Member Author

sids-b commented Oct 10, 2019

/cc @subpathdev @kadisi

@sids-b
Copy link
Member Author

sids-b commented Oct 10, 2019

/cc @qizha @Baoqiang-Zhang


+ Make sure role is set to edge for the node. For this a key of the form `"node-role.kubernetes.io/edge"` must be present in `labels` tag of `metadata`.
+ Please ensure to add the label `node-role.kubernetes.io/edge` to the `build/node.json` file.

Copy link
Member

Choose a reason for hiding this comment

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

Why support manual registration since it supports automatic registration?

Copy link
Member Author

Choose a reason for hiding this comment

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

We can still allow manual addition of nodes for the administrators to additionally configure nodes by adding labels and other fields. By default only role label and name label is added. But if someone wants to add nodes with common labels so that they can be grouped together, it can be useful.

@kadisi
Copy link
Member

kadisi commented Oct 16, 2019

Suppose there already exist a node (named node1) in kubernetes, and user want to add a new edged node(also named node1, Maybe he doesn't know that there is already a node named node1.). We assume that they set the same projectid 。 What will happens?

@sids-b
Copy link
Member Author

sids-b commented Oct 16, 2019

Suppose there already exist a node (named node1) in kubernetes, and user want to add a new edged node(also named node1, Maybe he doesn't know that there is already a node named node1.). We assume that they set the same projectid 。 What will happens?

Both the nodes will start updating the status of the same node. It happens with kubelet also if 2 machines have same hostname.

@kubeedge-bot kubeedge-bot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Oct 21, 2019
@kubeedge-bot
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
To fully approve this pull request, please assign additional approvers.
We suggest the following additional approver: sids-b

If they are not already assigned, you can assign the PR to them by writing /assign @sids-b in a comment when ready.

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:

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

@kubeedge-bot kubeedge-bot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Oct 21, 2019
Copy link
Member

@kevin-wangzefeng kevin-wangzefeng left a comment

Choose a reason for hiding this comment

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

The node create action (registration) should be triggered explicitly from an edge node, and CloudCore only reflect the request to call vanilla node creation.

Implicity node auto registration can't offer the same behaviour with upstream in cases that user unregister a node and then delete the machine:

  • edge node would continue reporting node status, while CloudCore can't distinguish that stat whether a node is a new or unregistered one.

@kubeedge-bot kubeedge-bot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Oct 21, 2019
Co-Authored-By: Kevin Wang <wangzefeng@huawei.com>
@sids-b
Copy link
Member Author

sids-b commented Oct 22, 2019

I like this new feature of github which directly lets you commit the review of reviewers :)

@@ -147,7 +147,8 @@ The cert/key will be generated in the `/etc/kubeedge/ca` and `/etc/kubeedge/cert


#### Deploy the edge node
We have provided a sample node.json to add a node in kubernetes. Please make sure edge-node is added in kubernetes. Run below steps to add edge-node.
KubeEdge supports auto registration of edge-nodes but we have provided a sample node.json to manually add a node in kubernetes if required.
Run below steps to add edge-node.
Copy link
Member

Choose a reason for hiding this comment

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

We should add the register-node: true configuration in line287 of setup.md

for {
resource := fmt.Sprintf("%s/%s/%s", e.namespace, model.ResourceTypeNodeStatus, e.nodeName)
msg := message.BuildMsg(modules.MetaGroup, "", modules.EdgedModuleName, resource, model.InsertOperation, "")
_, err := e.context.SendSync("websocket", *msg, 60*time.Second)
Copy link
Member

Choose a reason for hiding this comment

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

for websocket , Recommended to use constants 。

ModuleNameEdgeHub = "websocket"

@@ -372,6 +384,32 @@ func (uc *UpstreamController) updateNodeStatus(stop chan struct{}) {
}

switch msg.GetOperation() {
case model.InsertOperation:
klog.Infof("Received register node request, creating node")
if _, err := uc.createNode(name); err != nil {
Copy link
Member

Choose a reason for hiding this comment

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

We should receive the node obj instead of nodename from edge, kubelet has some features(flags like taints/labels) that set to node.

@@ -106,14 +106,40 @@ The cert/key will be generated in the `/etc/kubeedge/ca` and `/etc/kubeedge/cert
### Run Edge

#### Deploy the Edge node
We have provided a sample node.json to add a node in kubernetes. Please make sure edge-node is added in kubernetes. Run below steps to add edge-node.
KubeEdge supports auto registration of edge-nodes but we have provided a sample node.json to manually add a node in kubernetes if required.
Run below steps to add edge-node.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Run below steps to add edge-node.
KubeEdge supports auto registration of edge-nodes by default. But we also support manually add edge-nodes in kubernetes if required.
Run below steps to add edge-node manually.

cp $GOPATH/src/github.com/kubeedge/kubeedge/build/node.json ~/cmd/yaml
```

+ Make sure role is set to edge for the node. For this a key of the form `"node-role.kubernetes.io/edge"` must be present in `labels` tag of `metadata`.
Copy link
Member

Choose a reason for hiding this comment

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

Can delete this line? Add lable is enough. :)

}
```
**Note: you need to remember `metadata.name` , because edgecore need it**.
+ If role is not set for the node, the pods, configmaps and secrets created/updated in the cloud cannot be synced with the node they are targeted for.
Copy link
Member

Choose a reason for hiding this comment

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

lable is not set for the node. lable and role is different in k8s

@fisherxu
Copy link
Member

Any updates?

@kuramal
Copy link
Contributor

kuramal commented Dec 31, 2019

@sids-b @fisherxu Is there any plan of update?

@fisherxu
Copy link
Member

/close
Dumplicate with #1401

@kubeedge-bot
Copy link
Collaborator

@fisherxu: Closed this PR.

In response to this:

/close
Dumplicate with #1401

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Create and register node from edge part
6 participants