change kubeletEndpoint port when cloudstream conneted #3277

chenchunxiu · 2021-11-08T07:39:10Z

Signed-off-by: chenchunxiu chenchunxiu_yewu@cmss.chinamobile.com

What type of PR is this?
/kind bug

What this PR does / why we need it:
If cloudcore is multi-active (cloudcore1, cloudcore2, cloudcore3) and LB is used.
In bellowing case, kubectl exec/logs failed.

LB forward connection of cloudhub (from edge-node-01) to cloudcore1. --- cloudhub port 10000
LB forward connection of cloudstream (from edge-node-01) to cloudcore2. --- cloudstream port 10004

So, we should change kubeletEndpoint when cloudstrem conneted, not cloudhub connected.

Which issue(s) this PR fixes:
Fixes #

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

Signed-off-by: chenchunxiu <chenchunxiu_yewu@cmss.chinamobile.com>

chenchunxiu · 2021-11-15T07:23:43Z

anybody has time to review? @fisherxu

zc2638 · 2021-11-16T01:52:14Z

Thanks for fixing.
Can you add more descriptions of this patch?

Is it because the node has been created, and then LB needs to be added to the cloud, and the kubeletEndpoint needs to be changed at this time?

chenchunxiu · 2021-11-16T04:29:53Z

Thanks for fixing. Can you add more descriptions of this patch?

Is it because the node has been created, and then LB needs to be added to the cloud, and the kubeletEndpoint needs to be changed at this time?

When cloudcore is multi-active and LB is used, there is no guarantee that LB will forward the edgehub connection and edgestream connection of the same node to the same cloudcore.
If LB forwards both edgehub and edgestream connections to the same cloudcore, kubectl logs/exec works normally

If LB forwards both edgehub and edgestream connections to different cloudcore, kubectl logs/exec will not work because the kubectl logs/exec request is forwarded to the cloudcore without stream connection.

Therefore, we should keep the setting of kubeletEndpointPort consistent with the cloudcore where cloudstream is connected.

zc2638 · 2021-11-17T01:55:01Z

I have some doubts.

The original logic is to update the kubeletEndpoint when the node is created or the node status is updated.
Now, when the edge node connects to the cloud stream, first update the kubeletEndpoint.

What is the difference between them?
What is the difference between processing this logic in different places?

zc2638 · 2021-11-17T05:43:26Z

For edge nodes, only the address of LB.
When edgestream connects to the corresponding cloudstream, confirm the proxy port of the cloud.

e.g. maybe edgestream connects to cloudstream A, so we update kubeletEndpoint to 10351.
e.g. maybe edgestream connects to cloudstream B, so we update kubeletEndpoint to 10352.

This is my understanding of your explanation, is that the case?
@chenchunxiu

kevin-wangzefeng · 2021-11-17T08:20:09Z

This is caused by inconsistent load-balancing on multiple requests from a some edge node.
To quick fix, you may enable features like "sticky session", or "route by source IP" provided by the LB.

fisherxu · 2021-11-17T08:30:42Z

For edge nodes, only the address of LB. When edgestream connects to the corresponding cloudstream, confirm the proxy port of the cloud.

e.g. maybe edgestream connects to cloudstream A, so we update kubeletEndpoint to 10351. e.g. maybe edgestream connects to cloudstream B, so we update kubeletEndpoint to 10352.

This is my understanding of your explanation, is that the case? @chenchunxiu

CloudStream/EdgeStream use one connection, and the cloudhub/edgehub use another connection. The two connections may on two servers behide LB, and we use the cloud/edge hub to update the nodestatus, so CloudStream/EdgeStream will not work. @zc2638

fisherxu

Looks good, please resolve the confict :) @chenchunxiu

chenchunxiu · 2021-11-17T15:38:20Z

Looks good, please resolve the confict :) @chenchunxiu

Done

fisherxu · 2021-11-22T08:13:23Z

cloud/pkg/cloudstream/tunnelserver.go

+	getNode.Status.DaemonEndpoints.KubeletEndpoint.Port = int32(s.tunnelPort)
+	node, err := client.GetKubeClient().CoreV1().Nodes().UpdateStatus(context.Background(), getNode, metav1.UpdateOptions{})
+	if err != nil {
+		klog.Errorf("update node KubeletEndpoint Port failed with error: %s, node: %v, tunnelPort:%v",


We should have some try then exit directly if err occurs in this func. @chenchunxiu
Ref: https://github.com/kubernetes/kubernetes/blob/master/pkg/controller/nodelifecycle/node_lifecycle_controller.go#L792

chenchunxiu · 2021-11-25T08:04:29Z

ping @fisherxu @zc2638

fisherxu · 2021-11-27T03:12:52Z

cloud/pkg/cloudstream/tunnelserver.go

+		if errors.IsNotFound(err) {
+			klog.Warningf("node %s not found", nodeName)
+			return false, nil
+		}
+		if err != nil {
+			klog.Errorf("Failed while getting a Node to retry updating node KubeletEndpoint Port. error: %v", nodeName, err)
+			return false, err
+		}


Suggested change

if errors.IsNotFound(err) {

klog.Warningf("node %s not found", nodeName)

return false, nil

}

if err != nil {

klog.Errorf("Failed while getting a Node to retry updating node KubeletEndpoint Port. error: %v", nodeName, err)

return false, err

}

if err != nil {

klog.Errorf("Failed while getting a Node to retry updating node KubeletEndpoint Port, node: %s, error: %v", nodeName, err)

return false, nil

}

fisherxu · 2021-11-27T03:16:13Z

cloud/pkg/cloudstream/tunnelserver.go

+		getNode.Status.DaemonEndpoints.KubeletEndpoint.Port = int32(s.tunnelPort)
+		_, err = client.GetKubeClient().CoreV1().Nodes().UpdateStatus(context.Background(), getNode, metav1.UpdateOptions{})
+		if err != nil {
+			klog.Errorf("update node KubeletEndpoint Port failed with error: %s, node: %v, tunnelPort: %v",


Suggested change

klog.Errorf("update node KubeletEndpoint Port failed with error: %s, node: %v, tunnelPort: %v",

klog.Errorf("Failed to update node KubeletEndpoint Port, node: %s, tunnelPort: %s, err: %v", getNode.Name, s.tunnelPort, err)

fisherxu · 2021-11-27T03:22:14Z

cloud/pkg/cloudstream/tunnelserver.go

+		klog.Errorf("Update KubeletEndpoint Port of Node '%v' error: %v. ", nodeName, err)
+		os.Exit(1)
+	}
+	klog.Infof("update node KubeletEndpoint Port success. node: %s, tunnelPort: %v", nodeName, s.tunnelPort)


Suggested change

klog.Infof("update node KubeletEndpoint Port success. node: %s, tunnelPort: %v", nodeName, s.tunnelPort)

klog.V(2).Infof("Update node KubeletEndpoint Port successfully, node: %s, tunnelPort: %s", nodeName, s.tunnelPort)

V(4) maybe more fine, or large scale edge nodes will print lots of success logs.

fisherxu · 2021-11-27T03:23:04Z

cloud/pkg/cloudstream/tunnelserver.go

+		}
+		return true, nil
+	}); err != nil {
+		klog.Errorf("Update KubeletEndpoint Port of Node '%v' error: %v. ", nodeName, err)


Suggested change

klog.Errorf("Update KubeletEndpoint Port of Node '%v' error: %v. ", nodeName, err)

klog.Errorf("Failed to update node KubeletEndpoint Port, node: %s, tunnelPort: %s, err: %v", nodeName, s.tunnelPort, err)

fisherxu · 2021-11-27T03:34:46Z

cloud/pkg/cloudstream/tunnelserver.go

 	"github.com/kubeedge/kubeedge/pkg/stream"
 )

+const (
+	// The amount of time the tunnelserver should sleep between retrying node status updates
+	retrySleepTime        = 20 * time.Millisecond


Suggested change

retrySleepTime = 20 * time.Millisecond

retrySleepTime = 20 * time.Second

The retry time can be 20s here, if there is a delay in edge node creation, here need to wait more.

fisherxu · 2021-11-27T03:37:31Z

cloud/pkg/cloudstream/tunnelserver.go

@@ -169,3 +184,30 @@ func (s *TunnelServer) Start() {
 		return
 	}
 }
+
+func (s *TunnelServer) updateNodeKubeletEndpoint(nodeName string) {
+	if err := wait.PollImmediate(retrySleepTime, nodeStatusUpdateRetry, func() (bool, error) {


Suggested change

if err := wait.PollImmediate(retrySleepTime, nodeStatusUpdateRetry, func() (bool, error) {

if err := wait.Poll(retrySleepTime, nodeStatusUpdateRetry, func() (bool, error) {

Let's use wait.Poll here, PollImmediate will exit when err occur, here we need to retry.

Thanks for the reply.
I test in my machine, if err happens retry will execute.
wait.PollImmediate will run the func immediately, and wait.Poll will wait for the interval before run the func

Okey, sounds good, and so we need to return false,nil as I comment above. Or the retry will exit directly.

Thanks for the reply, I have changed the return false, err to return false, nil.

gy95 · 2021-11-27T07:56:46Z

cloud/pkg/cloudstream/tunnelserver.go

 type TunnelServer struct {
 	container *restful.Container
 	upgrader  websocket.Upgrader
 	sync.Mutex
 	sessions   map[string]*Session
 	nodeNameIP sync.Map
+	tunnelPort int


stupid question 😄 Is this tunnelPort from the configuration file and need to be configured by users themselves?

No, users do not need to configure manually.
tunnelPort will be automatically generated when cloudcore is started at first time and saved in configmap. It will be read directly from configmap the next time cloudcore started.

Signed-off-by: chenchunxiu <chenchunxiu_yewu@cmss.chinamobile.com>

fisherxu

/lgtm
/approve
Sorry for the delay.

kubeedge-bot · 2021-11-30T09:42:49Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: fisherxu

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:

~~OWNERS~~ [fisherxu]

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

kubeedge-bot added the kind/bug Categorizes issue or PR as related to a bug. label Nov 8, 2021

kubeedge-bot requested review from fisherxu and zc2638 November 8, 2021 07:39

kubeedge-bot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Nov 8, 2021

change kubeletEndpoint port when cloudstream conneted

4174812

Signed-off-by: chenchunxiu <chenchunxiu_yewu@cmss.chinamobile.com>

chenchunxiu force-pushed the b2-multi-cloudcore branch from a96c160 to 4174812 Compare November 8, 2021 10:36

kubeedge-bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 17, 2021

fisherxu reviewed Nov 17, 2021

View reviewed changes

kubeedge-bot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Nov 17, 2021

chenchunxiu force-pushed the b2-multi-cloudcore branch from fc3d411 to 4174812 Compare November 17, 2021 14:34

kubeedge-bot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Nov 17, 2021

Merge branch 'master' into b2-multi-cloudcore

a4e55bc

kubeedge-bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 17, 2021

fisherxu reviewed Nov 22, 2021

View reviewed changes

chenchunxiu mentioned this pull request Nov 26, 2021

REQUEST: New membership for chenchunxiu kubeedge/community#59

Closed

7 tasks

fisherxu reviewed Nov 27, 2021

View reviewed changes

gy95 reviewed Nov 27, 2021

View reviewed changes

add retry for node status update

30a32b5

Signed-off-by: chenchunxiu <chenchunxiu_yewu@cmss.chinamobile.com>

chenchunxiu force-pushed the b2-multi-cloudcore branch from fedff6d to 30a32b5 Compare November 29, 2021 01:23

fisherxu reviewed Nov 30, 2021

View reviewed changes

kubeedge-bot added the lgtm Indicates that a PR is ready to be merged. label Nov 30, 2021

kubeedge-bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Nov 30, 2021

kubeedge-bot merged commit c50af6a into kubeedge:master Nov 30, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

change kubeletEndpoint port when cloudstream conneted #3277

change kubeletEndpoint port when cloudstream conneted #3277

chenchunxiu commented Nov 8, 2021 •

edited

Loading

chenchunxiu commented Nov 15, 2021

zc2638 commented Nov 16, 2021

chenchunxiu commented Nov 16, 2021

zc2638 commented Nov 17, 2021

zc2638 commented Nov 17, 2021

kevin-wangzefeng commented Nov 17, 2021

fisherxu commented Nov 17, 2021

fisherxu left a comment

chenchunxiu commented Nov 17, 2021

fisherxu Nov 22, 2021

chenchunxiu Nov 23, 2021

chenchunxiu commented Nov 25, 2021

fisherxu Nov 27, 2021

chenchunxiu Nov 29, 2021

fisherxu Nov 27, 2021

chenchunxiu Nov 29, 2021

fisherxu Nov 27, 2021

fisherxu Nov 27, 2021

chenchunxiu Nov 29, 2021

fisherxu Nov 27, 2021

chenchunxiu Nov 29, 2021

fisherxu Nov 27, 2021

chenchunxiu Nov 29, 2021

fisherxu Nov 27, 2021

chenchunxiu Nov 27, 2021 •

edited

Loading

fisherxu Nov 27, 2021

chenchunxiu Nov 29, 2021

gy95 Nov 27, 2021 •

edited

Loading

chenchunxiu Nov 29, 2021

fisherxu left a comment

kubeedge-bot commented Nov 30, 2021

	klog.Errorf("update node KubeletEndpoint Port failed with error: %s, node: %v, tunnelPort: %v",
	klog.Errorf("Failed to update node KubeletEndpoint Port, node: %s, tunnelPort: %s, err: %v", getNode.Name, s.tunnelPort, err)

	klog.Infof("update node KubeletEndpoint Port success. node: %s, tunnelPort: %v", nodeName, s.tunnelPort)
	klog.V(2).Infof("Update node KubeletEndpoint Port successfully, node: %s, tunnelPort: %s", nodeName, s.tunnelPort)

	klog.Errorf("Update KubeletEndpoint Port of Node '%v' error: %v. ", nodeName, err)
	klog.Errorf("Failed to update node KubeletEndpoint Port, node: %s, tunnelPort: %s, err: %v", nodeName, s.tunnelPort, err)

	retrySleepTime = 20 * time.Millisecond
	retrySleepTime = 20 * time.Second

	if err := wait.PollImmediate(retrySleepTime, nodeStatusUpdateRetry, func() (bool, error) {
	if err := wait.Poll(retrySleepTime, nodeStatusUpdateRetry, func() (bool, error) {

change kubeletEndpoint port when cloudstream conneted #3277

change kubeletEndpoint port when cloudstream conneted #3277

Conversation

chenchunxiu commented Nov 8, 2021 • edited Loading

chenchunxiu commented Nov 15, 2021

zc2638 commented Nov 16, 2021

chenchunxiu commented Nov 16, 2021

zc2638 commented Nov 17, 2021

zc2638 commented Nov 17, 2021

kevin-wangzefeng commented Nov 17, 2021

fisherxu commented Nov 17, 2021

fisherxu left a comment

Choose a reason for hiding this comment

chenchunxiu commented Nov 17, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chenchunxiu commented Nov 25, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chenchunxiu Nov 27, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gy95 Nov 27, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fisherxu left a comment

Choose a reason for hiding this comment

kubeedge-bot commented Nov 30, 2021

chenchunxiu commented Nov 8, 2021 •

edited

Loading

chenchunxiu Nov 27, 2021 •

edited

Loading

gy95 Nov 27, 2021 •

edited

Loading