New binder tests (#2160)

* Test restarts for binder

* Move connected_ports up in the file

* ENV replace -> update

* Prep for socket activation tests

* Small refactor

lib/puma/binder.rb

@@ -54,26 +54,30 @@ module Puma
       @ios.each { |i| i.close }
     end
 
-    def import_from_env
-      remove = []
+    def connected_ports
+      ios.map { |io| io.addr[1] }.uniq
+    end
 
-      ENV.each do |k,v|
-        if k =~ /PUMA_INHERIT_\d+/
-          fd, url = v.split(":", 2)
-          @inherited_fds[url] = fd.to_i
-          remove << k
-        elsif k == 'LISTEN_FDS' && ENV['LISTEN_PID'].to_i == $$
-          v.to_i.times do |num|
-            fd = num + 3
-            sock = TCPServer.for_fd(fd)
-            begin
-              key = [ :unix, Socket.unpack_sockaddr_un(sock.getsockname) ]
-            rescue ArgumentError
+    def import_from_env(env_hash)
+      remove = []
+      remove += create_inherited_fds(env_hash)
+      env_hash.each do |k,v|
+        if k == 'LISTEN_FDS' && ENV['LISTEN_PID'].to_i == $$
+          # systemd socket activation.
+          # LISTEN_FDS = number of listening sockets. e.g. 2 means accept on 2 sockets w/descriptors 3 and 4.
+          # LISTEN_PID = PID of the service process, aka us
+          # see https://www.freedesktop.org/software/systemd/man/systemd-socket-activate.html
+
+          number_of_sockets_to_listen_on = v.to_i
+          number_of_sockets_to_listen_on.times do |index|
+            fd = index + 3 # 3 is the magic number you add to follow the SA protocol
+            sock = TCPServer.for_fd(fd)  # TODO: change to BasicSocket?
+            key = begin # Try to parse as a path
+              [:unix, Socket.unpack_sockaddr_un(sock.getsockname)]
+            rescue ArgumentError # Try to parse as a port/ip
               port, addr = Socket.unpack_sockaddr_in(sock.getsockname)
-              if addr =~ /\:/
-                addr = "[#{addr}]"
-              end
-              key = [ :tcp, addr, port ]
+              addr = "[#{addr}]" if addr =~ /\:/
+              [:tcp, addr, port]
             end
             @activated_sockets[key] = sock
             @events.debug "Registered #{key.join ':'} for activation from LISTEN_FDS"
@@ -81,10 +85,7 @@ module Puma
           remove << k << 'LISTEN_PID'
         end
       end
-
-      remove.each do |k|
-        ENV.delete k
-      end
+      remove
     end
 
     def parse(binds, logger, log_msg = 'Listening')
@@ -232,10 +233,6 @@ module Puma
       tcp_server
     end
 
-    def connected_ports
-      ios.map { |io| io.addr[1] }.uniq
-    end
-
     def inherit_tcp_listener(host, port, fd)
       if fd.kind_of? TCPServer
         s = fd
@@ -366,14 +363,17 @@ module Puma
     end
 
     def redirects_for_restart
-      redirects = {:close_others => true}
-      @listeners.each_with_index do |(l, io), i|
-        ENV["PUMA_INHERIT_#{i}"] = "#{io.to_i}:#{l}"
-        redirects[io.to_i] = io.to_i
-      end
+      redirects = listeners.map { |a| [a[1].to_i, a[1].to_i] }.to_h
+      redirects[:close_others] = true
       redirects
     end
 
+    def redirects_for_restart_env
+      listeners.each_with_object({}).with_index do |(listen, memo), i|
+        memo["PUMA_INHERIT_#{i}"] = "#{listen[1].to_i}:#{listen[0]}"
+      end
+    end
+
     private
 
     def loopback_addresses
@@ -381,5 +381,15 @@ module Puma
         addrinfo.ipv6_loopback? || addrinfo.ipv4_loopback?
       end.map { |addrinfo| addrinfo.ip_address }.uniq
     end
+
+    # def create_activated_sockets(env_hash)
+    # end
+
+    def create_inherited_fds(env_hash)
+      env_hash.select {|k,v| k =~ /PUMA_INHERIT_\d+/}.each do |_k, v|
+        fd, url = v.split(":", 2)
+        @inherited_fds[url] = fd.to_i
+      end.keys # pass keys back for removal
+    end
   end
 end

lib/puma/launcher.rb

@@ -48,7 +48,8 @@ module Puma
       @config        = conf
 
       @binder        = Binder.new(@events)
-      @binder.import_from_env
+      env_to_remove = @binder.import_from_env(ENV)
+      env_to_remove.each { |k| ENV.delete k }
 
       @environment = conf.environment
 
@@ -253,6 +254,7 @@ module Puma
       else
         argv = restart_args
         Dir.chdir(@restart_dir)
+        ENV.update(@binder.redirects_for_restart_env)
         argv += [@binder.redirects_for_restart]
         Kernel.exec(*argv)
       end

test/test_binder.rb

@@ -192,7 +192,25 @@ class TestBinder < TestBinderBase
     assert @mocked_ios.each(&:verify)
   end
 
-  # test redirect for restart
+  def test_redirects_for_restart_creates_a_hash
+    @binder.parse ["tcp://127.0.0.1:0"], @events
+
+    result = @binder.redirects_for_restart
+    ios = @binder.listeners.map { |_l, io| io.to_i }
+
+    ios.each { |int| assert_equal int, result[int] }
+    assert result[:close_others]
+  end
+
+  def test_redirects_for_restart_env
+    @binder.parse ["tcp://127.0.0.1:0"], @events
+
+    result = @binder.redirects_for_restart_env
+
+    @binder.listeners.each_with_index do |l, i|
+      assert_equal "#{l[1].to_i}:#{l[0]}", result["PUMA_INHERIT_#{i}"]
+    end
+  end
 
   def test_close_listeners_closes_ios
     @binder.parse ["tcp://127.0.0.1:#{UniquePort.call}"], @events
@@ -230,6 +248,22 @@ class TestBinder < TestBinderBase
     refute File.socket?("test/#{name}_server.sock")
   end
 
+  def test_import_from_env_listen_inherit
+    @binder.parse ["tcp://127.0.0.1:0"], @events
+    removals = @binder.import_from_env(@binder.redirects_for_restart_env)
+
+    @binder.listeners.each do |url, io|
+      assert_equal io.to_i, @binder.inherited_fds[url]
+    end
+    assert_includes removals, "PUMA_INHERIT_0"
+  end
+
+  # test socket activation with tcp
+  # test socket activation with IPv6
+  # test socket activation with Unix
+  # test socket activation logs to events
+  # test socket activation returns the right keys to remove
+
   private
 
   def assert_parsing_logs_uri(order = [:unix, :tcp])