试着为 Ansible 贡献模块的故事

这是Ansible Advent Calendar第12天的文章。https://qiita.com/advent-calendar/2018/ansible

首先

这是一个有些正经的文章。Ansible通常是作为操作系统层以上的配置管理工具而闻名的。最近它也被提及作为网络设备配置管理工具,但实际上它所涵盖的范围非常广泛。

在这其中,我特别注重的是Cloud模块。
它可以用于使用AWS、GCP、Azure等云供应商时。
https://github.com/ansible/ansible/tree/devel/lib/ansible/modules/cloud
在AWS的配置管理中,经常提到的有CloudFormation和TerraForm等,
但是由于可以将幂等性应用到云实例中,并且不需要管理状态等原因,
所以我最喜欢Ansible的Cloud模块。
通过使用它,可以像一般被广泛知晓的Ansible管理方法一样,
来管理云供应商的资源。

目前状态是由于模块尚未完全建设,仍处于发展初期的阶段。但是我希望能有更多人使用它,即使只是一点点增加也好。因此,我打算写下我本次对原作者做出贡献的方法。具体创建模块的源代码级内容将在另一篇文章中进行讨论,但我希望它能对时间序列和顺序等方面提供一些参考。

一旦熟悉了,贡献开源软件并不可怕!

排列顺序

假设条件

这次有一个明显的错误需要修正。有一个名为ec2_instance模块的预览版模块,用于管理AWS上的EC2实例。目前使用的是名为ec2的模块。在之前的文章中也有提到一点。请参考这篇文章:https://qiita.com/morin_river/items/29f853c834f4c7cc756e

在这个模块中,尽管系统原本支持check_mode,但实际运行时无论check_mode如何,都会执行导致了这个问题。

我认为预览模块非常积极,因为它添加了新代码。
但出于工作上的原因,如果保持这个问题而不修复,会导致使用上的不便,所以我决定尝试进行修正。

提出问题

首先,我们搞清楚了问题的本质。

特别是在开源软件中,我们认为让观察者尽可能地理解问题是很重要的。对于Ansible,由于ISSUE_TEMPLATE很好,只要按照一定的模板填写,就可以写出内容。

在写作内容时需要用英语来表达,但基本上我认为只在谷歌翻译的基础上稍作改动是没有问题的。
(对于技术文章的翻译,这样做是远远不够好的……只是宣传而已)。
我尽量选择简单的句子进行翻译,然后自己阅读英语版本,力求不感到不自然。

我认为可以写清楚哪些是不行的,以及本来应该怎么做。

然后,当你提出ISSUE时,机器人会很好地给它贴上标签。
此外,在这种情况下,我进入了Ansible社区的IRC,
我在频道里对大家说,“我遇到了这个问题,你们有什么想法?如果没有人处理,我可以修复。”
我曾经开发过AWS NLB模块,但事实上,我曾经有一个经验,就是在我开始工作之前,已经有其他人在处理它了
所以我确认没有人在处理这个问题。
(虽然这项工作实际上是不必要的,如果有更好的办法那就更好了)。
然后,因为当时Ansible的IRC通知功能没有正常工作,
所以我们讨论了如果能够修复问题后通过IRC通知的事情。
我认为这也是直接沟通的优点之一,可以得到这样的建议。

写代码

这段代码本身非常简单。
代码只是添加了一个简单的逻辑。
如果参数带有check_mode,则只是跳过处理。
由于代码很长,所以跟踪有点困难,但这可能是因为我们适应了。
(对于专业程序员来说,可能可以处理更长的代码。)

写测试代码

在这次中,我最想强调的是这个内容。
评论者可能没有完全意识到修正内容的问题所在。
尤其是如果评论者不是代码作者的话,问题可能更加突出。
在这种情况下,我认为最容易进行沟通的方法就是编写测试代码以清楚地展示变更点。
在开源软件中或者日常沟通中,将信息传达给他人易于理解是非常重要的。

此外,就 Ansible 而言,还存在一些测试代码跟不上的地方。
在这种情况下,将测试代码提交为 PR 可能更容易合并。
特别是对于能够复现错误的 bug,编写并修复相应的测试可能是最快捷的方法。

在 ec2_instance 模块中,虽然原本已经存在测试代码,但缺少了检查模式的测试。
因此,我们在现有的测试代码中添加了新的测试,并为检查模式添加了新的测试。

我给现有的测试添加了一部分内容。

https://github.com/ansible/ansible/pull/46774/files#diff-6216fdd46b49fcf4fbd75b0738584021 的差异
https://github.com/ansible/ansible/pull/46774/files#diff-25bb5354720327ddc9b74a09d8f8d87c 的差异
https://github.com/ansible/ansible/pull/46774/files#diff-d3c4f3a9ae9f9ea2a57ccd90ac879a07 的差异
https://github.com/ansible/ansible/pull/46774/files#diff-dbe25295683fa8c1f9c4cab7ac4b8421 的差异
https://github.com/ansible/ansible/pull/46774/files#diff-62fa2d7358adc4e68481c9a465243402 的差异
https://github.com/ansible/ansible/pull/46774/files#diff-714d27f274a842068e1259711a66ef2d 的差异
https://github.com/ansible/ansible/pull/46774/files#diff-fe16e0c6df48c9423e6c2a8cbd76658f 的差异

新的测试

将这些内容记录下来就容易理解了,但测试代码比编写的代码要多得多。当然,也有Python代码和YAML表达能力的差异。然而,我花在测试代码上的时间更多。但是,为了向他人解释,这可能是最直接的方法,所以我想写得很仔细。在编写这个测试代码的过程中,我了解了以前不知道的boto3库中waiter的概念,而且通过编写测试代码,我也能学到相关技术。

进行测试

在Ansible的测试代码中,包括集成测试和健全性测试。
可以使用以下命令分别进行测试。

$ ansible-test sanity --test pep8 ec2_instance
$ ansible-test integration ec2_instance

然而,这次无法进行集成测试。
后来,我们发现当时的AWS测试流水线不够完善,
导致在进行集成测试时没有运行AWS资源,
但当时我们并不知情。
因此,我们贴出了执行结果,明确表示进行了集成测试但未成功运行。

创建公关

https://github.com/ansible/ansible/pull/46774
现在只需将之前的内容整理好,并创建PR即可。

    • 困っていた内容

 

    • 書いたコードの内容

 

    • 実行して、上手く言った結果

 

    実行したけどうまく行かなかった結果

尽量保留尽可能多的内容,但不要使文章过于冗长。
……由于修正代码的是一位Red Hat的高级工程师,我有些不安,但是我已经很好地写下了我想要修正的逻辑及其证明。
因此,我以坚定的态度提交了PR。

最終結果

经过约一天的时间,我的代码提交被批准并合并。
我预计这个改动将在接下来的Ansible2.8版本中发布。
真是太棒了,看到自己的源代码被使用在平时所使用的工具中,感觉真的很开心。

スクリーンショット 2018-12-11 20.48.39.png

当我收到评论时,感受到了得到积极反馈时的高兴,所以我意识到只要坚持做必要的工作,我就能做出贡献给OSS。如果OSS能更加活跃起来(甚至希望Ansible的云模块变得更加活跃),那会非常开心。如果有人考虑贡献给Ansible或者其他方面,我希望能对他们有所帮助。

内部交流

迄目前為止,我只寫了非常完成且美麗的內容,但在這背後卻有很多失敗。

https://github.com/ansible/ansible/pull/37820

最初我提交的PR非常糟糕。
未進行任何Sanity測試,也缺乏說明。
最終雖然經過了很多程式碼審查,但由於結果是相同功能已在另外一個模組中實現,
所以PR並未被合併。
(我認為這是正確的,因此我已經將此PR關閉)

此外,下一个公关(PR)的准确性也不高。
由于没有添加测试代码,所以即使添加了代码,第三方对于“这是否正确?”很难理解。
现在才明白这一点。
如果只进行源代码审查,即使审查人员能力出众,判断也可能需要花费时间,
这可能导致合并代码的过程变得耗时。
实际上,在这个PR中,合并代码花费了大约三周的时间。

自己编写的代码应该是自己最能理解的,所以在开放源代码等情况下,我认为重要的是如何让他人能理解自己想要做什么。测试代码只是其中一个例子,但结果上来说我觉得这样做更好。

起初,可能会认为与不懂英语的海外工程师沟通是可怕的。然而,我认为无论内容如何,它都应该是日常技术的延伸,只要能充分发挥,大多数问题都可以解决。此外,我认为重要的是不要放弃,即使在评论中遭受挫折。我希望能够养成检查错误并修正的习惯,以便能够持续进步。

编辑历史

2018年12月16日
因为日语存在错误,进行了大幅修正。

bannerAds