You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@apisix.apache.org by GitBox <gi...@apache.org> on 2020/09/22 12:49:38 UTC
[GitHub] [apisix] imjoey opened a new pull request #2279: feat: Add labels for upstream object
imjoey opened a new pull request #2279:
URL: https://github.com/apache/apisix/pull/2279
Signed-off-by: imjoey <ma...@gmail.com>
### What this PR does / why we need it:
Hi fellows, we could use `labels` (as it in Kubernetes) to specify attributes that attached an object. This is a kind of extensibility mechanism for developers. Once I had a short discussion with @moonming at https://github.com/apache/apisix-dashboard/issues/431#issuecomment-684139567 , so this PR is going to take `upstream` as the starting point and add a new `labels` property, via a rather simple implementation.
Looking forward to the feedback. Thanks.
### Pre-submission checklist:
* [x] Did you explain what problem does this PR solve? Or what new features have been added?
* [x] Have you added corresponding test cases?
* [x] Have you modified the corresponding document?
* [x] Is this PR backward compatible?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] imjoey commented on pull request #2279: feat: Add labels for upstream object
Posted by GitBox <gi...@apache.org>.
imjoey commented on pull request #2279:
URL: https://github.com/apache/apisix/pull/2279#issuecomment-697208483
> This is a very interesting topic.
> I hope to add labels to all objects in apisix, so that some strong bindings through `xxx_id` can be changed to loose binding relationships through `labels`. this is very useful and better fit with etcd design.
@gxthrj yep, glad to hear your thoughts. Yesterday I was about to migrate the functionality of RouteGroup from MySQL to etcd. But it seemed still under discussions in community. So I deliberated on the whole design, I guess labels could make the relationships of objects in APISIX much more loosely coupled, and also make the extensibility more easily.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] membphis commented on a change in pull request #2279: feat: Add labels for upstream object
Posted by GitBox <gi...@apache.org>.
membphis commented on a change in pull request #2279:
URL: https://github.com/apache/apisix/pull/2279#discussion_r493163377
##########
File path: apisix/schema_def.lua
##########
@@ -332,6 +332,10 @@ local upstream_schema = {
description = "enable websocket for request",
type = "boolean"
},
+ labels = {
+ description = "key/value pairs to specify attributes",
Review comment:
Why we need a `key-value` table for labels? I think array object is simpler.
##########
File path: apisix/schema_def.lua
##########
@@ -332,6 +332,10 @@ local upstream_schema = {
description = "enable websocket for request",
type = "boolean"
},
+ labels = {
+ description = "key/value pairs to specify attributes",
+ type = "table"
Review comment:
I think it should be `object` here.
##########
File path: apisix/schema_def.lua
##########
@@ -332,6 +332,10 @@ local upstream_schema = {
description = "enable websocket for request",
type = "boolean"
},
+ labels = {
+ description = "key/value pairs to specify attributes",
+ type = "table"
Review comment:
Need to limit the depth to `1`
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] imjoey commented on a change in pull request #2279: feat: Add labels for upstream object
Posted by GitBox <gi...@apache.org>.
imjoey commented on a change in pull request #2279:
URL: https://github.com/apache/apisix/pull/2279#discussion_r495651788
##########
File path: doc/zh-cn/architecture-design.md
##########
@@ -273,6 +273,7 @@ APISIX 的 Upstream 除了基本的复杂均衡算法选择外，还支持对上
|checks |可选|配置健康检查的参数，详细可参考[health-check](../health-check.md)|
|retries |可选|使用底层的 Nginx 重试机制将请求传递给下一个上游，默认 APISIX 会启用重试机制，根据配置的后端节点个数设置重试次数，如果此参数显式被设置将会覆盖系统默认设置的重试次数。|
|enable_websocket|可选| 是否启用 `websocket`（布尔值），默认不启用|
+|labels |可选| 用于标识属性的键值对。 |
Review comment:
@membphis thanks for reviewing. I will add more guidance docs about labels usage and best practice in APISIX after labels are added in all objects.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] membphis commented on pull request #2279: feat: Add labels for upstream object
Posted by GitBox <gi...@apache.org>.
membphis commented on pull request #2279:
URL: https://github.com/apache/apisix/pull/2279#issuecomment-699747432
@imjoey many thx, merged
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] imjoey commented on a change in pull request #2279: feat: Add labels for upstream object
Posted by GitBox <gi...@apache.org>.
imjoey commented on a change in pull request #2279:
URL: https://github.com/apache/apisix/pull/2279#discussion_r493305139
##########
File path: apisix/schema_def.lua
##########
@@ -332,6 +332,10 @@ local upstream_schema = {
description = "enable websocket for request",
type = "boolean"
},
+ labels = {
+ description = "key/value pairs to specify attributes",
+ type = "table"
Review comment:
@membphis agreed, nice catch, thanks. I would make changes as the your comments and add one more failure test case after I find the lua way. 😄
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] membphis commented on a change in pull request #2279: feat: Add labels for upstream object
Posted by GitBox <gi...@apache.org>.
membphis commented on a change in pull request #2279:
URL: https://github.com/apache/apisix/pull/2279#discussion_r495573065
##########
File path: doc/zh-cn/architecture-design.md
##########
@@ -273,6 +273,7 @@ APISIX 的 Upstream 除了基本的复杂均衡算法选择外，还支持对上
|checks |可选|配置健康检查的参数，详细可参考[health-check](../health-check.md)|
|retries |可选|使用底层的 Nginx 重试机制将请求传递给下一个上游，默认 APISIX 会启用重试机制，根据配置的后端节点个数设置重试次数，如果此参数显式被设置将会覆盖系统默认设置的重试次数。|
|enable_websocket|可选| 是否启用 `websocket`（布尔值），默认不启用|
+|labels |可选| 用于标识属性的键值对。 |
Review comment:
need more doc, we need an example about how to use this field.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] tokers commented on pull request #2279: feat: Add labels for upstream object
Posted by GitBox <gi...@apache.org>.
tokers commented on pull request #2279:
URL: https://github.com/apache/apisix/pull/2279#issuecomment-697108446
@imjoey One thing confuses me that the labels field is attached on the whole `upstream` object, It's closer to the labels on Kubernetes Service, not the Pod, and labels on Pod are more pragmatic, so what about attaching labels for each node in this upstream, with this support, we can group the nodes by labels, and in turn implementing some features like blue green release, canary release or other route rules.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] tokers commented on pull request #2279: feat: Add labels for upstream object
Posted by GitBox <gi...@apache.org>.
tokers commented on pull request #2279:
URL: https://github.com/apache/apisix/pull/2279#issuecomment-697108446
@imjoey One thing confuses me that the labels field is attached on the whole `upstream` object, It's closer to the labels on Kubernetes Service, not the Pod, and labels on Pod are more pragmatic, so what about attaching labels for each node in this upstream, with this support, we can group the nodes by labels, and in turn implementing some features like blue green release, canary release or other route rules.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] imjoey commented on pull request #2279: feat: Add labels for upstream object
Posted by GitBox <gi...@apache.org>.
imjoey commented on pull request #2279:
URL: https://github.com/apache/apisix/pull/2279#issuecomment-697196007
> @imjoey One thing confuses me that the labels field is attached on the whole `upstream` object, It's closer to the labels on Kubernetes Service, not the Pod, and labels on Pod are more pragmatic, so what about attaching labels for each node in this upstream, with this support, we can group the nodes by labels, and in turn implementing some features like blue green release, canary release or other route rules.
Hi @tokers , thanks for your feedback. I guess labels could be very useful for every single object in APISIX. While currently `Node` is not an independent object so we are not able to mange the labels attached. We may refer to the existing field named `metadata` for your requirements.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] imjoey commented on a change in pull request #2279: feat: Add labels for upstream object
Posted by GitBox <gi...@apache.org>.
imjoey commented on a change in pull request #2279:
URL: https://github.com/apache/apisix/pull/2279#discussion_r493305139
##########
File path: apisix/schema_def.lua
##########
@@ -332,6 +332,10 @@ local upstream_schema = {
description = "enable websocket for request",
type = "boolean"
},
+ labels = {
+ description = "key/value pairs to specify attributes",
+ type = "table"
Review comment:
@membphis agreed, nice catch, thanks. I would make changes as the your suggestions and add one more failure test case after I find the lua way. 😄
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] tokers commented on pull request #2279: feat: Add labels for upstream object
Posted by GitBox <gi...@apache.org>.
tokers commented on pull request #2279:
URL: https://github.com/apache/apisix/pull/2279#issuecomment-698245062
@imjoey @gxthrj @nic-chen @membphis @moonming I have submitted an issue about traffic split https://github.com/apache/apisix/issues/2303.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] membphis commented on a change in pull request #2279: feat: Add labels for upstream object
Posted by GitBox <gi...@apache.org>.
membphis commented on a change in pull request #2279:
URL: https://github.com/apache/apisix/pull/2279#discussion_r493163377
##########
File path: apisix/schema_def.lua
##########
@@ -332,6 +332,10 @@ local upstream_schema = {
description = "enable websocket for request",
type = "boolean"
},
+ labels = {
+ description = "key/value pairs to specify attributes",
Review comment:
Why we need a `key-value` table for labels? I think array object is simpler.
##########
File path: apisix/schema_def.lua
##########
@@ -332,6 +332,10 @@ local upstream_schema = {
description = "enable websocket for request",
type = "boolean"
},
+ labels = {
+ description = "key/value pairs to specify attributes",
+ type = "table"
Review comment:
I think it should be `object` here.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] nic-chen commented on a change in pull request #2279: feat: Add labels for upstream object
Posted by GitBox <gi...@apache.org>.
nic-chen commented on a change in pull request #2279:
URL: https://github.com/apache/apisix/pull/2279#discussion_r493262202
##########
File path: apisix/schema_def.lua
##########
@@ -332,6 +332,10 @@ local upstream_schema = {
description = "enable websocket for request",
type = "boolean"
},
+ labels = {
+ description = "key/value pairs to specify attributes",
Review comment:
I think `key-value` table is ok. but we need to set some limit to it, such as `maxProperties`, item type and length ...
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] imjoey commented on a change in pull request #2279: feat: Add labels for upstream object
Posted by GitBox <gi...@apache.org>.
imjoey commented on a change in pull request #2279:
URL: https://github.com/apache/apisix/pull/2279#discussion_r494889346
##########
File path: apisix/schema_def.lua
##########
@@ -332,6 +332,10 @@ local upstream_schema = {
description = "enable websocket for request",
type = "boolean"
},
+ labels = {
+ description = "key/value pairs to specify attributes",
+ type = "table"
Review comment:
@membphis PR is updated and labels are limited to string key/value pairs, so fix the depth problem. 😄
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] imjoey commented on pull request #2279: feat: Add labels for upstream object
Posted by GitBox <gi...@apache.org>.
imjoey commented on pull request #2279:
URL: https://github.com/apache/apisix/pull/2279#issuecomment-699594693
@membphis @nic-chen @gxthrj PR updated and any feedback would be much appreciated.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] imjoey commented on a change in pull request #2279: feat: Add labels for upstream object
Posted by GitBox <gi...@apache.org>.
imjoey commented on a change in pull request #2279:
URL: https://github.com/apache/apisix/pull/2279#discussion_r494888017
##########
File path: apisix/schema_def.lua
##########
@@ -332,6 +332,10 @@ local upstream_schema = {
description = "enable websocket for request",
type = "boolean"
},
+ labels = {
+ description = "key/value pairs to specify attributes",
Review comment:
Hi @membphis @nic-chen @gxthrj , PR is updated and the format of value in `labels` is limited to `string`, with set maxProperties to 16. A new test case which contains invalid value format is also added. Feel free to leave your comments. Thanks.
##########
File path: apisix/schema_def.lua
##########
@@ -332,6 +332,10 @@ local upstream_schema = {
description = "enable websocket for request",
type = "boolean"
},
+ labels = {
+ description = "key/value pairs to specify attributes",
+ type = "table"
Review comment:
@membphis PR is updated and labels are limited to string key/value pairs, so fix the depth problem. 😄
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] membphis commented on a change in pull request #2279: feat: Add labels for upstream object
Posted by GitBox <gi...@apache.org>.
membphis commented on a change in pull request #2279:
URL: https://github.com/apache/apisix/pull/2279#discussion_r495572938
##########
File path: apisix/schema_def.lua
##########
@@ -332,6 +332,10 @@ local upstream_schema = {
description = "enable websocket for request",
type = "boolean"
},
+ labels = {
+ description = "key/value pairs to specify attributes",
Review comment:
ok, got it. many thx for your explain.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] imjoey edited a comment on pull request #2279: feat: Add labels for upstream object
Posted by GitBox <gi...@apache.org>.
imjoey edited a comment on pull request #2279:
URL: https://github.com/apache/apisix/pull/2279#issuecomment-697196007
> @imjoey One thing confuses me that the labels field is attached on the whole `upstream` object, It's closer to the labels on Kubernetes Service, not the Pod, and labels on Pod are more pragmatic, so what about attaching labels for each node in this upstream, with this support, we can group the nodes by labels, and in turn implementing some features like blue green release, canary release or other route rules.
Hi @tokers , thanks for your feedback. I guess labels could be very useful for every single object in APISIX. While currently `Node` is not an independent object so we are not able to mange the labels attached. We may refer to the existing field named `metadata` in `Node` for your requirements.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] gxthrj commented on pull request #2279: feat: Add labels for upstream object
Posted by GitBox <gi...@apache.org>.
gxthrj commented on pull request #2279:
URL: https://github.com/apache/apisix/pull/2279#issuecomment-697198175
> @imjoey One thing confuses me that the labels field is attached on the whole `upstream` object, It's closer to the labels on Kubernetes Service, not the Pod, and labels on Pod are more pragmatic, so what about attaching labels for each node in this upstream, with this support, we can group the nodes by labels, and in turn implementing some features like blue green release, canary release or other route rules.
Your idea is great,
But we also need to pay attention to the structure of APISIX,
1. The `nodes` under `upstream` is not an object yet;
2. Blue-green or canary releases are more tied to business, not suitable for writing in upstream objects, but more suitable for extension in plug-ins, But upstream currently does not provide any plug-in implementation;
if we add labels in nodes, I think it will be a big change.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] gxthrj commented on a change in pull request #2279: feat: Add labels for upstream object
Posted by GitBox <gi...@apache.org>.
gxthrj commented on a change in pull request #2279:
URL: https://github.com/apache/apisix/pull/2279#discussion_r493261175
##########
File path: apisix/schema_def.lua
##########
@@ -332,6 +332,10 @@ local upstream_schema = {
description = "enable websocket for request",
type = "boolean"
},
+ labels = {
+ description = "key/value pairs to specify attributes",
Review comment:
I think `key-value` is more descriptive, and we can make some conventions on the `key`, which is more readable.
e.g.
```
lables: {
"version": "v1"
}
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] membphis commented on a change in pull request #2279: feat: Add labels for upstream object
Posted by GitBox <gi...@apache.org>.
membphis commented on a change in pull request #2279:
URL: https://github.com/apache/apisix/pull/2279#discussion_r493169984
##########
File path: apisix/schema_def.lua
##########
@@ -332,6 +332,10 @@ local upstream_schema = {
description = "enable websocket for request",
type = "boolean"
},
+ labels = {
+ description = "key/value pairs to specify attributes",
+ type = "table"
Review comment:
Need to limit the depth to `1`
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] imjoey commented on a change in pull request #2279: feat: Add labels for upstream object
Posted by GitBox <gi...@apache.org>.
imjoey commented on a change in pull request #2279:
URL: https://github.com/apache/apisix/pull/2279#discussion_r494888017
##########
File path: apisix/schema_def.lua
##########
@@ -332,6 +332,10 @@ local upstream_schema = {
description = "enable websocket for request",
type = "boolean"
},
+ labels = {
+ description = "key/value pairs to specify attributes",
Review comment:
Hi @membphis @nic-chen @gxthrj , PR is updated and the format of value in `labels` is limited to `string`, with set maxProperties to 16. A new test case which contains invalid value format is also added. Feel free to leave your comments. Thanks.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] imjoey commented on pull request #2279: feat: Add labels for upstream object
Posted by GitBox <gi...@apache.org>.
imjoey commented on pull request #2279:
URL: https://github.com/apache/apisix/pull/2279#issuecomment-697212801
> > > @imjoey One thing confuses me that the labels field is attached on the whole `upstream` object, It's closer to the labels on Kubernetes Service, not the Pod, and labels on Pod are more pragmatic, so what about attaching labels for each node in this upstream, with this support, we can group the nodes by labels, and in turn implementing some features like blue green release, canary release or other route rules.
> >
> >
> > Your idea is great,
> > But we also need to pay attention to the structure of APISIX,
> >
> > 1. The `nodes` under `upstream` is not an object yet;
> > 2. Blue-green or canary releases are more tied to business, not suitable for writing in upstream objects, but more suitable for extension in plug-ins, But upstream currently does not provide any plug-in implementation;
> > if we add labels in nodes, I think it will be a big change.
>
> I think we don't need to separate nodes from upstream, just redefine the data structure of nodes.
> Of course, there is also a lot of work in this way, but it is better to understand.
@nic-chen Just FYI, we could make use of the existing `metadata` field in `node`, pls refer to https://github.com/apache/apisix/blob/master/apisix/schema_def.lua#L263 for details.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] imjoey commented on a change in pull request #2279: feat: Add labels for upstream object
Posted by GitBox <gi...@apache.org>.
imjoey commented on a change in pull request #2279:
URL: https://github.com/apache/apisix/pull/2279#discussion_r493273076
##########
File path: apisix/schema_def.lua
##########
@@ -332,6 +332,10 @@ local upstream_schema = {
description = "enable websocket for request",
type = "boolean"
},
+ labels = {
+ description = "key/value pairs to specify attributes",
Review comment:
@membphis I guess key-value may be much useful for marking/filtering/grouping objects, such as we could use the labels `{"env": "release"}` and `{"env": "testing"}` to distinguish the publishing stage for a Route.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] tokers commented on pull request #2279: feat: Add labels for upstream object
Posted by GitBox <gi...@apache.org>.
tokers commented on pull request #2279:
URL: https://github.com/apache/apisix/pull/2279#issuecomment-697242300
> > @imjoey One thing confuses me that the labels field is attached on the whole `upstream` object, It's closer to the labels on Kubernetes Service, not the Pod, and labels on Pod are more pragmatic, so what about attaching labels for each node in this upstream, with this support, we can group the nodes by labels, and in turn implementing some features like blue green release, canary release or other route rules.
>
> Your idea is great,
> But we also need to pay attention to the structure of APISIX,
>
> 1. The `nodes` under `upstream` is not an object yet;
> 2. Blue-green or canary releases are more tied to business, not suitable for writing in upstream objects, but more suitable for extension in plug-ins, But upstream currently does not provide any plug-in implementation;
> if we add labels in nodes, I think it will be a big change.
1. I think whether the `Node` is embedded in `Upstream` or as a separate object doesn't matter, even in XDS protocol, the CDS does not necessarily depend on EDS.
2. What we can do is not implementing these route rules in upstream logic, instead, a traffic split plugin can be introduced to solve these problems, I will submit an issue this few days, with some details.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] membphis merged pull request #2279: feat: Add labels for upstream object
Posted by GitBox <gi...@apache.org>.
membphis merged pull request #2279:
URL: https://github.com/apache/apisix/pull/2279
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] gxthrj commented on pull request #2279: feat: Add labels for upstream object
Posted by GitBox <gi...@apache.org>.
gxthrj commented on pull request #2279:
URL: https://github.com/apache/apisix/pull/2279#issuecomment-697194079
This is a very interesting topic.
I hope to add labels to all objects in apisix, so that some strong bindings through `xxx_id` can be changed to loose binding relationships through `labels`. this is very useful and better fit with etcd design.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] nic-chen commented on pull request #2279: feat: Add labels for upstream object
Posted by GitBox <gi...@apache.org>.
nic-chen commented on pull request #2279:
URL: https://github.com/apache/apisix/pull/2279#issuecomment-697211199
> > @imjoey One thing confuses me that the labels field is attached on the whole `upstream` object, It's closer to the labels on Kubernetes Service, not the Pod, and labels on Pod are more pragmatic, so what about attaching labels for each node in this upstream, with this support, we can group the nodes by labels, and in turn implementing some features like blue green release, canary release or other route rules.
>
> Your idea is great,
> But we also need to pay attention to the structure of APISIX,
>
> 1. The `nodes` under `upstream` is not an object yet;
> 2. Blue-green or canary releases are more tied to business, not suitable for writing in upstream objects, but more suitable for extension in plug-ins, But upstream currently does not provide any plug-in implementation;
> if we add labels in nodes, I think it will be a big change.
I think we don't need to separate nodes from upstream, just redefine the data structure of nodes.
Of course, there is also a lot of work in this way, but it is better to understand.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] tokers commented on pull request #2279: feat: Add labels for upstream object
Posted by GitBox <gi...@apache.org>.
tokers commented on pull request #2279:
URL: https://github.com/apache/apisix/pull/2279#issuecomment-698245062
@imjoey @gxthrj @nic-chen @membphis @moonming I have submitted an issue about traffic split https://github.com/apache/apisix/issues/2303.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org