From 9124a77b6eb05ffe512f28a07f5c559afa63c322 Mon Sep 17 00:00:00 2001
From: Michael Grosser <michael@grosser.it>
Date: Wed, 10 Jul 2019 17:12:10 -0700
Subject: [PATCH] be more efficient by reusing the response of create and
 update

---
 plugins/kubernetes/app/models/kubernetes/resource.rb      | 8 ++------
 .../test/models/kubernetes/deploy_executor_test.rb        | 7 ++++---
 .../kubernetes/test/models/kubernetes/resource_test.rb    | 4 ++--
 3 files changed, 8 insertions(+), 11 deletions(-)

diff --git a/plugins/kubernetes/app/models/kubernetes/resource.rb b/plugins/kubernetes/app/models/kubernetes/resource.rb
index 3f5e27b8a1..37413dfb7c 100644
--- a/plugins/kubernetes/app/models/kubernetes/resource.rb
+++ b/plugins/kubernetes/app/models/kubernetes/resource.rb
@@ -117,23 +117,19 @@ def expire_resource_cache
         remove_instance_variable(:@resource) if defined?(@resource)
       end
 
-      # TODO: remove the expire_cache and assign @resource but that breaks a bunch of deploy_executor tests
       def create
         return if @delete_resource
         restore_template do
           @template[:metadata].delete(:resourceVersion)
-          request(:create, @template)
+          @resource = request(:create, @template)
         end
-        expire_resource_cache
       rescue Kubeclient::ResourceNotFoundError => e
         raise_kubernetes_error(e.message)
       end
 
-      # TODO: remove the expire_cache and assign @resource but that breaks a bunch of deploy_executor tests
       def update
         ensure_not_updating_match_labels
-        request(:update, template_for_update)
-        expire_resource_cache
+        @resource = request(:update, template_for_update)
       end
 
       def ensure_not_updating_match_labels
diff --git a/plugins/kubernetes/test/models/kubernetes/deploy_executor_test.rb b/plugins/kubernetes/test/models/kubernetes/deploy_executor_test.rb
index 72b1726705..9af6d99904 100644
--- a/plugins/kubernetes/test/models/kubernetes/deploy_executor_test.rb
+++ b/plugins/kubernetes/test/models/kubernetes/deploy_executor_test.rb
@@ -82,7 +82,8 @@ def worker_is_unstable
 
       stub_request(:get, "#{deployments_url}/test-app-server").to_return(status: 404) # previous deploys ? -> none!
       stub_request(:get, "#{deployments_url}/test-resque-worker").to_return(status: 404) # previous deploys ? -> none!
-      stub_request(:post, deployments_url).to_return(body: "{}") # creates deployment
+      stub_request(:post, deployments_url). # creates deployment
+        to_return(body: {spec: {selector: {matchLabels: {project: "some-project", role: "some-role"}}}}.to_json)
       stub_request(:put, "#{deployments_url}/test-resque-worker").to_return(body: '{}') # during delete for rollback
 
       Kubernetes::DeployExecutor.any_instance.stubs(:sleep) # not using .executor to keep it uninitialized
@@ -608,7 +609,7 @@ def worker_is_unstable
         out.must_include rollback_indicator
         out.must_include "DONE" # DONE is shown ... we got past the rollback
         out.wont_include "SUCCESS"
-        out.wont_include "FAILED"
+        out.wont_include "FAILED" # rollback failed
       end
 
       it "deletes when there was no previous deployed resource" do
@@ -620,7 +621,7 @@ def worker_is_unstable
         out.wont_include rollback_indicator
         out.must_include "DONE" # DONE is shown ... we got past the rollback
         out.wont_include "SUCCESS"
-        out.wont_include "FAILED"
+        out.wont_include "FAILED" # rollback failed
       end
 
       it "does not crash when rollback fails" do
diff --git a/plugins/kubernetes/test/models/kubernetes/resource_test.rb b/plugins/kubernetes/test/models/kubernetes/resource_test.rb
index cc15cde02a..79f7b78679 100644
--- a/plugins/kubernetes/test/models/kubernetes/resource_test.rb
+++ b/plugins/kubernetes/test/models/kubernetes/resource_test.rb
@@ -107,7 +107,7 @@ def assert_create_and_delete_requests(**args, &block)
       let(:url) { "#{origin}/apis/extensions/v1beta1/namespaces/pod1/deployments/some-project" }
 
       it "creates when missing" do
-        assert_request(:get, url, to_return: [{status: 404}, {body: "{}"}]) do
+        assert_request(:get, url, to_return: {status: 404}) do
           assert_request(:post, base_url, to_return: {body: "{}"}) do
             resource.deploy
           end
@@ -119,7 +119,7 @@ def assert_create_and_delete_requests(**args, &block)
       end
 
       it "updates existing" do
-        assert_request(:get, url, to_return: {body: "{}"}, times: 2) do
+        assert_request(:get, url, to_return: {body: "{}"}) do
           args = ->(x) { x.body.must_include '"replicas":2'; true }
           assert_request(:put, url, to_return: {body: "{}"}, with: args) do
             resource.deploy