diff --git a/lib/kubernetes-deploy/sync_mediator.rb b/lib/kubernetes-deploy/sync_mediator.rb index f90e45e47..265a39d3d 100644 --- a/lib/kubernetes-deploy/sync_mediator.rb +++ b/lib/kubernetes-deploy/sync_mediator.rb @@ -32,7 +32,7 @@ def get_all(kind, selector = nil) def sync(resources) clear_cache - if resources.count < LARGE_BATCH_THRESHOLD + if resources.count > LARGE_BATCH_THRESHOLD dependencies = resources.map(&:class).uniq.flat_map do |c| c::SYNC_DEPENDENCIES if c.const_defined?('SYNC_DEPENDENCIES') end diff --git a/test/test_helper.rb b/test/test_helper.rb index ab4a2d561..f766b3a7a 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -192,7 +192,7 @@ def fixture_path(set_name) def stub_kubectl_response(*args, resp:, err: "", success: true, json: true, times: 1) resp = resp.to_json if json - response = [resp, err, stub(success?: success)] + response = times > 0 ? [resp, err, stub(success?: success)] : [] KubernetesDeploy::Kubectl.any_instance.expects(:run) .with(*args) .returns(response) diff --git a/test/unit/sync_mediator_test.rb b/test/unit/sync_mediator_test.rb index 809d0ce7c..cc86284f6 100644 --- a/test/unit/sync_mediator_test.rb +++ b/test/unit/sync_mediator_test.rb @@ -90,7 +90,8 @@ def test_sync_caches_the_types_of_the_instances_given all_cm = [@fake_cm.kubectl_response, @fake_cm2.kubectl_response] stub_kubectl_response('get', 'FakeConfigMap', *@params, resp: { "items" => all_cm }, times: 1) stub_kubectl_response('get', 'FakePod', *@params, resp: { "items" => [@fake_pod.kubectl_response] }, times: 1) - mediator.sync([@fake_cm, @fake_pod]) + # Cache is only warmed if we batch fetch + mediator.sync([@fake_cm, @fake_pod] * KubernetesDeploy::SyncMediator::LARGE_BATCH_THRESHOLD) # These should use the warm cache assert_equal @fake_cm.kubectl_response, mediator.get_instance('FakeConfigMap', @fake_cm.name) @@ -106,7 +107,9 @@ def test_sync_caches_the_types_of_the_dependencies_of_the_instances_given stub_kubectl_response('get', 'FakeDeployment', *@params, resp: { "items" => [@fake_deployment.kubectl_response] }, times: 1) stub_kubectl_response('get', 'FakePod', *@params, resp: { "items" => [@fake_pod.kubectl_response] }, times: 1) - mediator.sync([@fake_deployment]) # pod is a depedency so should get cached too + # Dependency fetching is only done if we batch fetch + # pod is a depedency so should get cached too + mediator.sync([@fake_deployment] * (KubernetesDeploy::SyncMediator::LARGE_BATCH_THRESHOLD + 1)) assert_equal @fake_deployment.kubectl_response, mediator.get_instance('FakeDeployment', @fake_deployment.name) assert_equal @fake_pod.kubectl_response, mediator.get_instance('FakePod', @fake_pod.name) @@ -118,18 +121,31 @@ def test_calling_instance_sync_does_not_allow_instances_to_affect_the_global_cac bad_citizen = BadCitizen.new('foo') stub_kubectl_response('get', 'BadCitizen', *@params, resp: { "items" => [bad_citizen.kubectl_response] }, times: 1) - mediator.sync([bad_citizen]) + # Cache is only warmed if we batch fetch + mediator.sync([bad_citizen] * (KubernetesDeploy::SyncMediator::LARGE_BATCH_THRESHOLD + 1)) assert_equal bad_citizen.kubectl_response, mediator.get_instance('BadCitizen', bad_citizen.name) # still cached end def test_sync_calls_sync_on_each_instance - stub_kubectl_response('get', 'FakeDeployment', *@params, - resp: { "items" => [@fake_deployment.kubectl_response] }, times: 1) - stub_kubectl_response('get', 'FakePod', *@params, resp: { "items" => [@fake_pod.kubectl_response] }, times: 1) - all_cm = [@fake_cm.kubectl_response, @fake_cm2.kubectl_response] - stub_kubectl_response('get', 'FakeConfigMap', *@params, resp: { "items" => all_cm }, times: 1) + test_resources = [@fake_pod, @fake_cm, @fake_deployment] + test_resources.each { |r| r.expects(:sync).once } + mediator.sync(test_resources) + end + + def test_sync_does_not_batch_with_few_resources + stub_kubectl_response('get', 'FakeConfigMap', @fake_cm.name, *@params, + resp: { "items" => [@fake_cm.kubectl_response] }, times: 0) + + test_resources = [@fake_cm] + test_resources.each { |r| r.expects(:sync).once } + mediator.sync(test_resources) + end + + def test_sync_does_batch_with_enough_resources + stub_kubectl_response('get', 'FakeConfigMap', *@params, + resp: { "items" => [@fake_cm.kubectl_response] }, times: 1) - test_resources = [@fake_pod, @fake_cm, @fake_cm2, @fake_deployment] + test_resources = [@fake_cm] * (KubernetesDeploy::SyncMediator::LARGE_BATCH_THRESHOLD + 1) test_resources.each { |r| r.expects(:sync).once } mediator.sync(test_resources) end