IT博客汇 | 参与Apisix开源的一次完整提交过程分享

参与Apisix开源的一次完整提交过程分享

nieyong发表于 2021-03-10 03:20:00

前言

参与开源不是为了证明什么，而是为了更好的配合工作。开源和工作在绝大部分时间，都是可以和谐共处，互相促进，Win-WIn双赢。

本文内容记录了为 apisix 项目提交的一次pull request提交（访问地址：https://github.com/apache/apisix/pull/3615 ）完整过程，提交内容为一个独立的服务发现模块，本文目的是为团队的其他同学参与社区项目分享的行为提供一个简单可遵循、可操作模型。

概括来讲，简要操作流程如下：

首先，确定需要开源的部分
其次，在项目社区中分享我们的看法和后续行为等
然后，准备提交内容
接着，提交pull request，接受社区审核，反复调整修改
后续，关注社区的走向，持续改进

下面为每一步具体操作的流水账。

提前预警，图多费流量，慎入 :))

首先，我们有一个Consul KV服务发现组件

作为Nginx用户，我们实际场景使用 Nginx Upsync 模块，结合Consul KV作为服务注册和发现形式。

我们基于Apisix构建HTTP API服务网关，没有发现现成的Consul KV形式服务发现模块，既然实际业务需要，我们需要把它按照接口规范开发出来，以适应我们自己的实际场景。

当服务发现模块功能开发出来后，也是仅仅能满足基本需求，还不够完善，但这时改进的思路并不是非常清楚，
既然开源社区也有类似的需求，那我们可以考虑分享开源出去，接收整个社区的考验，大家一起改进。

限于日常思维角度的局限，若是仅仅满足工作需要，那么开源出去会让你的代码接受到社区方方面面的审核，尤其是针对代码风格、功能、执行等有严格要求的apisix项目。摆正心态，接受代码评审并调整，最终结果无疑是让代码更加健壮，好事一桩嘛。

当然开源出去之后，该模块的变更以及优化等行为就完全归属整个社区了，群策群力，是一种比较期待的演进方式。

第一步，咨询社区意见

一个优秀的开源项目，为了稳定健康发展，一般会提供邮件组方便社区参与者咨询、沟通协调等。

一般来说，Github会提供issues列表方便项目使用者提交BUG，若我们想在社区中表达意图、观点等，就不如发在社区邮件组中，这样能够得到更多的关注。比如，我们想给社区共享一个完整的服务发现模块，就可以直接在邮件组中描述大致功能，以及大致处理流程等，让社区知道我们的真实意图。

Apisix开发邮件组地址为：dev@apisix.apache.org，但一般的邮件组都需要注意如下事项：

沟通需要使用英文
- 这也是Apisix项目国际化需求
- 虽然你也知道阅读邮件的有几个中国的糙老爷们，但也会有来自其他国家的用户
- 当然在Github上所有的项目沟通都需要使用英文，这是一个良好的开源社区沟通习惯
- 推荐一个微软英语在线协作辅助工具：https://aimwriting.mtutor.engkoo.com/ ，可以帮助校验语法错误等
无法传递富文本
- 使用纯文本即可
- 类似我有格式化强迫症患者，直接粘贴 markdown 格式文本
无法传递图片
- 直接传递图片URL地址
- 若需要传递图片，提供一个小技巧：新建一个issues表单，直接拖拽图片到表单处，然后获得图片地址即可，无须提交issues表单
上传图片

下面是我发送的邮件截图：

因为apache邮件组不支持富文本和图片，实际看到的效果就没有那么好看了，下面的连接包含了该讨论完整的回复内容：

https://lists.apache.org/thread.html/rf9e392dd76e4701935940d22b4b9d9f8ed130cca34a2e951357a4c2a%40%3Cdev.apisix.apache.org%3E

不方便打开的话，下面提供完整邮件讨论截图，很长的截图，呵呵：

总之，断断续续经过三周时间的讨论，这个过程需要有些耐心。发完邮件等有了积极反馈，下面就可以着手准备提交代码了。

第二步，准备提交

Fork到自己仓库

去 https://github.com/apache/apisix Fork到自己仓库中，然后克隆到自己工作机来。

注意，需要时刻保持和主干保持一致：

git remote add upstream https://github.com/apache/apisix.git

下面就是动手开干了。

按需调整代码

Consul KV服务发现模块文件是 consul_kv.lua，相对位置为：apisix/discovery/consul_kv.lua。我们想提交到项目主干，那么代码就必须遵循已有规范。

针对apisix的服务发现代码，需要有配置项，就必须给出一套完整的服务配置 schema 定义，如下。

local schema = {
    type = "object",
    properties = {
        servers = {
            type = "array",
            minItems = 1,
            items = {
                type = "string",
            }
        },
        fetch_interval = {type = "integer", minimum = 1, default = 3},
        keepalive = {
            type = "boolean",
            default = true
        },
        prefix = {type = "string", default = "upstreams"},
        weight = {type = "integer", minimum = 1, default = 1},
        timeout = {
            type = "object",
            properties = {
                connect = {type = "integer", minimum = 1, default = 2000},
                read = {type = "integer", minimum = 1, default = 2000},
                wait = {type = "integer", minimum = 1, default = 60}
            },
            default = {
                connect = 2000,
                read = 2000,
                wait = 60,
            }
        },
        skip_keys = {
            type = "array",
            minItems = 1,
            items = {
                type = "string",
            }
        },
        default_service = {
            type = "object",
            properties = {
                host = {type = "string"},
                port = {type = "integer"},
                metadata = {
                    type = "object",
                    properties = {
                        fail_timeout = {type = "integer", default = 1},
                        weigth = {type = "integer", default = 1},
                        max_fails = {type = "integer", default = 1}
                    },
                    default = {
                        fail_timeout = 1,
                        weigth = 1,
                        max_fails = 1
                    }
                }
            }
        }
    },

    required = {"servers"}
}

当然，你需要区分每一个配置项是不是必填项，非必传项需要具有默认值，以及上限或下限约束等。

下面需要在该模块启动时进行检测用户配置是否错误，无法兼容、恢复错误的话，需要直接使用Lua内置错误日志接口输出：

error("Errr MSG")

另外，若要引入 resty.worker.events 组件，不要提前require，比如在文件头部提前声明时：

loca  events = require("resty.worker.events")

启动后，就有可能在日志文件中出现如下异常：

2021/02/23 02:32:20 [error] 7#7: init_worker_by_lua error: /usr/local/share/lua/5.1/resty/worker/events.lua:175: attempt to index local 'handler_list' (a nil value)
stack traceback:
    /usr/local/share/lua/5.1/resty/worker/events.lua:175: in function 'do_handlerlist'
    /usr/local/share/lua/5.1/resty/worker/events.lua:215: in function 'do_event_json'
    /usr/local/share/lua/5.1/resty/worker/events.lua:361: in function 'post'
    /usr/local/share/lua/5.1/resty/worker/events.lua:614: in function 'configure'
    /usr/local/apisix/apisix/init.lua:94: in function 'http_init_worker'
    init_worker_by_lua:5: in main chunk

推荐做法是延迟加载，在该模块被加载时进行引用。

local events
local events_list

......

function _M.init_worker()
        ......
        events = require("resty.worker.events")
        events_list = events.event_list(
                "discovery_consul_update_application",
                "updating"
        )
        if 0 ~= ngx.worker.id() then
                events.register(discovery_consul_callback, events_list._source, events_list.updating)
                return
        end
        ......
end

单元测试依赖

单元测试代码的执行，会在你提交PR代码后自动执行持续集成行为内执行。

首先，需要本机执行单元测试前，需要提前准备好所需Docker测试实例：

docker run --rm --name consul_1 -d -p 8500:8500 consul:1.7 consul agent -server -bootstrap-expect=1 -client 0.0.0.0 -log-level info -data-dir=/consul/data
docker run --rm --name consul_2 -d -p 8600:8500 consul:1.7 consul agent -server -bootstrap-expect=1 -client 0.0.0.0 -log-level info -data-dir=/consul/data

docker run --rm -d \
       -e ETCD_ENABLE_V2=true \
       -e ALLOW_NONE_AUTHENTICATION=yes \
       -e ETCD_ADVERTISE_CLIENT_URLS=http://0.0.0.0:2379 \
       -e ETCD_LISTEN_CLIENT_URLS=http://0.0.0.0:2379 \
       -p 2379:2379 \
    registry.api.weibo.com/wesync/wbgw/etcd:3.4.9

然后，安装项目依赖：

make deps

其次，别忘记在apisix项目持续集成脚本相应位置添加相应依赖。

比如，因为单元测试依赖于端口分别为7500和7600的两个Consul Server实例，需要在执行单元测试之前提前运行，因此你需要在对应的持续集成文件上添加所需运行实例。比如其中一个位置：

无测试不编码

仅仅提供服务发现consul_kv.lua这一个文件，是无法被仓库管理员采纳的，因为除了你自己以外，别人无法确定你提交的代码所提供功能是否足够让人信服，除非你能提供较为完整的 Test::Nginx 单元测试支持，自我证明。

Test::Nginx 单元测试可能针对很多人来讲，是一个拦路虎，但其实有些耐心，你会发现它的美妙之处。

简单入门可参考 https://time.geekbang.org/column/article/109506 （若只需要学习单元测试，其实不需要购买整个专辑的）。在使用过程中需要参考在线文档：https://metacpan.org/pod/Test::Nginx::Socket ，需要一些耐心花费一点时间慢慢消化。

如何运行Nginx单元测试案例，具体参看：
https://github.com/apache/apisix/blob/master/doc/zh-cn/how-to-build.md

至于Apisix定制部分单元测试部分，可以直接参考已有的单元测试文件即可。

Consul KV服务发现的单元测试模块相对路径 t/discovery/consul_kv.lua，在线地址为： https://github.com/apache/apisix/blob/master/t/discovery/consul_kv.t 。该文件大约500多行，比真正的模块consul_kv.lua代码行数还多。但比较完整覆盖了所能想到的所有场景，虽然写起来虽然有些麻烦，但针对应用到线上大量业务的核心代码，无论多认真和谨慎都是不为过的。

以往针对关键核心模块的每一次迭代，心里面大概有些忐忑七上八下吧，也不太敢直接应用到线上。现在有了单元测试各种场景的覆盖辅助验证迭代变更效果，自信心是有了，也可以给别人拍着胸脯保证修改没问题。当然若后续发现隐藏的问题，直接添加上对应的单元测试覆盖上即可。

我们这次只提供一个服务发现模块，因此只需要单独测试consul_kv.t文件即可：

# prove -Itest-nginx/lib -I./ t/discovery/consul_kv.t
......
t/discovery/consul_kv.t .. ok
All tests successful.
Files=1, Tests=102, 36 wallclock secs ( 0.05 usr  0.01 sys +  0.78 cusr  0.41 csys =  1.25 CPU)
Result: PASS

出现测试案例失败问题，可以去 apisix/t/servroot/logs 路径下查看 error.log 文件暴露出的异常等问题。

有些一些测试用例需要组合一组较为复杂的使用场景，比如我们准备一组后端节点：

127.0.0.1:30511，输出 server 1
127.0.0.1:30512，输出 server 2
127.0.0.1:30513，输出 server 3
127.0.0.1:30514，输出 server 4

这些节点将被频繁执行注册Consul节点然后再解除注册若干循环过程：清理注册 -> 注册 -> 解除注册 -> 注册 -> 解除注册 -> 注册 -> 解除注册 -> 注册 ，目的检验已解除注册的失效节点是否还会存在内存中等。

有些操作，比如注册或解除注册节点这些操作，网关的consul_kv.lua服务模块在物理层面需要wait一点时间等待网关消化这些变化，因此我们需要额外提供一个 /sleep 接口，请求时需要故意休眠几秒钟时间等待下一次请求生效。

=== TEST 7: test register & unregister nodes
--- yaml_config eval: $::yaml_config
--- apisix_yaml
routes:
  -
    uri: /*
    upstream:
      service_name: http://127.0.0.1:8500/v1/kv/upstreams/webpages/
      discovery_type: consul_kv
      type: roundrobin
#END
--- config
location /v1/kv {
    proxy_pass http://127.0.0.1:8500;
}
location /sleep {
    content_by_lua_block {
        local args = ngx.req.get_uri_args()
        local sec = args.sec or "2"
        ngx.sleep(tonumber(sec))
        ngx.say("ok")
    }
}
--- timeout: 6
--- request eval
[
    "DELETE /v1/kv/upstreams/webpages/?recurse=true",
    "PUT /v1/kv/upstreams/webpages/127.0.0.1:30511\n" . "{\"weight\": 1, \"max_fails\": 2, \"fail_timeout\": 1}",
    "GET /sleep?sec=5",
    "GET /hello",

    "PUT /v1/kv/upstreams/webpages/127.0.0.1:30512\n" . "{\"weight\": 1, \"max_fails\": 2, \"fail_timeout\": 1}",
    "GET /sleep",
    "GET /hello",
    "GET /hello",

    "DELETE /v1/kv/upstreams/webpages/127.0.0.1:30511",
    "DELETE /v1/kv/upstreams/webpages/127.0.0.1:30512",
    "PUT /v1/kv/upstreams/webpages/127.0.0.1:30513\n" . "{\"weight\": 1, \"max_fails\": 2, \"fail_timeout\": 1}",
    "PUT /v1/kv/upstreams/webpages/127.0.0.1:30514\n" . "{\"weight\": 1, \"max_fails\": 2, \"fail_timeout\": 1}",
    "GET /sleep",

    "GET /hello?random1",
    "GET /hello?random2",
    "GET /hello?random3",
    "GET /hello?random4",

    "DELETE /v1/kv/upstreams/webpages/127.0.0.1:30513",
    "DELETE /v1/kv/upstreams/webpages/127.0.0.1:30514",
    "PUT /v1/kv/upstreams/webpages/127.0.0.1:30511\n" . "{\"weight\": 1, \"max_fails\": 2, \"fail_timeout\": 1}",
    "PUT /v1/kv/upstreams/webpages/127.0.0.1:30512\n" . "{\"weight\": 1, \"max_fails\": 2, \"fail_timeout\": 1}",
    "GET /sleep?sec=5",

    "GET /hello?random1",
    "GET /hello?random2",
    "GET /hello?random3",
    "GET /hello?random4",
]
--- response_body_like eval
[
    qr/true/,
    qr/true/,
    qr/ok\n/,
    qr/server 1\n/,

    qr/true/,
    qr/ok\n/,
    qr/server [1-2]\n/,
    qr/server [1-2]\n/,

    qr/true/,
    qr/true/,
    qr/true/,
    qr/true/,
    qr/ok\n/,

    qr/server [3-4]\n/,
    qr/server [3-4]\n/,
    qr/server [3-4]\n/,
    qr/server [3-4]\n/,

    qr/true/,
    qr/true/,
    qr/true/,
    qr/true/,
    qr/ok\n/,

    qr/server [1-2]\n/,
    qr/server [1-2]\n/,
    qr/server [1-2]\n/,
    qr/server [1-2]\n/
]

准备文档

除了代码能够正常运转，我们还需要准备相应的Markdown文档辅助说明如何使用我们的模块，帮助社区用户更好使用它。

社区一般以英文文档为先，只有在精力满足的情况下，可以补充中文文档。

下面就是要准备Markdown文档了，其文档路径为：doc/discovery/consul_kv.md，单独的文档需要在其它已有文档挂接上对应链接，方便索引。

文档路径为：doc/discovery/consul_kv.md，在线地址：https://github.com/apache/apisix/blob/master/docs/en/latest/discovery/consul_kv.md

一般建议需要在文档中能够清楚说明模块的使用方式，以及注意事项，尤其是配置参数使用方式等。比如下面的配置项说明：

```yaml
discovery:
  consul_kv:
    servers:
      - "http://127.0.0.1:8500"
      - "http://127.0.0.1:8600"
    prefix: "upstreams"
    skip_keys:                    # if you need to skip special keys
      - "upstreams/unused_api/"
    timeout:
      connect: 1000               # default 2000 ms
      read: 1000                  # default 2000 ms
      wait: 60                    # default 60 sec
    weight: 1                     # default 1
    fetch_interval: 5             # default 3 sec, only take effect for keepalive: false way
    keepalive: true               # default true, use the long pull way to query consul servers
    default_server:               # you can define default server when missing hit
      host: "127.0.0.1"
      port: 20999
      metadata:
        fail_timeout: 1           # default 1 ms
        weight: 1                 # default 1
        max_fails: 1              # default 1
 ```
......

The `keepalive` has two optional values:

- `true`, default and recommend value, use the long pull way to query consul servers
- `false`, not recommend, it would use the short pull way to query consul servers, then you can set the `fetch_interval` for fetch interval

每一个文档都不应该成为信息孤岛，它需要在其它文档上挂载上一个连接地址，因此我们需要在合适的地方，比如需要在 doc/discovery.md最下面添加链接地址描述：

## Discovery modules

- eureka
- [Consul KV](discovery/consul_kv.md)

模块代码，测试文件，以及文档等准备好了之后，下面就是准备提交代码到自己仓库。

验证提交语法规范

所有内容准备好之后，建议执行 make lint 和 make license-check 两个命令检测代码、markdown文档等是否满足项目规范要求。

# make lint
./utils/check-lua-code-style.sh
+ luacheck -q apisix t/lib
Total: 0 warnings / 0 errors in 133 files
+ find apisix -name '*.lua' '!' -wholename apisix/cli/ngx_tpl.lua -exec ./utils/lj-releng '{}' +
+ grep -E 'ERROR.*.lua:' /tmp/check.log
+ true
+ '[' -s /tmp/error.log ']'
./utils/check-test-code-style.sh
+ find t -name '*.t' -exec grep -E '\-\-\-\s+(SKIP|ONLY|LAST)$' '{}' +
+ true
+ '[' -s /tmp/error.log ']'
+ find t -name '*.t' -exec ./utils/reindex '{}' +
+ grep done. /tmp/check.log
+ true
+ '[' -s /tmp/error.log ']'
# make license-check
.travis/openwhisk-utilities/scancode/scanCode.py --config .travis/ASF-Release.cfg ./
Reading configuration file [.travis/ASF-Release.cfg]...
Scanning files starting at [./]...
All checks passed.

若检查出语法方面问题，认真调整，直到找不到问题所在。

这次PR提交之前，忘记这回事了，会导致多了若干次次submit提交。

第三步，提交Pull Request

去官网：https://github.com/apache/apisix/pulls 新建一个New pull request，后面将使用PR指代pull request。

PR标题格式

PR提交标题是规范要求的，模板如下：

{type}: {desc}

其中{type}指代本次PR类型，具体值如下，尽量不要搞错：

feat：新功能（feature）
fix：修补bug
docs：文档（documentation）
style：格式（不影响代码运行的变动）
refactor：重构（即不是新增功能，也不是修改bug的代码变动）
test：增加测试
chore：构建过程或辅助工具的变动
……

其中{desc}需要概括本次提交内容。

比如这次标题为：feat: add consul kv discovery module。

填充PR内容

PR内容模板化，为标准的Github Markdown格式，主要目的说明本次提交内容，示范如下：

### What this PR does / why we need it:
<!--- Why is this change required? What problem does it solve? -->
<!--- If it fixes an open issue, please link to the issue here. -->

### 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](https://github.com/apache/apisix/tree/master#community) first**

按照模板格式填写，省心省力，如下：

### What this PR does / why we need it:

As I  mentioned previously in the mail-list, my team submit our `consul_kv` discovery module now.

More introductions here: 
 https://github.com/yongboy/apisix/blob/consul_kv/doc/discovery/consul_kv.md

### 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? **If it is not backward compatible, please discuss on the [mailing list](https://github.com/apache/apisix/tree/master#community) first**

认真接受评审和建议

提交PR之后，才是一个开始，起点。

Apisix项目会自动针对我们所提交内容执行持续集成，apisix项目的检查项很多，比如针对Markdown格式就很严格：

持续集成不通过，按照要求微调吧，也是标准化的要求。

我们在PUSH代码之前，使用 make lint 和 make license-check 两个命令提前检测还是十分有必要的，提前检测语法等。

首先，一定要确保持续集成不能出错。持续集成通不过，说明我们的准备还不充分，继续调整修改，继续提交，一直到持续集成完全执行成功为止。

保证持续集成执行成功，这是最基本的要求，否则社区无法确认我们的代码是否基本合格。

放松心态，准备开始改进BUG，以及接受社区的各种代码评审和改进意见吧。

其次，就是要虚心接受社区代码评审和改进意见了，这是最关键的一步。

下面是一些建议：

真正代码BUG，认真修改
逻辑处理不合理的地方，思考并给出一些处理思路，确定好之后开始调整即可
有些提议可能会超出本次提交范围，说明原因，给出拒绝理由，可以婉拒嘛，比如可以放在下一次的提交中。
若有遇到自己处理不了的问题，积极向社区寻求帮助吧。
针对一批次修改再次提交后，会再次执行持续集成，一样确保持续集成不能够失败，然后继续等待下一轮的审核

认真对待每一个建议，有则改之无则加勉，不知不觉之间就进步了很多，代码质量也得到了提升。

经过多次的微调，我们的服务发现核心模块基本上已趋于完善了一版，这已经和还没准备分享出来之前的原始文件相比已经天差地别了 :))

下面是本次PR包含的多次提交、代码评审以及答复等完整流程截图：

被合并到主分支之后，有没有感觉到整个社区都在帮助我们一起改进，快不快哉？

关于依赖项的处理

本次提交的服务发现模块依赖一个组件：lua-resty-consul，其仓库地址：https://github.com/hamishforbes/lua-resty-consul，最新版本为：`0.3.2`。因为我们在实际部署定制时，直接下载了该文件，简单直接粗暴。

但apisix项目针对项目依赖，采用的 LuaRocks 管理，在 2021-2-20 之前该组件托管在 https://luarocks.org/modules/hamish/lua-resty-consul 上面最新版本为 0.2-0，这就很难办了。

我的处理步骤如下：

首先我在github上面向作者提交一个求助：https://github.com/hamishforbes/lua-resty-consul/issues/20，然而并没有在一两周时间内没有等到作者回复
无奈，只好自己在 LuaRocks 单独提交一个暂时性的解决方案：https://luarocks.org/modules/yongboy/lua-resty-consul-0.3.2，在本次PR中直接包含了该组件临时地址
三周左右，终于等到该组件作者提交最新版到 LuaRocks 站点，既然官方更新了，那就把服务发现模块里面的依赖修改为官方最新地址吧，再次提价一个PR：https://github.com/apache/apisix/pull/3654

有些一波三折 :))

第四步，关于后续

一旦合并到主分支后，后续的演进整个社区都可以参与进来，可能有人提 issue，可能有人提 PR 修改等，后续我们想为该模块继续提交，那将是另外一个PR的事情。

我们可以继续做以下事情：

根据实际需要重构
若有人提Issue是，自然是Fixbug；实践中遇到的Bug，修复它
需要添加新的单元测试覆盖到新的特性
若有需要，就需要添加新的文档进行描述

毫无疑问，这是一个良性循环。

小结

参与社区开发的其它类型提交，可能会比上面所述简单很多，但大都可以看做是以上行为的一个子集。

参与开源，也会为我们打开一扇窗户，去除自身的狭隘。积极向社区靠拢，这需要磨去一些思维或认知的棱角，虚心认识到自我的不足，并不断调整不断进步。

加油！

nieyong 2021-03-10 11:20 发表评论