feat: add consumer group by kingluo · Pull Request #7980 · apache/apisix

kingluo · 2022-09-23T09:18:53Z

Description

add consumer group

Checklist

I have explained the need for this PR and the problem it solves
I have explained the changes or the new features added to this PR
I have added tests corresponding to this change
I have updated the documentation to reflect this change
I have verified that this change is backward compatible (If not, please discuss on the APISIX mailing list first)

spacewander · 2022-09-25T12:44:13Z

+    conf.id = id
+
+    core.log.info("conf: ", core.json.delay_encode(conf))
+    local ok, err = core.schema.check(core.schema.plugin_config, conf)


Don't hijack the schema of plugin_config in other places.

spacewander · 2022-09-25T12:45:27Z

+end
+
+
+function _M.put(id, conf)


We should also add a check in consumers. Let's make sure consumer.group_id points to an actual consumer group.

spacewander · 2022-09-25T12:46:23Z

+    local err
+    consumer_groups, err = core.config.new("/consumer_groups", {
+        automatic = true,
+        item_schema = nil,


Why don't we use schema to check the item?

spacewander · 2022-09-25T12:48:44Z

+        $block->set_value("request", "GET /t");
+    }
+
+    if (!$block->no_error_log) {


It is more common to use this style:

apisix/t/admin/filter.t

Line 44 in 3bbc7ae

if ((!defined $block->error_log) && (!defined $block->no_error_log)) {

tzssangglass · 2022-09-26T02:13:29Z

+        }
+    }
+--- response_body
+passed


we need to verify that the plugin works well in the consumer_group

we need to verify that consumer_group and consumer work well together

Just like what other resources does, e.g. plugin_config, consumer, this test file is only for admin api test.
I would put plugin test and consumer stuff in t/node/consumer-group.t, and moreover, t/config-center-yaml/consumer-group.t, please keep in patient.

spacewander · 2022-09-27T05:57:20Z

    end

+    if conf.group_id then
+        if consumer_group.get(conf.group_id) == nil then


For code style, I will recommend using the same solution in

apisix/apisix/admin/routes.lua

Line 79 in 3c18031

if upstream_id then

fetch the data from etcd if possible

use a similar error message

spacewander · 2022-10-08T05:03:09Z

+
+When APISIX can't find the Consumer Group with the `group_id`, the Admin API is terminated with a status code of 400.
+
+If a Consumer already has the `plugins` field configured, the plugins in the Consumer Group will effectively be merged to it. The same plugin in the Consumer Group will not override the ones configured directly in the Consumer.


Suggested change

If a Consumer already has the `plugins` field configured, the plugins in the Consumer Group will effectively be merged to it. The same plugin in the Consumer Group will not override the ones configured directly in the Consumer.

If a Consumer already has the `plugins` field configured, the plugins in the Consumer Group will effectively be merged into it. The same plugin in the Consumer Group will not override the one configured directly in the Consumer.

spacewander · 2022-10-08T05:05:06Z

+instead of managing each consumer individually.
+
+While configuring the same plugin for the same route, only one copy of the configuration is valid.
+The order of precedence is `Consumer` > `Consumer Group` > `Route` > `plugin_config` > `Service`.


Better to put the order to one place:

apisix/docs/en/latest/terminology/plugin.md

Line 30 in 61afafd

While configuring the same plugin, only one copy of the configuration is valid. The order of precedence is always `Consumer` > `Route` > `Plugin Config` > `Service`.

spacewander · 2022-10-08T05:05:33Z

+
+For example, if we configure a Consumer Group as shown below
+
+```


Let's add language mark like other examples

spacewander · 2022-10-08T05:05:55Z

+#
+-->
+
+Consumer Groups are used to extract commonly used [Plugin](./plugin.md) configurations and can be bound directly to a [Consumer](./consumer.md).


We should also update the admin-api.md?

spacewander · 2022-10-08T05:07:17Z

    local apisix_var_names = {
        balancer_ip = true,
        balancer_port = true,
+        consumer_group_id = true,


Need to update https://github.com/apache/apisix/blob/master/docs/en/latest/apisix-variable.md for the new variable.

SylviaBABY · 2022-10-08T05:22:54Z

+
+If a Consumer already has the `plugins` field configured, the plugins in the Consumer Group will effectively be merged to it. The same plugin in the Consumer Group will not override the ones configured directly in the Consumer.
+
+For example, if we configure a Consumer Group as shown below


Add . or : in the end of sentence

SylviaBABY · 2022-10-08T05:23:55Z

+}
+```
+
+to a Consumer as shown below,


Suggested change

to a Consumer as shown below,

To a Consumer as shown below.

SylviaBABY · 2022-10-08T05:24:28Z

+
+to a Consumer as shown below,
+
+```


need to add code language too

SylviaBABY · 2022-10-08T05:25:01Z

+}'
+```
+
+When APISIX can't find the Consumer Group with the `group_id`, the Admin API is terminated with a status code of 400.


Suggested change

When APISIX can't find the Consumer Group with the `group_id`, the Admin API is terminated with a status code of 400.

When APISIX can't find the Consumer Group with the `group_id`, the Admin API is terminated with a status code of `400`.

spacewander · 2022-10-09T05:47:43Z

 | balancer_ip         | core       | The IP of picked upstream server.                                                   | 192.168.1.2    |
 | balancer_port       | core       | The port of picked upstream server.                                                 | 80             |
 | consumer_name       | core       | Username of Consumer.                                                               |                |
+| consumer_group_id   | core       | Group name of Consumer.                                                             |                |


spacewander · 2022-10-09T05:50:37Z

 | create_time | False    | Epoch timestamp (in seconds) of the created time. If missing, this field will be populated automatically.             | 1602883670 |
 | update_time | False    | Epoch timestamp (in seconds) of the updated time. If missing, this field will be populated automatically.             | 1602883670 |

+## Consumer group


Let's update the Resources that support paging queries: list in this page

spacewander · 2022-10-10T06:03:17Z

 | balancer_ip         | core       | 上游服务器的 IP 地址。                                                            | 192.168.1.2      |
 | balancer_port       | core       | 上游服务器的端口。                                                                | 80               |
 | consumer_name       | core       | 消费者的名称。                                                                    |                  |
+| consumer_group_id   | core       | 消费者所在的组的名称。                                                            |                  |


Let's sync the change to the Chinese version too

kingluo added 3 commits September 23, 2022 17:16

feat: add consumer group

445d160

fix indent

8db35af

bugfix

a66cea6

spacewander reviewed Sep 25, 2022

View reviewed changes

tzssangglass reviewed Sep 26, 2022

View reviewed changes

Comment thread apisix/plugin.lua

tzssangglass reviewed Sep 26, 2022

View reviewed changes

Comment thread t/admin/consumer-group.t

tzssangglass reviewed Sep 26, 2022

View reviewed changes

Comment thread t/admin/consumer-group.t

tzssangglass reviewed Sep 26, 2022

View reviewed changes

kingluo added 2 commits September 27, 2022 11:42

fix PR

6bae49b

fix indent

e998b80

spacewander reviewed Sep 27, 2022

View reviewed changes

fix PR

a4aeb32

spacewander previously approved these changes Sep 27, 2022

View reviewed changes

tzssangglass previously approved these changes Sep 27, 2022

View reviewed changes

add t/node/consumer-group.t and consumer-group.md

e36abeb

kingluo dismissed stale reviews from tzssangglass and spacewander via e36abeb September 29, 2022 07:12

add doc in config.json

f8f6b06

spacewander reviewed Oct 8, 2022

View reviewed changes

spacewander requested a review from SylviaBABY October 8, 2022 05:12

SylviaBABY reviewed Oct 8, 2022

View reviewed changes

fix PR

1ec167b

SylviaBABY requested review from spacewander and tzssangglass October 9, 2022 01:22

spacewander reviewed Oct 9, 2022

View reviewed changes

fix PR

1a7b264

spacewander reviewed Oct 10, 2022

View reviewed changes

fix PR

d0d4f88

spacewander approved these changes Oct 10, 2022

View reviewed changes

SylviaBABY approved these changes Oct 10, 2022

View reviewed changes

tzssangglass approved these changes Oct 10, 2022

View reviewed changes

spacewander merged commit 7e2cc4f into apache:master Oct 11, 2022

Liu-Junlin pushed a commit to Liu-Junlin/apisix that referenced this pull request Nov 4, 2022

feat: add consumer group (apache#7980)

c290c6e


		When APISIX can't find the Consumer Group with the `group_id`, the Admin API is terminated with a status code of 400.

		If a Consumer already has the `plugins` field configured, the plugins in the Consumer Group will effectively be merged to it. The same plugin in the Consumer Group will not override the ones configured directly in the Consumer.


		For example, if we configure a Consumer Group as shown below

		```

	\| consumer_group_id \| core \| Group name of Consumer. \| \|
	\| consumer_group_id \| core \| Group ID of Consumer. \| \|

Uh oh!

Conversation

kingluo commented Sep 23, 2022

Description

Checklist

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

spacewander Oct 8, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

spacewander Oct 8, 2022 •

edited

Loading