nazoです。現在は厳密には攻殻機動隊（システムユニット）ではないのですが、開発に関する大きな動きがありましたのでまとめてみたいと思います。
何の話？
LMND というシステムがあるのですが、これの開発を急ピッチで進めたいという事で急遽移動することになり、快適な開発環境になるまでに行ったことについての話になります。
LMNDのサービスの詳細についてはここでは割愛させて頂きますが、システム的にはRails4系にMySQLという、少し古いものの一般的な構成で作られたものになっております。2018年9月に吸収合併を行ったのですが、その際に開発体制が完全内製に切り替わってしまったため、引き継ぎがほとんどない状態から体制が切り替わってしまったという状態になります。社内の開発者が先に割り当てられていたのですが、困っているので助けてくれということで私が途中から入ることになりました。
作り直すかどうか
事前情報では「やばかったら作り直してもいいよ」とは言われたのですが、作り直しというのは一見楽しそうですが難しい問題を多く抱えています。
- サービスをやり直すわけではないのでデータは既存のまま使えるようにしないといけない
- 機能が多く、かつそれらが契約上廃棄できるものではない場合、それらを全て同様に移植しなければならない
一方で、それでも作り直さないといけないという場合も存在します。
- コードの汚さがあまりにどうしようもなく、何をするにもほぼスクラッチで開発するしかないような状態
- フレームワークなどが一般的ではないまたは独自のもので、今後のアップデートに対応することがほぼ不可能な状態
- 予算が十分にあり、かつ作り直しのビジョンが明確にある場合
ただし予算があるケースの場合以外は、最終的には全て作り直すとしても一部分から作り直すとか新しいものと古いものを共存できるようにするとか、様々な手法があると思います。規模の大きなアプリケーションの場合、そのアプリケーションが解決しようとしているもの（事業）が何かを適切に見極め、交通整理を行うことでどこをどう作り直すべきなのかが見えてくると思います。
今回のプロジェクトはRails製で、コードが読みにくい箇所が多いもののRailsそのものを魔改造しているというわけでもなく、作りながら書き直すことが十分に可能なレベルでしたので、基本的には作り直さないという方針を採ることにしました。
CIを整備
まず最初に目についたのは、デプロイフローがほぼ手動（一応Capistranoは入っているが）で、デプロイに特殊な作業が要求される、そしてCIが動いていないということでした。
CIが動いていない場合、そのコードの品質が不明ですので早急に改善したいところなのですが、途中からCIを入れるというのは基本的に難しいという事情があります。
- rubocopのようなlinterツールを途中から入れると絶対にエラーが出まくるが、直すには影響箇所の把握が必要なため困難
rubocopについては絶対に導入すべきなのですが、rubocopに準拠していないプロジェクトを途中から全てのチェック（私は onkcop をベースにさせていただくことが多いです）をクリアさせることは無理なので、以下のようにします。
- CI上でのfail levelを1段階上げてほとんどのものは無視するようにし、どうしても直すべきというものは.rubocop.ymlでレベルを上げる
- auto correctできるものはなるべくする（ただし、たまに壊れるので緩やかに進める）
- auto correctで消せるものはfail levelを上げてCIで止まる対象にする
これで緩やかにrubocopを導入することができます。
他の種類のlinterや、JavaScriptを途中からTypeScriptにするような場合には、このような手法で緩やかに入れていくという戦略が有効です。ただし緩やかに導入するというのは難易度が高く、AbcSizeのようなものに至ってはコード全体の書き直しを避けて通ることができませんので、プロジェクトの一番最初になるべく極限まで厳しくした設定を導入するのが絶対に良いです。プロジェクトの立ち上げ時にはまずCIを整備するようにしましょう。
その他、テスト（次項で詳細）を通すようにしたり、将来的なインフラ以降を想定してコンテナでのビルドを行うようにしました。現在はそこそこの品質をクリアしていないとCIが通らないようになっております。
壊れたテストを直す（あるいは、テストが書けるコードの書き方）
CIを回す上でテストは欠かせません。しかしながら、既存のテストはCIを通していないせいか半分近くが通らないものとなっておりました。
通らないテストを残しておいても何の意味もなく、直すべきか判断するのも無駄なので、ここは悩まずに動くものも含めて一度全部削除することにしました。
元はRSpecで書かれていたようですが、minitest + power-assertを主軸に新たに書き直すことにしました。とはいえ既存のロジックにテストを書いていくのも大変なのと、そもそもテストが書けるようなコードになっていないので、基本的には新規のコードに対してテストを書くという方針で進めました。
ここでの「テスト」というのは、その対象（メソッド）が役割を果たせるかどうかを、なるべく依存度の少ない状態で書くテストのことを指します。Google Testing BlogのTest Sizesで言うところのSmallなテストを中心に書いていきます。Medium～Largeの範囲は「念のため通す程度」に留めておきます。コントローラーを直接叩くようなテストはMediumに当たるものと思われます。
Smallなテストが書けるコードというのは、以下の条件を満たしている必要があります。
- 依存関係が明確になっていて、差し替え可能になっている
- 外部リソースへのアクセス
- 内部で行っている別の処理
ControllerやViewにロジックが紛れ込んでいると、それをテストするためにそのテストとは無関係な「入力値の処理」「バリデーション」「テンプレートの出力」といったものが動作してしまい、何を検証していたのかわからなくなります。Railsでクリーンアーキテクチャ（というかドメインを意識したレイヤードアーキテクチャ）を大雑把に行うには、以下のような規則を入れると簡単です。
- バリデーションはモデルで行わずformクラスで行う
- データの保存とバリデーションはレイヤーが違うため、連動して行うのは層がずれているため
- callbackを使わない
- 上記同様
- 純粋なRubyクラスのロジック層（いわゆるサービスとかユースケースとか呼ばれるやつ）を用意し、そこに重要な処理を書く
- SRP原則を忘れずに
- 別の層にアクセスする（特に外部APIの通信）はコンストラクタでインジェクションし、テスト時に差し替えれるようにする
- DBアクセスはRepository層を作るのが理想ではあるものの、そこはがんばらなくても良い（ただしSmallの定義からはやや外れる）
- 「抽出条件の処理はscope、プロパティアクセスの部分はModelのメソッド」として書くのは徹底し、これら単体でのテストは必ず書くようにする
- コントローラーやサービスにwhereがダイレクトに書いてあったり条件を複数繋いでいたりするものは滅ぼす
- ActiveRecord::Relationからさらに絞り込みを行うことはしないようにする（層をまたぐことになるので）
このような条件を踏まえて、ロジックをテストが書けるような構造に書き換えつつテストを書いていくというようなことを続けております。
データの重要性
DB設計が一部残念なところがあり、一部データの再設計が必要な箇所がありました。DBは手を入れたくないところなのですが、今対応しておかないと後で確実に問題になる箇所があったので、構造の変更に踏み切りました。
DBスキーマを変更する場合、テーブルやカラムを増やすだけであればあまり問題になりませんが、減らしたり型を変えたりすると現在動いているものが動かなくなってしまいますので、なるべく増やすだけで対応をするようにしました。メンテナンスを入れて一気に変更しても良いのですが、それでも本番のデータで本当に正しく動くのかというのを（もちろんテストは書いているのですが）確認したかったので、新しくテーブルを作ってそこにデータをわざと（一部）重複させて、緩やかに新しいデータに移行させていく手法を採りました。この手法だと「本番サーバーのみでデータが怪しい可能性がある場合に、先にデータの計算だけ本番サーバーで実施できるので、本番で動かしつつ間違ってもやり直せる」といったメリットがあります。あまりベストプラクティスとは呼べないので、メンテ入れて置き換えで対応できる場合はそちらを採ったほうが良いと思われます。
具体的には、入り口（データの新規作成や更新が行われる箇所）が特定しやすいものはデータの作成処理をPOROに切り出して、そこで追加データの作成を行うようにします。入り口がすぐに特定できない（特に複数箇所から更新されるようなもの）ものに関しては、callbackを使って無理やり旧テーブルを新テーブルを同期させるようなことをしています。callbackは基本的に使うべきではない（前述）ので、これは早めに削除していきたいと思います。
命名が気になるとか、状態をフラグで管理しているとか、使ってない項目があるとかもありましたが、重要度に応じて無視したり対応したりするようにしました。「気に入らない」程度のものであれば無視するほうがプロジェクト全体の貢献度は高いと思われます。
どこを作り直すか
作り直しのポイントがいくつか出てきましたが、基本的には重要度の高いものだけを作り直しする必要があります。
たいていの場合はその時点でサービスが極端に落ちる回数が多いとかでなければ、開発側として辛いところがあってもそのままの状態でサービスは成立します。その上で作り直すポイントがあるとすれば以下のような判断基準になると思います。
- それを放置するといつか大問題になる
- 修正コストが極端に高いコード（コピペで似たような処理が複数存在する、など）
後者はそのままですが、前者で言えばインフラが手作りで落ちた場合の復元方法がなかったため、デプロイフローなどと併せて作り直すことにしました。インフラについてはこの後で解説します。
また、フロント寄りのコードで一部作り直す必要がありましたが、フロントに興味があるメンバーがいたので監修だけして任せることにしました。Vuex+TypeScriptでメンテナンスしやすいコードに少しずつ生まれ変わっています。
全体的なコードを直す上でも、「作り直さないとどうにもならない箇所」と「多少の修正でどうにかなる場所」と「多少汚くても放置しても問題がない箇所」の切り分けをしっかり行い、スコープを狭めて対応していくことで、サービスを止めずに自然な感じで対応してくことが可能になります。
インフラ
インフラは元々AWS（一部Heroku）にあったのですが、特に管理のされていないEC2に手動で構築したものが乗っかっているだけの状態でした。AWSは割とインスタンスを突然シャットダウンすることがあり、AMIを取っておけばある程度復元はできるものの障害があった時の不安や、そもそもOSのバージョンや入っているソフトウェアに一部問題がある状態だったので、このままでは継続的な開発に支障が出るだろうということで真っ先にリプレイスすることにしました。 今回はサービスの特性上、監視系やスケーリングで問題になる可能性が少なく、また少人数なのでなるべく運用で楽をしたいということでFargateを採用し、Terraformで一括管理するようにしています。以前までは環境変数の管理やベースイメージの作成にAnsibleを使っていましたが、利用ツールを減らしたかったのでAnsibleは使わずにDockerfileだけで構築するようにしました。事業規模が大きく、イメージの基本設定をRoleで使いまわしたい場合にはありなのかと思います。
Fargateにおけるジョブスケジューラーには Apache Airflow を採用しました。ECS Scheduled Tasksでもいいかなと思ったのですが、CloudWatch Eventsの制限数がきつかったのと、AWSのコンソールに結果を見に行かないといけないのが大変なので代替を検証していたところ、ECSのタスクをキックするECSOperatorが標準で存在するAirflowを採用しました。検証中に見つけたバグを修正したりしましたが、GCPでもCloud Composerとして採用されているため、基本的には安定性は高いと思われます。ECSの単発タスクをスケジューリングしているだけですが、将来的にはPythonでの分析タスクなども追加していきたいと思います。
Airflowの難点としては、AirflowからECSのタスクのログを確認することができず、タスクIDからCloudWatch Logsを見に行かないといけないというのが難点です。また、現行のAWSのrun-taskはLogConfigurationの上書きが許可されておらず、全てのタスクが同一のロググループ/ログストリーム名になってしまい、視認性が非常に悪いという問題があります。そこまでエラーが発生するシステムではないのと、Sentryを併用しているというところでフォローができるので今回は無視しました。
Fargateで動かす、つまりアプリケーションをDockerで動かすには、Dockerに対応する構造にする、具体的には12Factor Appに概ね準拠する必要があります。一番問題になりそうなローカルストレージの利用は一時ファイル以外にはなかったので問題がなかったのですが、ログ出力や設定の環境変数化などの細かい修正は多数存在しました。そもそも環境ごとにRAILS_ENVを作っており、そこに設定を直書きしていたので、それを少しずつ現行環境で剥がしながら、最終的にはRAILS_ENVをproduction/development/testのみするというところまで落とし込むことができました。厳密に12Factor Appに準拠するとRAILS_ENVも単一になるのではないかと思いますが、現実的にはdevelopment（自分の手元の開発環境のみで使用するもの）ではデバッガなどの開発用設定が多数入り、test（自動テスト時にのみ適用される）では外部通信の制御などの特殊な処理が多数入るため、これらの設定は利用したほうがいいのではないかと思います。それ以外は全て（ステージングとかは）productionで良いと思います。
ついでに開発環境もDocker化しておりますが、docker-composeで全て上がる環境と、アプリケーションはローカルの開発環境で直接上がる（MySQLやRedisといったTCPでのみやりとりするものだけdocker-composeで起動）環境の２種類を用意しております。非エンジニアの方や外部の方には前者ですぐ試せる環境、がっつり開発する場合には後者の環境という感じで使い分けております。これにより手軽さを担保しつつDocker(for Mac)で問題になりやすいストレージの遅さなどの問題を回避することができます。
インフラをDocker化し、staging/本番の環境差異をなくしつつミドルウェアの差し替えを容易にすることで、Rails本体やgemのアップグレードの検証も容易になります。ここは今後メンテナンスしていく上で欠かせないと思いましたので、無事に達成できてよかったと思います。これから順次アップグレードしていく予定です。
メンバーの立て直し
私より先に２名のエンジニアが参加していたのですが、新しい体制への対応と未知のコードへの対処に苦労しているようでした。
特に手が止まってしまう理由としては「何をどうすべきなのか交通整理ができない」というものがほとんどで、仮にプロジェクトの目的がしっかりしていたとしても、既存コードの理解が困難だったり、上記の作り直しの項目で取り上げた切り分けがうまくいかないという状態になっていることが多いようです。
このようになっている場合はまずタスクの振り分け作業を引き取り、作業できる粒度のタスクにまで落としてからタスクを渡してあげると上手く行きます。「悩んでいる」ということが良くない原因ですので、悩まないで快適に作業できる環境を提供してあげましょう。
コードレビューは行いますが、基本的には「こっちはテスターじゃないのでバグがないものを上げてるよね？」というスタンスで確認します。バグがなければ多少の書き方で気になるところは指摘しないようにします。「直さないと今後システム全体に大きな影響を与えるか」という観点で大枠ではレビューします。ただし厳しくしてほしいという人もいなくはないので、そのあたりは個別に相談して検討するのが良いでしょう。人数が少ないので基本的に全員がレビューするスタイルです。もちろん私のコードも他のメンバーにレビューしてもらいます。
ある程度書けるようになってきたら少しずつタスクを大きくし、現在は大きなタスクも手放しで任せられるようになりました。
まとめ
「レガシープロジェクトをどうにかする」というプロジェクトは過去にも何度か関わってきましたが、どの状況においても難しく考えずにレイヤーを分けて整理するというだけで目的に達成に近づくことができます。
その当時に最適なアーキテクチャでも技術スタックの進化などによっていずれは刷新を迫られるとは思いますが、特にアーキテクチャレベルでのレイヤーの分離度が高ければ高いほど、部分的に作り直すといった行為は手軽に行えるようになりますので、初期設計が良いアプリケーションであるほど作り直しが簡単になります。理想を言えばマイクロサービスと呼ばれている形式が良いのかもしれませんが、最近良く言われるマイクロサービスの形（それぞれ別のアプリケーションでAPIを通して結合するもの）をいきなり目指さなくても、モノリシックなアプリケーションでもそれぞれのレイヤーを適切に分離しておくことで、今後の変化にも柔軟に対応することが可能になります。
プロジェクト全体としても見通しを立ててポイントで価値の高い改善を行うことが重要で、そのためには場合によっては業務そのものに手を加える必要が出てくることもあります。適切なアーキテクチャ設計を行って良いプロダクト作りを行いましょう。