fix edgemesh-agent proxy fetching protocol bug #175

king-jingxiang · 2021-11-10T09:17:47Z

Signed-off-by: JingXiang jingxiang@leinao.ai
#173

kubeedge-bot · 2021-11-10T09:17:56Z

Welcome @king-jingxiang! It looks like this is your first PR to kubeedge/edgemesh 🎉

khalid-huang · 2021-11-10T09:32:26Z

agent/pkg/chassis/registry/registry.go

-			protocol = strings.Split(p.Name, "-")[0]
+			pro := strings.Split(p.Name, "-")
+			if len(pro) >= 2 {
+				protocol = pro[0]


I think we need to consider the validity of the value pro. EdgeMesh only support http, tcp now. If the value of pro is not valid, we can fetch the p.Protocol or just set the tcp

yes, the value of pro needs to check. And the websocket、grpc protocol have any plan to support it?

Websocket is support by http protocol, link. The grpc will support if it is necessary.

khalid-huang · 2021-11-10T09:33:23Z

@Poorunga

khalid-huang · 2021-11-15T09:24:10Z

agent/pkg/chassis/registry/registry.go

@@ -176,6 +176,10 @@ func getPortAndProtocol(svc *v1.Service, svcPort int) (targetPort int, protocol
 	for _, p := range svc.Spec.Ports {
 		if p.Protocol == "TCP" && int(p.Port) == svcPort {
 			protocol = strings.Split(p.Name, "-")[0]
+			if protocol != "http" {


关于protocol的提取，从代码的可扩展性出发，我们是不是考虑如下的策略会比较好：

默认从port.Name提取第一个字段，校验这个字段是不是合法有效的协议；如果不是合法的协议我们就从port.Protocol获取，如果port.Protocol没有值，我们就默认使用TCP

关于合法有效的定义，为了可扩展性，可以让每个协议实现的时候都把自己注册到一个合法有效的协议列表；然后校验协议的有效性的时候，判断协议是不是在这个列表里面

k8s service支持的port.Protocol有以下几种，在创建svc时不设置Protocol，默认会被赋值为TCP。从name中提取的应用层协议，类型会有很多，而且目前仅支持http，对于其它类型的协议会导致连接问题，应用层的协议需要维护一个列表进行校验吗？

const ( // ProtocolTCP is the TCP protocol. ProtocolTCP Protocol = "TCP" // ProtocolUDP is the UDP protocol. ProtocolUDP Protocol = "UDP" // ProtocolSCTP is the SCTP protocol. ProtocolSCTP Protocol = "SCTP" )

name其实可以是四层协议，比如“tcp-0”，我们要优先从name里面提出出合法有效的协议名称。为了把这个过程做成可扩展的，这个名称我理解要从/pkg/chassis/protocol里面注册出来，比如pkg/chassis/protocol/protocol.go里面的Protocol接口可以定义一个register方法，然后他的实现类都去实现这个方法，把自己的协议名称都注册到一个数组里面。在做协议合法有效校验的时候，就校验协议名称是不是在这个数组里面就可以了。这是我个人的看法，我们可以讨论下有没有更好的方法实现这个

参照于istio支持的协议，有以下协议可能作为svc port name的前缀 http,http2,https,tcp,tls,grpc,grpc-web,mongo,mysql,redis,udp

你好，按照之前说的，我又修改了一下，麻烦帮忙看一下这么修改可以吗

提了一点小建议，其他我这边觉得ok，多谢你的贡献

agent/pkg/chassis/registry/registry.go

agent/pkg/proxy/controller/controller.go

khalid-huang · 2021-11-16T02:47:23Z

/lgtm

khalid-huang · 2021-11-16T02:48:38Z

/close

kubeedge-bot · 2021-11-16T02:48:41Z

@khalid-jobs: Closed this PR.

In response to this:

/close

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.

khalid-huang · 2021-11-16T02:48:50Z

/reopen

kubeedge-bot · 2021-11-16T02:48:53Z

@khalid-jobs: Reopened this PR.

In response to this:

/reopen

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.

khalid-huang · 2021-11-16T02:51:37Z

I do not know why ci do not start at first. I reopen the pr and the ci has started. Please make sure all the ci items pass.
And we recommand compressing all commits into one, you can refer link.

Poorunga · 2021-11-16T02:54:07Z

agent/pkg/chassis/protocol/http/http.go

@@ -21,6 +21,7 @@ import (
 	"bytes"
 	"errors"
 	"fmt"
+	"github.com/kubeedge/edgemesh/agent/pkg/chassis/protocol"


go语言包的引入顺序请遵守以下规则：

内置包

第三方包

项目内部包

agent/pkg/proxy/iptables.go

khalid-huang · 2021-11-16T03:37:15Z

这里的commit信息，建议做下删减

可以参考下

king-jingxiang · 2021-11-16T03:52:51Z

改好了

khalid-huang · 2021-11-16T06:07:45Z

/close

kubeedge-bot · 2021-11-16T06:07:48Z

@khalid-jobs: Closed this PR.

In response to this:

/close

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.

Poorunga · 2021-11-16T08:39:55Z

/reopen

kubeedge-bot · 2021-11-16T08:39:57Z

@Poorunga: Reopened this PR.

In response to this:

/reopen

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.

khalid-huang · 2021-11-16T08:58:57Z

你可以使用go fmt处理下这个文件，go fmt agent/pkg/chassis/protocol/protocol.go

khalid-huang · 2021-11-16T10:48:12Z

/close

kubeedge-bot · 2021-11-16T10:48:16Z

@khalid-jobs: Closed this PR.

In response to this:

/close

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.

khalid-huang · 2021-11-16T10:48:19Z

/reopen

kubeedge-bot · 2021-11-16T10:48:21Z

@khalid-jobs: Reopened this PR.

In response to this:

/reopen

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.

Poorunga · 2021-11-16T11:06:26Z

@king-jingxiang CI已经通过了，还有两件事：
1.文件edgemesh/agent/pkg/proxy/iptables.go里面item.DeepCopy一共有两处需要修改的地方，请再修改一下
2.将多个commit合并成1个

king-jingxiang · 2021-11-17T01:04:33Z

@king-jingxiang CI已经通过了，还有两件事： 1.文件edgemesh/agent/pkg/proxy/iptables.go里面item.DeepCopy一共有两处需要修改的地方，请再修改一下 2.将多个commit合并成1个

好的，我再修改一下，当时看kube-dns只有一个svc，就没有改那个

Signed-off-by: JingXiang <jingxiang@leinao.ai>

khalid-huang · 2021-11-17T01:30:48Z

/close

kubeedge-bot · 2021-11-17T01:30:52Z

@khalid-jobs: Closed this PR.

In response to this:

/close

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.

khalid-huang · 2021-11-17T01:30:58Z

/reopen

kubeedge-bot · 2021-11-17T01:31:01Z

@khalid-jobs: Reopened this PR.

In response to this:

/reopen

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.

khalid-huang · 2021-11-17T02:03:00Z

/lgtm

Poorunga · 2021-11-17T02:14:41Z

/approve

kubeedge-bot · 2021-11-17T02:14:48Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Poorunga

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

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

kubeedge-bot requested review from fisherxu and kevin-wangzefeng November 10, 2021 09:17

kubeedge-bot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Nov 10, 2021

khalid-huang reviewed Nov 10, 2021

View reviewed changes

khalid-huang reviewed Nov 15, 2021

View reviewed changes

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 Nov 16, 2021

khalid-huang reviewed Nov 16, 2021

View reviewed changes

agent/pkg/chassis/registry/registry.go Show resolved Hide resolved

agent/pkg/proxy/controller/controller.go Show resolved Hide resolved

kubeedge-bot assigned khalid-huang Nov 16, 2021

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

kubeedge-bot closed this Nov 16, 2021

kubeedge-bot reopened this Nov 16, 2021

Poorunga reviewed Nov 16, 2021

View reviewed changes

king-jingxiang force-pushed the leinao-fix-protocol-pr branch from ac65102 to f3a4b65 Compare November 16, 2021 03:17

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

Poorunga reviewed Nov 16, 2021

View reviewed changes

agent/pkg/proxy/iptables.go Show resolved Hide resolved

king-jingxiang force-pushed the leinao-fix-protocol-pr branch from f3a4b65 to 63c7b20 Compare November 16, 2021 03:51

kubeedge-bot closed this Nov 16, 2021

kubeedge-bot reopened this Nov 16, 2021

kubeedge-bot closed this Nov 16, 2021

kubeedge-bot reopened this Nov 16, 2021

fix fetching protocol

113fe55

Signed-off-by: JingXiang <jingxiang@leinao.ai>

king-jingxiang force-pushed the leinao-fix-protocol-pr branch from 44a44bc to 113fe55 Compare November 17, 2021 01:07

kubeedge-bot closed this Nov 17, 2021

kubeedge-bot reopened this Nov 17, 2021

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

kubeedge-bot assigned Poorunga Nov 17, 2021

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

kubeedge-bot merged commit d952173 into kubeedge:main Nov 17, 2021

This was referenced Nov 17, 2021

Enhance the ease of use of EdgeMesh #132

Closed

The service port name restriction should be removed #105

Closed

king-jingxiang deleted the leinao-fix-protocol-pr branch November 26, 2021 06:05

fix edgemesh-agent proxy fetching protocol bug #175

fix edgemesh-agent proxy fetching protocol bug #175

Conversation

king-jingxiang commented Nov 10, 2021

kubeedge-bot commented Nov 10, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

khalid-huang commented Nov 10, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

khalid-huang Nov 16, 2021 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

khalid-huang Nov 16, 2021 • edited

Choose a reason for hiding this comment

khalid-huang commented Nov 16, 2021

khalid-huang commented Nov 16, 2021

kubeedge-bot commented Nov 16, 2021

khalid-huang commented Nov 16, 2021

kubeedge-bot commented Nov 16, 2021

khalid-huang commented Nov 16, 2021 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

khalid-huang commented Nov 16, 2021

king-jingxiang commented Nov 16, 2021

khalid-huang commented Nov 16, 2021

kubeedge-bot commented Nov 16, 2021

Poorunga commented Nov 16, 2021

kubeedge-bot commented Nov 16, 2021

khalid-huang commented Nov 16, 2021

khalid-huang commented Nov 16, 2021

kubeedge-bot commented Nov 16, 2021

khalid-huang commented Nov 16, 2021

kubeedge-bot commented Nov 16, 2021

Poorunga commented Nov 16, 2021

king-jingxiang commented Nov 17, 2021

khalid-huang commented Nov 17, 2021

kubeedge-bot commented Nov 17, 2021

khalid-huang commented Nov 17, 2021

kubeedge-bot commented Nov 17, 2021

khalid-huang commented Nov 17, 2021

Poorunga commented Nov 17, 2021

kubeedge-bot commented Nov 17, 2021

khalid-huang Nov 16, 2021 •

edited

khalid-huang Nov 16, 2021 •

edited

khalid-huang commented Nov 16, 2021 •

edited