Code review

Shopify · Apr 4, 2018 · a732963 · a732963
1 parent e9e8d30
commit a732963
Show file tree

Hide file tree

Showing 4 changed files with 6 additions and 16 deletions.
diff --git a/lib/kubernetes-deploy/kubernetes_resource/cloudsql.rb b/lib/kubernetes-deploy/kubernetes_resource/cloudsql.rb
@@ -11,11 +11,7 @@ def sync(mediator)
     end
 
     def status
-      if proxy_deployment_ready? && proxy_service_ready?
-        "Provisioned"
-      else
-        "Unknown"
-      end
+      deploy_succeeded? ? "Provisioned" : "Unknown"
     end
 
     def deploy_succeeded?

diff --git a/lib/kubernetes-deploy/kubernetes_resource/memcached.rb b/lib/kubernetes-deploy/kubernetes_resource/memcached.rb
@@ -13,15 +13,11 @@ def sync(mediator)
     end
 
     def status
-      if deployment_ready? && service_ready? && configmap_ready?
-        "Provisioned"
-      else
-        "Unknown"
-      end
+      deploy_succeeded? ? "Provisioned" : "Unknown"
     end
 
     def deploy_succeeded?
-      @deployment_exists && @service_exists && @configmap_exists
+      deployment_ready? && service_ready? && configmap_ready?
     end
 
     def deploy_failed?

diff --git a/lib/kubernetes-deploy/kubernetes_resource/redis.rb b/lib/kubernetes-deploy/kubernetes_resource/redis.rb
@@ -12,11 +12,7 @@ def sync(mediator)
     end
 
     def status
-      if deployment_ready? && service_ready?
-        "Provisioned"
-      else
-        "Unknown"
-      end
+      deploy_succeeded? ? "Provisioned" : "Unknown"
     end
 
     def deploy_succeeded?

diff --git a/test/unit/sync_mediator_test.rb b/test/unit/sync_mediator_test.rb
@@ -53,6 +53,8 @@ def test_get_all_populates_cache_and_returns_array_of_instance_hashes
   def test_get_all_does_not_cache_error_result_from_kubectl
     stub_kubectl_response('get', 'FakeConfigMap', *@params, success: false, resp: { "items" => [] }, err: 'no').times(2)
     stub_kubectl_response('get', 'FakeConfigMap', @fake_cm.name, *@params, resp: @fake_cm.kubectl_response, times: 1)
+
+    # Neither the main code path nor the selector-based code path should cause error results to be cached
     assert_equal [], mediator.get_all('FakeConfigMap')
     assert_equal [], mediator.get_all('FakeConfigMap', "fake" => "false", "type" => "fakeconfigmap")
     assert_equal @fake_cm.kubectl_response, mediator.get_instance('FakeConfigMap', @fake_cm.name)