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

feat: Implement traffic splitting plugin #2935

Merged
merged 31 commits into from
Dec 25, 2020

Conversation

Firstsawyou
Copy link
Contributor

@Firstsawyou Firstsawyou commented Dec 2, 2020

close #2303 close #2603

What this PR does / why we need it:

Implement a traffic splitting plugin, the plugin is named traffic-split. The main function of the plug-in is to divide the request traffic according to the specified ratio and divert it to the corresponding "upstream". The functions of Gray release, Blue-green release and Custom release can be realized through this plugin.

Related issue:
https://github.com/apache/apisix/issues/2303

https://github.com/apache/apisix/issues/2603

Mailing list discussion address:
https://lists.apache.org/thread.html/rf02dc53a4af5d98d2513d89256b47466934d129af06d0bdcdb49cc8e%40%3Cdev.apisix.apache.org%3E

Pre-submission checklist:

  • Did you explain what problem does this PR solve? Or what new features have been added?
  • Have you added corresponding test cases?
  • Have you modified the corresponding document?
  • Is this PR backward compatible? If it is not backward compatible, please discuss on the mailing list first

@moonming
Copy link
Member

moonming commented Dec 3, 2020

Please write a better title and desc

@Firstsawyou Firstsawyou changed the title feat: traffic split plugin. feat: Implement traffic splitting plugin Dec 3, 2020
@tokers
Copy link
Contributor

tokers commented Dec 3, 2020

@Firstsawyou CI failed, please fix it.

@Firstsawyou
Copy link
Contributor Author

Firstsawyou commented Dec 3, 2020

@spacewander @membphis @tokers
Help me take a look at this test case: https://github.com/apache/apisix/pull/2935/files#diff-1badbb10ecfbb255409af122651c06a0b308ba0d51e672180d4e78719dce7e88R128. When executing this test case, the following error message is prompted:

nginx: [emerg] Lua code block missing the closing long bracket "]=]" in /home/runner/work/apisix/apisix/t/servroot/conf/nginx.conf:148
nginx: [emerg] Lua code block missing the closing long bracket "]=]" in /home/runner/work/apisix/apisix/t/servroot/conf/nginx.conf:148
nginx: [emerg] Lua code block missing the closing long bracket "]=]" in /home/runner/work/apisix/apisix/t/servroot/conf/nginx.conf:148

I don't know what caused this.

@moonming
Copy link
Member

moonming commented Dec 3, 2020

@Firstsawyou What is the name of this plugin? the PR title and code file are not the same.

@Firstsawyou Firstsawyou changed the title feat: Implement traffic splitting plugin feat: Implement dynamic-upstream plugin Dec 3, 2020
@Firstsawyou
Copy link
Contributor Author

@Firstsawyou What is the name of this plugin? the PR title and code file are not the same.

updated.

@moonming
Copy link
Member

moonming commented Dec 3, 2020

@Firstsawyou What is the name of this plugin? the PR title and code file are not the same.

updated.

Not yet , please read your desc again

=== TEST 5: Multiple vars rules and multiple plugin upstream
--- config
location /t {
content_by_lua_block {
Copy link
Member

Choose a reason for hiding this comment

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

Reduce the total code length in content_by_lua_block

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you very much, I will try.

@tokers
Copy link
Contributor

tokers commented Dec 3, 2020

We should use traffic split instead dynamic-upstream, in the future, the traffic split plugin will provide the labels based selector to choose nodes inside a upstream, this function is not covered by the name dynamic-upstream, which gives us an illusion this plugin is used for choosing one upstream from multiple upstreams.

@Firstsawyou
Copy link
Contributor Author

We should use traffic split instead dynamic-upstream, in the future, the traffic split plugin will provide the labels based selector to choose nodes inside a upstream, this function is not covered by the name dynamic-upstream, which gives us an illusion this plugin is used for choosing one upstream from multiple upstreams.

Ok i will update later.

@Firstsawyou Firstsawyou changed the title feat: Implement dynamic-upstream plugin feat: Implement traffic splitting plugin Dec 4, 2020
@Firstsawyou
Copy link
Contributor Author

There was a problem when this pr handled the conflict. A related pr has been newly submitted.

@Firstsawyou Firstsawyou closed this Dec 6, 2020
@moonming
Copy link
Member

moonming commented Dec 7, 2020

No, please fix it in this PR

@Firstsawyou Firstsawyou reopened this Dec 7, 2020
@Firstsawyou Firstsawyou force-pushed the dynamic-upstream-plugin branch 3 times, most recently from 6d88ccb to f11e9b2 Compare December 7, 2020 08:36
@Firstsawyou Firstsawyou marked this pull request as ready for review December 7, 2020 09:08
doc/plugins/traffic-split.md Outdated Show resolved Hide resolved
doc/plugins/traffic-split.md Outdated Show resolved Hide resolved
doc/plugins/traffic-split.md Outdated Show resolved Hide resolved
doc/plugins/traffic-split.md Outdated Show resolved Hide resolved
apisix/plugins/traffic-split.lua Outdated Show resolved Hide resolved
@tokers
Copy link
Contributor

tokers commented Dec 9, 2020

@Firstsawyou Could you paste some sample traffic split configurations here to help us understand the schema better.

apisix/plugins/traffic-split.lua Outdated Show resolved Hide resolved
apisix/plugins/traffic-split.lua Show resolved Hide resolved
apisix/plugins/traffic-split.lua Show resolved Hide resolved
apisix/plugins/traffic-split.lua Show resolved Hide resolved
@tokers
Copy link
Contributor

tokers commented Dec 9, 2020

@Firstsawyou Please resolve the conflicts.

@tokers tokers self-requested a review December 9, 2020 09:39
@tokers tokers self-requested a review December 9, 2020 09:46
@Firstsawyou
Copy link
Contributor Author

@Firstsawyou Could you paste some sample traffic split configurations here to help us understand the schema better.

@tokers These are two cases, which will help you understand:

Grayscale Release

Traffic is split according to the weight value configured by upstreams in the plugin (the rule of match is not configured, and match is passed by default). The request traffic is divided into 4:2, 2/3 of the traffic reaches the upstream of the 1981 port in the plugin, and 1/3 of the traffic reaches the upstream of the default 1980 port on the route.

{
    "weight": 2
}

There is only a weight value in the plugin upstreams, which represents the weight value of the upstream traffic arriving on the route.

curl http://127.0.0.1:9080/apisix/admin/routes/1 -H 'X-API-KEY: edd1c9f034335f136f87ad84b625c8f1' -X PUT -d '
{
    "uri": "/index.html",
    "plugins": {
        "traffic-split": {
            "rules": [
                {
                    "upstreams": [
                        {
                            "upstream": {
                                "name": "upstream_A",
                                "type": "roundrobin",
                                "nodes": {
                                    "127.0.0.1:1981":10
                                },
                                "timeout": {
                                    "connect": 15,
                                    "send": 15,
                                    "read": 15
                                }
                            },
                            "weight": 4
                        },
                        {
                            "weight": 2
                        }
                    ]
                }
            ]
        }
    },
    "upstream": {
            "type": "roundrobin",
            "nodes": {
                "127.0.0.1:1980": 1
            }
    }
}'

Custom Release

Multiple matching rules can be set in match (multiple conditions in vars are the relationship of add, and the relationship between multiple vars rules is the relationship of or; as long as one of the vars rules passes, it means match passed), only one is configured here, and the traffic is divided into 4:2 according to the value of weight. Among them, only the weight part represents the proportion of upstream on the route. When match fails to match, all traffic will only hit upstream on the route.

curl http://127.0.0.1:9080/apisix/admin/routes/1 -H 'X-API-KEY: edd1c9f034335f136f87ad84b625c8f1' -X PUT -d '
{
    "uri": "/index.html",
    "plugins": {
        "traffic-split": {
            "rules": [
                {
                    "match": [
                        {
                            "vars": [
                                ["arg_name","==","jack"],
                                ["http_user-id",">","23"],
                                ["http_apisix-key","~~","[a-z]+"]
                            ]
                        }
                    ],
                    "upstreams": [
                        {
                            "upstream": {
                                "name": "upstream_A",
                                "type": "roundrobin",
                                "nodes": {
                                    "127.0.0.1:1981":10
                                }
                            },
                            "weight": 4
                        },
                        {
                            "weight": 2
                        }
                    ]
                }
            ]
        }
    },
    "upstream": {
            "type": "roundrobin",
            "nodes": {
                "127.0.0.1:1980": 1
            }
    }
}'

@moonming moonming added this to the 2.2 milestone Dec 24, 2020
doc/README.md Outdated Show resolved Hide resolved
doc/plugins/traffic-split.md Outdated Show resolved Hide resolved
doc/zh-cn/plugins/traffic-split.md Outdated Show resolved Hide resolved
doc/zh-cn/plugins/traffic-split.md Outdated Show resolved Hide resolved
doc/zh-cn/plugins/traffic-split.md Outdated Show resolved Hide resolved
doc/zh-cn/plugins/traffic-split.md Outdated Show resolved Hide resolved
doc/zh-cn/plugins/traffic-split.md Outdated Show resolved Hide resolved
doc/zh-cn/plugins/traffic-split.md Outdated Show resolved Hide resolved
doc/plugins/traffic-split.md Outdated Show resolved Hide resolved
doc/plugins/traffic-split.md Outdated Show resolved Hide resolved
doc/zh-cn/plugins/traffic-split.md Outdated Show resolved Hide resolved
doc/zh-cn/plugins/traffic-split.md Outdated Show resolved Hide resolved
doc/plugins/traffic-split.md Outdated Show resolved Hide resolved
doc/plugins/traffic-split.md Outdated Show resolved Hide resolved
Comment on lines +263 to +264
["http_user-id",">","23"],
["http_apisix-key","~~","[a-z]+"]
Copy link
Member

Choose a reason for hiding this comment

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

This should not work :(

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
7 participants