Merge pull request #2974 from ganmacs/fix-warning-of-sd-manager

Fix warning of sd manager

lib/fluent/plugin_helper/service_discovery.rb

@@ -43,6 +43,13 @@ def start
         super
       end
 
+      %i[after_start stop before_shutdown shutdown after_shutdown close terminate].each do |mth|
+        define_method(mth) do
+          @discovery_manager&.__send__(mth)
+          super()
+        end
+      end
+
       private
 
       # @param title [Symbol] the thread name. this value should be unique.

lib/fluent/plugin_helper/service_discovery/manager.rb

@@ -60,6 +60,14 @@ def start
           end
         end
 
+        %i[after_start stop before_shutdown shutdown after_shutdown close terminate].each do |mth|
+          define_method(mth) do
+            @discoveries.each do |d|
+              d.__send__(mth)
+            end
+          end
+        end
+
         def run_once
           # Don't care race in this loop intentionally
           s = @queue.size

test/plugin_helper/test_service_discovery.rb

@@ -4,9 +4,6 @@
 require 'fluent/plugin/output'
 
 class ServiceDiscoveryHelper < Test::Unit::TestCase
-  PORT = unused_port
-  NULL_LOGGER = Logger.new(nil)
-
   class Dummy < Fluent::Plugin::TestBase
     helpers :service_discovery
 
@@ -30,6 +27,7 @@ def discovery_manager
     if @d
       @d.stop unless @d.stopped?
       @d.shutdown unless @d.shutdown?
+      @d.after_shutdown unless @d.after_shutdown?
       @d.close unless @d.closed?
       @d.terminate unless @d.terminated?
     end
@@ -40,7 +38,7 @@ def discovery_manager
 
     d.service_discovery_create_manager(
       :service_discovery_helper_test,
-      configurations: [{ type: :static, conf:  config_element('root', '', {}, [config_element('service', '', { 'host' => '127.0.0.1', 'port' => '1234' })]) }],
+      configurations: [{ type: :static, conf: config_element('root', '', {}, [config_element('service', '', { 'host' => '127.0.0.1', 'port' => '1234' })]) }],
     )
 
     assert_true !!d.discovery_manager
@@ -49,6 +47,7 @@ def discovery_manager
     mock.proxy(d).timer_execute(:service_discovery_helper_test, anything).never
 
     d.start
+    d.event_loop_wait_until_start
 
     services = d.discovery_manager.services
     assert_equal 1, services.size
@@ -68,5 +67,39 @@ def discovery_manager
     mock.proxy(d.discovery_manager).start.once
     mock(d).timer_execute(:service_discovery_helper_test, anything).once
     d.start
+    d.event_loop_wait_until_start
+  end
+
+  test 'exits service discovery instances without any errors' do
+    d = @d = Dummy.new
+    mockv = flexmock('dns_resolver', getaddress: '127.0.0.1')
+              .should_receive(:getresources)
+              .and_return([Resolv::DNS::Resource::IN::SRV.new(1, 10, 8081, 'service1.example.com')])
+              .mock
+    mock(Resolv::DNS).new { mockv }
+
+    d.service_discovery_create_manager(
+      :service_discovery_helper_test2,
+      configurations: [{ type: :srv, conf: config_element('service_discovery', '', { 'service' => 'service1', 'hostname' => 'example.com' }) }],
+    )
+
+    assert_true !!d.discovery_manager
+    mock.proxy(d.discovery_manager).start.once
+    mock(d).timer_execute(:service_discovery_helper_test2, anything).once
+
+    # To avoid claring `@logs` during `terminate` step
+    # https://github.com/fluent/fluentd/blob/bc78d889f93dad8c2a4e0ad1ca802546185dacba/lib/fluent/test/log.rb#L33
+    mock(d.log).reset.twice
+
+    d.start
+    d.event_loop_wait_until_start
+
+    d.stop unless d.stopped?
+    d.shutdown unless d.shutdown?
+    d.after_shutdown unless d.after_shutdown?
+    d.close unless d.closed?
+    d.terminate unless d.terminated?
+
+    assert_false(d.log.out.logs.any? { |e| e.match?(/thread doesn't exit correctly/) })
   end
 end