Don't spin on containers that are auto-restarting

Don’t spin on containers that are auto-restarting

State.Running=true does not mean that a container is running… oh no, that would be too fucking simple. Only State.Status=“running” means that the container is actually running.

diff --git a/lib/mobystash/container.rb b/lib/mobystash/container.rb
index b87687b..cfe1cbc 100644
--- a/lib/mobystash/container.rb
+++ b/lib/mobystash/container.rb
@@ -157,16 +157,16 @@ module Mobystash
     end
 
     def process_events(conn)
-      if tty?(conn)
-        @config.log_entries_sent_counter.increment({ container_name: @name, container_id: @id, stream: "tty" }, 0)
-      else
-        @config.log_entries_sent_counter.increment({ container_name: @name, container_id: @id, stream: "stdout" }, 0)
-        @config.log_entries_sent_counter.increment({ container_name: @name, container_id: @id, stream: "stderr" }, 0)
-      end
+      begin
+        if tty?(conn)
+          @config.log_entries_sent_counter.increment({ container_name: @name, container_id: @id, stream: "tty" }, 0)
+        else
+          @config.log_entries_sent_counter.increment({ container_name: @name, container_id: @id, stream: "stdout" }, 0)
+          @config.log_entries_sent_counter.increment({ container_name: @name, container_id: @id, stream: "stderr" }, 0)
+        end
 
-      if @capture_logs
-        begin
-          unless Docker::Container.get(@id, {}, conn).info.fetch("State", {}).fetch("Running")
+        if @capture_logs
+          unless Docker::Container.get(@id, {}, conn).info.fetch("State", {})["Status"] == "running"
             @logger.debug(progname) { "Container is not running; waiting for it to start or be destroyed" }
             wait_for_container_to_start(conn)
           else
@@ -195,24 +195,24 @@ module Mobystash
               response_block: chunk_parser
             )
           end
-        rescue Docker::Error::NotFoundError, Docker::Error::ServerError
-          # This happens when the container terminates, but we beat the System
-          # in the race and we call Docker::Container.get before the System
-          # shuts us down.  Since we'll be terminated soon anyway, we may as
-          # well do it first.
-          @logger.info(progname) { "Container has terminated." }
-          raise TerminateEventWorker
+        else
+          @logger.debug(progname) { "Not capturing logs because mobystash is disabled" }
+          sleep
         end
-      else
-        @logger.debug(progname) { "Not capturing logs because mobystash is disabled" }
-        sleep
+      rescue Docker::Error::NotFoundError, Docker::Error::ServerError
+        # This happens when the container terminates, but we beat the System
+        # in the race and we call Docker::Container.get before the System
+        # shuts us down.  Since we'll be terminated soon anyway, we may as
+        # well do it first.
+        @logger.info(progname) { "Container has terminated." }
+        raise TerminateEventWorker
       end
     end
 
     def wait_for_container_to_start(conn)
       @logger.debug(progname) { "Asking for events since #{@last_log_timestamp}" }
 
-      Docker::Event.since(Time.strptime(@last_log_timestamp, "%FT%T.%N%Z") + ONE_NANOSECOND.strftime("%s.%N"), {}, conn) do |event|
+      Docker::Event.since((Time.strptime(@last_log_timestamp, "%FT%T.%N%Z") + ONE_NANOSECOND).strftime("%s.%N"), {}, conn) do |event|
         @last_log_timestamp = event.time
 
         @logger.debug(progname) { "Docker event@#{event.timeNano}: #{event.Type}.#{event.Action} on #{event.ID}" }
diff --git a/spec/container_spec.rb b/spec/container_spec.rb
index 488967b..968b5c5 100644
--- a/spec/container_spec.rb
+++ b/spec/container_spec.rb
@@ -55,7 +55,7 @@ describe Mobystash::Container do
       allow(mock_conn).to receive(:get).and_raise(Mobystash::MobyEventWorker.const_get(:TerminateEventWorker))
       allow(Docker::Container).to receive(:new).with(instance_of(Docker::Connection), instance_of(Hash)).and_call_original
       allow(Docker::Container).to receive(:get).with(container_id, {}, mock_conn).and_return(mock_moby_container)
-      allow(mock_moby_container).to receive(:info).and_return("Config" => { "Tty" => false }, "State" => { "Running" => true })
+      allow(mock_moby_container).to receive(:info).and_return("Config" => { "Tty" => false }, "State" => { "Status" => "running" })
 
       # I'm a bit miffed we have to do this; to my mind, a double should
       # lie a little
@@ -194,7 +194,7 @@ describe Mobystash::Container do
       let(:container_id)   { "asdfasdfdisabled" }
 
       it "does not ask for logs" do
-        expect(Docker::Container).to_not receive(:get)
+        allow(Docker::Container).to receive(:get).and_return(Struct.new(:info).new("Config" => {}, "State" => { "Status" => "running" }))
         expect(container).to receive(:sleep).with(no_args).and_raise(Mobystash::MobyEventWorker.const_get(:TerminateEventWorker))
 
         container.run
@@ -354,7 +354,7 @@ describe Mobystash::Container do
       let(:container_id)   { "asdfasdftty" }
 
       before :each do
-        allow(mock_moby_container).to receive(:info).and_return("Config" => { "Tty" => true }, "State" => { "Running" => true })
+        allow(mock_moby_container).to receive(:info).and_return("Config" => { "Tty" => true }, "State" => { "Status" => "running" })
       end
 
       it "asks for a pro-TTY chunk parser" do
@@ -671,7 +671,7 @@ describe Mobystash::Container do
       allow(Docker::Connection).to receive(:new).with("unix:///var/run/docker.sock", read_timeout: 3600).and_return(mock_conn)
       allow(Docker::Container).to receive(:new).with(instance_of(Docker::Connection), instance_of(Hash)).and_call_original
       allow(Docker::Container).to receive(:get).with(container_id, {}, mock_conn).and_return(mock_moby_container)
-      allow(mock_moby_container).to receive(:info).and_return("Config" => { "Tty" => false }, "State" => { "Running" => true })
+      allow(mock_moby_container).to receive(:info).and_return("Config" => { "Tty" => false }, "State" => { "Status" => "running" })
 
       # I'm a bit miffed we have to do this; to my mind, a double should
       # lie a little

GitHub sha: a4b08be6

2 Likes