check if accessConfig exists before deleting

lib/hosting/gcp_apis.rb

@@ -170,9 +170,6 @@ def delete_ephermal_ipv4(vm_name, zone)
     connection = Excon.new(@host[:connection_string], headers: @host[:headers])
     query = {accessConfig: "External NAT", networkInterface: "nic0"}
     response = connection.post(path: "/compute/v1/projects/#{@project}/zones/#{zone}/instances/#{vm_name}/deleteAccessConfig", query: query, expects: [200, 400, 404])
-    if response.status == 404
-      return
-    end
     Hosting::GcpApis.check_errors(response)
     data = JSON.parse(response.body)
     wait_for_operation(zone, data["id"])

model/gcp_vm.rb

@@ -72,17 +72,25 @@ def swap_ip(vm)
     gcp_client = Hosting::GcpApis.new
     zone1 = "#{location}-a"
     zone2 = "#{vm.location}-a"
-    gcp_client.delete_ephermal_ipv4(name, zone1)
-    gcp_client.delete_ephermal_ipv4(vm.name, zone2)
-    vm_info = gcp_client.get_vm(name, zone1)
+    vm_info1 = gcp_client.get_vm(name, zone1)
+    vm_info2 = gcp_client.get_vm(vm.name, zone2)
+
+    if vm_info1["networkInterfaces"][0]["accessConfigs"].find { _1["name"] == "External NAT" }
+      gcp_client.delete_ephermal_ipv4(name, zone1)
+    end
+
+    if vm_info2["networkInterfaces"][0]["accessConfigs"].find { _1["name"] == "External NAT" }
+      gcp_client.delete_ephermal_ipv4(vm.name, zone2)
+    end
+
     # we are explicitly checking if ip is already assigned
     # because the operation can be terminated while running
     # and on next retry we will have error that the external ip is already assigned
-    if !vm_info["networkInterfaces"][0]["accessConfigs"].find { _1["natIP"] == vm.sshable.host }
+    if !vm_info1["networkInterfaces"][0]["accessConfigs"].find { _1["natIP"] == vm.sshable.host }
       gcp_client.assign_static_ipv4(name, vm.sshable.host, zone1)
     end
-    vm_info = gcp_client.get_vm(vm.name, zone2)
-    if !vm_info["networkInterfaces"][0]["accessConfigs"].find { _1["natIP"] == sshable.host }
+
+    if !vm_info2["networkInterfaces"][0]["accessConfigs"].find { _1["natIP"] == sshable.host }
       gcp_client.assign_static_ipv4(vm.name, sshable.host, zone2)
     end
 

spec/lib/hosting/gcp_apis_spec.rb

@@ -101,13 +101,6 @@
     end
 
     describe "#delete_ephermal_ipv4" do
-      it "does nothing on 404" do
-        stub_request(:post, "https://oauth2.googleapis.com/token").to_return(status: 200, body: JSON.dump({}))
-        stub_request(:post, "https://compute.googleapis.com/compute/v1/projects/test-project/zones/us-central1-a/instances/dummy-vm/deleteAccessConfig?accessConfig=External%20NAT&networkInterface=nic0").to_return(status: 404, body: JSON.dump({"errors" => [{"error" => "test"}]}), headers: {"Content-Type" => "application/json"})
-        api = described_class.new
-        expect { api.delete_ephermal_ipv4("dummy-vm", "us-central1-a") }.not_to raise_error
-      end
-
       it "deletes ephermal address from vm" do
         stub_request(:post, "https://oauth2.googleapis.com/token").to_return(status: 200, body: JSON.dump({}), headers: {"Content-Type" => "application/json"})
         stub_request(:post, "https://compute.googleapis.com/compute/v1/projects/test-project/zones/us-central1-a/instances/dummy-vm/deleteAccessConfig?accessConfig=External%20NAT&networkInterface=nic0").to_return(status: 200, body: JSON.dump({"id" => "test-op"}), headers: {"Content-Type" => "application/json"})

spec/model/gcp_vm_spec.rb

@@ -77,7 +77,25 @@
       api = instance_double(Hosting::GcpApis)
       expect(api).to receive(:delete_ephermal_ipv4).with("vm1", "us-central1-a")
       expect(api).to receive(:delete_ephermal_ipv4).with("vm2", "us-central1-a")
-      expect(api).to receive(:get_vm).and_return({"networkInterfaces" => [{"accessConfigs" => [{"natIP" => "ip2"}]}]}, {"networkInterfaces" => [{"accessConfigs" => [{"natIP" => "ip1"}]}]})
+      expect(api).to receive(:get_vm).and_return({"networkInterfaces" => [{"accessConfigs" => [{"name" => "External NAT", "natIP" => "ip2"}]}]}, {"networkInterfaces" => [{"accessConfigs" => [{"name" => "External NAT", "natIP" => "ip1"}]}]})
+      expect(Hosting::GcpApis).to receive(:new).and_return(api)
+      vm2 = instance_double(described_class, name: "vm2", address_name: "vm2-addr", location: "us-central1", sshable: instance_double(Sshable, host: "ip2"))
+      expect(gcp_vm).to receive(:sshable).and_return(instance_double(Sshable, host: "ip1")).at_least(:once)
+      expect(gcp_vm.sshable).to receive(:invalidate_cache_entry)
+      expect(vm2.sshable).to receive(:invalidate_cache_entry)
+      expect(gcp_vm.sshable).to receive(:update).with(host: "temp_vm1")
+      expect(gcp_vm.sshable).to receive(:update).with(host: "ip2")
+      expect(vm2.sshable).to receive(:update).with(host: "ip1")
+      expect(gcp_vm).to receive(:update).with(address_name: "vm2-addr")
+      expect(vm2).to receive(:update).with(address_name: "vm1-addr")
+      expect { gcp_vm.swap_ip(vm2) }.not_to raise_error
+    end
+
+    it "does not delete if no public ip" do
+      api = instance_double(Hosting::GcpApis)
+      expect(api).to receive(:assign_static_ipv4).with("vm1", "ip2", "us-central1-a")
+      expect(api).to receive(:assign_static_ipv4).with("vm2", "ip1", "us-central1-a")
+      expect(api).to receive(:get_vm).and_return({"networkInterfaces" => [{"accessConfigs" => []}]}).at_least(:once)
       expect(Hosting::GcpApis).to receive(:new).and_return(api)
       vm2 = instance_double(described_class, name: "vm2", address_name: "vm2-addr", location: "us-central1", sshable: instance_double(Sshable, host: "ip2"))
       expect(gcp_vm).to receive(:sshable).and_return(instance_double(Sshable, host: "ip1")).at_least(:once)
@@ -97,7 +115,7 @@
       expect(api).to receive(:delete_ephermal_ipv4).with("vm2", "us-central1-a")
       expect(api).to receive(:assign_static_ipv4).with("vm1", "ip2", "us-central1-a")
       expect(api).to receive(:assign_static_ipv4).with("vm2", "ip1", "us-central1-a")
-      expect(api).to receive(:get_vm).and_return({"networkInterfaces" => [{"accessConfigs" => []}]}).at_least(:once)
+      expect(api).to receive(:get_vm).and_return({"networkInterfaces" => [{"accessConfigs" => [{"name" => "External NAT", "natIP" => "ip1"}]}]}, {"networkInterfaces" => [{"accessConfigs" => [{"name" => "External NAT", "natIP" => "ip2"}]}]})
       expect(Hosting::GcpApis).to receive(:new).and_return(api)
       vm2 = instance_double(described_class, name: "vm2", address_name: "vm2-addr", location: "us-central1", sshable: instance_double(Sshable, host: "ip2"))
       expect(gcp_vm).to receive(:sshable).and_return(instance_double(Sshable, host: "ip1")).at_least(:once)