Warn at boot when a cell runs another hotcell release

CHANGELOG.md

@@ -38,6 +38,7 @@ Some actions that application developers should consider taking when upgrading f
 
 #### Added
 
+* `HotCell.describe_cells` warns at boot when a cell's `hotcell-server` version differs from the application's `hotcell-client` version. (#21)
 * `HotCell::LogSubscriber` writes one `info` line to the Rails log for each call. The line contains the cell, the operation, the outcome and both durations. It also contains the byte counts when the client measures them. The railtie attaches the subscriber. See the README's "Application logs".
 * `HotCell::HealthController` and `HotCell::DiagnosticsController` check each registered cell. `HotCell::HealthController` uses only the control socket and returns `OK` or `FAIL`. It takes no worker. `HotCell::DiagnosticsController` also sends `health.echo` and `health.reopen` on the work socket. It takes a worker for each round trip. It returns each result as JSON. To use the controllers, add a route for each to the application. Put the diagnostics route behind authentication. To do this, set `HotCell.diagnostics_controller_parent` in an initializer to the name of an authenticated controller class. Alternatively, subclass `HotCell::DiagnosticsController`, authenticate in the subclass, and route to the subclass. See the README's "Rails healthcheck".
 

README.md

@@ -260,7 +260,7 @@ class for each side of the permanent split:
 HotCell.root  = ENV["HOTCELL_ROOT"]  # unset means every cell is off
 HotCell.group = ENV["HOTCELL_GROUP"] # the gid shared between app and cell
 
-# Quick health check at boot. Warns about a cell that is unreachable, slower than this client waits, or in the wrong group.
+# Quick health check at boot. Warns about a cell that is unreachable, slower than this client waits, in the wrong group, or on another hotcell version.
 Rails.application.config.after_initialize { HotCell.describe_cells }
 ```
 

docs/DEPLOYMENT.md

@@ -71,9 +71,9 @@ and not only `/run/hotcell`. Docker creates a missing last level as root, and th
 socket in it. The installed Dockerfile does this. The application's image needs nothing at its own mount
 point.
 
-**Keep the app's and the cell's lockfiles in step.** They resolve hotcell separately. A skew between the
-client the app loads and the server the cell runs answers `protocol` on every request. While either tracks
-a branch rather than a released version, assert in a test that both lockfiles name the same revision.
+**Keep the app's and the cell's lockfiles in step.** They resolve hotcell separately.
+`HotCell.describe_cells` warns at boot when the cell's `hotcell-server` version differs from the app's
+`hotcell-client` version.
 
 
 ### Bringing your own container
@@ -422,7 +422,7 @@ HotCell.register "images",
 | `timeout:` | `30` | Seconds this caller waits for an answer to a work request. It must clear the cell's `answer_within`. A positive, finite number: `register` raises `ConfigurationError` on anything else, including `nil`, which stops the boot rather than leaving the wait for a cell to decide. |
 | `control_timeout:` | `5` | Seconds this caller waits for `describe` or `metrics`. The supervisor answers both inline, with no fork and no queue, so this is short on purpose and must not be raised toward `timeout`. It is what bounds app boot when a cell accepts connections and never answers, and what lets a health check report a wedged cell instead of waiting out a work timeout. Bounded like `timeout:`, and refused the same way. |
 | `permanent:`, `transient:` | the gem's classes | The exception classes the application raises for each side of the split. `transient` must not descend from `permanent`, and the client refuses to start if it does. |
-| `on_contract_skew:` | — | Called when a cell answers `protocol`, so a version mismatch is visible to an application that rescues broadly. |
+| `on_contract_skew:` | — | Called when a cell answers `protocol`, so a protocol version mismatch is visible to an application that rescues broadly. |
 
 ### The socket's file mode
 

hotcell-client/lib/hot_cell/cell.rb

@@ -74,7 +74,7 @@ def report_contract_skew(error)
     # deployment rather than a broken one, and an application that refuses to start because its thumbnail
     # cell is restarting is worse than one that serves placeholders. So this warns and carries on.
     #
-    # Nor when a cell answers something this client cannot read. The two warnings below reach into the
+    # Nor when a cell answers something this client cannot read. The warnings below reach into the
     # description without checking types, so a cell that sends the wrong ones raises here — and the README
     # calls `describe_cells` from `after_initialize`, where that is not a failed check but an application
     # that does not boot. The process that wrote the description is the one that runs untrusted content.
@@ -88,6 +88,7 @@ def describe
       response.result.tap do |described|
         warn_about_timeout described
         warn_about_group_skew described
+        warn_about_version_skew described
       end
     rescue StandardError => error
       HotCell.logger.warn "hotcell #{name}: this cell's description could not be read and is being " \
@@ -222,6 +223,14 @@ def warn_about_group_skew(described)
                             "every operation that gives a tool a filename will fail with EACCES."
       end
 
+      def warn_about_version_skew(described)
+        reported = described[:server_version]
+        return if reported == Client::VERSION
+
+        HotCell.logger.warn "hotcell #{name}: this client is #{Client::VERSION} and the cell reports " \
+                            "hotcell-server #{Failure.one_line reported.inspect}."
+      end
+
       # A `nil` timeout reaches `Transport::Socket#receive` as `deadline: nil`, and reading with no deadline
       # blocks: a cell that accepts the connection and then never answers holds this caller for good. At
       # boot that is an application that never finishes starting, with no exception and nothing to rescue.

hotcell-client/lib/hot_cell/install/Gemfile.tt

@@ -5,8 +5,7 @@
 
 source "https://rubygems.org"
 
-# Pinned to the client that installed it: the two are halves of one wire contract, and a skew between
-# them answers `protocol` on every request.
+# Keep this at the application's hotcell-client version; HotCell.describe_cells warns at boot otherwise.
 gem "hotcell-server", "<%= HotCell::Client::VERSION %>"
 
 # Gems your operations need go here. For example:

hotcell-client/test/describe_survival_test.rb

@@ -78,6 +78,17 @@ def test_a_cell_cannot_forge_a_log_line
     end
   end
 
+  def test_a_cell_cannot_forge_a_log_line_through_its_version
+    forged = %(ERROR -- : hotcell test: everything is fine)
+
+    with_cell_describing(%({"server_version":"0.0.1\\n#{forged}"})) do |cell|
+      warnings = capturing_warnings { cell.describe }
+
+      assert_match "0.0.1", warnings
+      assert_equal 1, warnings.lines.grep(/hotcell test/).size, "the cell wrote a line of its own: #{warnings}"
+    end
+  end
+
   def test_one_unusable_cell_does_not_stop_the_others_being_read
     Dir.mktmpdir "hc" do |root|
       answering root, "hostile", %({"answer_within":"attacker-controlled"})

hotcell-client/test/install_test.rb

@@ -41,8 +41,6 @@ def test_every_installer_template_is_packaged
     assert_empty templates - spec.files, "an installer template the published gem would not carry"
   end
 
-  # The Gemfile template is ERB rather than a copy, because the cell's server and this client are halves of
-  # one wire contract: a skew between them answers `protocol` on every request.
   def test_install_pins_the_cell_to_the_installing_clients_version
     Dir.mktmpdir do |root|
       HotCell::Install.call(root, out: StringIO.new)

hotcell-client/test/integration_test.rb

@@ -239,6 +239,30 @@ def test_describe_reports_the_groups_the_cell_runs_in
     end
   end
 
+  def test_describe_is_quiet_when_the_cell_runs_this_clients_version
+    with_cell do
+      refute_match "reports hotcell-server", capturing_warnings { HotCell.describe_cells }
+    end
+  end
+
+  def test_describe_warns_when_the_cell_runs_another_version
+    register_describing server_version: "0.0.1"
+
+    warnings = capturing_warnings { HotCell.describe_cells }
+
+    assert_match %(this client is #{HotCell::Client::VERSION} and the cell reports hotcell-server "0.0.1"), warnings
+  end
+
+  def test_describe_reads_and_warns_about_a_cell_too_old_to_report_its_version
+    register_describing operations: [ "test.echo" ]
+
+    described = nil
+    warnings = capturing_warnings { described = HotCell.describe_cells }
+
+    assert_equal [ "test.echo" ], described["test"][:operations]
+    assert_match "this client is #{HotCell::Client::VERSION} and the cell reports hotcell-server nil", warnings
+  end
+
   # Counts lag responses — the worker answers the caller before the supervisor reads its idle report and
   # increments the counter — so the wait is the assertion, exactly as in the server suite's metrics test.
   def test_metrics_come_back_through_the_registration
@@ -277,6 +301,12 @@ class Blocking < HotCell::Client
   end
 
   private
+    def register_describing(result)
+      HotCell.root = "/nowhere"
+      HotCell.register "test", permanent: Unprocessable, transient: TemporarilyUnavailable,
+                               transport: ->(*) { HotCell::Response.ok(result: result, timing: {}) }
+    end
+
     # A gid this process cannot be holding, so the check has something unambiguous to fail on.
     def unheld_group
       @unheld_group ||= ((1..65_000).to_a - Process.groups - [ Process.gid, Process.egid ]).last