make swap_ips function failure resistant

lib/hosting/gcp_apis.rb

@@ -170,6 +170,9 @@ 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"])
@@ -276,7 +279,7 @@ def create_service_account(name, description = "")
       }
     }
 
-    response = connection.post(path: "/v1/projects/#{@project}/serviceAccounts", body: JSON.dump(body), expects: [200, 400, 403])
+    response = connection.post(path: "/v1/projects/#{@project}/serviceAccounts", body: JSON.dump(body), expects: [200, 400, 403, 409])
 
     Hosting::GcpApis.check_errors(response)
 

model/gcp_vm.rb

@@ -74,8 +74,17 @@ def swap_ip(vm)
     zone2 = "#{vm.location}-a"
     gcp_client.delete_ephermal_ipv4(name, zone1)
     gcp_client.delete_ephermal_ipv4(vm.name, zone2)
-    gcp_client.assign_static_ipv4(name, vm.sshable.host, zone1)
-    gcp_client.assign_static_ipv4(vm.name, sshable.host, zone2)
+    vm_info = gcp_client.get_vm(name, zone1)
+    # 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 }
+      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 }
+      gcp_client.assign_static_ipv4(vm.name, sshable.host, zone2)
+    end
 
     # update sshable hosts
     current_host = sshable.host

spec/lib/hosting/gcp_apis_spec.rb

@@ -101,6 +101,13 @@
     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

@@ -73,12 +73,31 @@
   end
 
   describe "#swap_ip" do
+    it "does not assign ip if already assigned" do
+      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(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 "swap server ips" do
       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(: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)