From cfd99cdb91ca93040e55ff21d74901adb9aab335 Mon Sep 17 00:00:00 2001 From: Varik Matevosyan Date: Thu, 27 Jun 2024 14:22:47 +0400 Subject: [PATCH] make swap_ips function failure resistant --- lib/hosting/gcp_apis.rb | 5 ++++- model/gcp_vm.rb | 13 +++++++++++-- spec/lib/hosting/gcp_apis_spec.rb | 7 +++++++ spec/model/gcp_vm_spec.rb | 19 +++++++++++++++++++ 4 files changed, 41 insertions(+), 3 deletions(-) diff --git a/lib/hosting/gcp_apis.rb b/lib/hosting/gcp_apis.rb index ae177500d..59415b3f6 100644 --- a/lib/hosting/gcp_apis.rb +++ b/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) diff --git a/model/gcp_vm.rb b/model/gcp_vm.rb index df53a3e98..37606e5fe 100644 --- a/model/gcp_vm.rb +++ b/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 diff --git a/spec/lib/hosting/gcp_apis_spec.rb b/spec/lib/hosting/gcp_apis_spec.rb index dd54f8cec..8a9f4a0de 100644 --- a/spec/lib/hosting/gcp_apis_spec.rb +++ b/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"}) diff --git a/spec/model/gcp_vm_spec.rb b/spec/model/gcp_vm_spec.rb index 3acfd0584..2bec8ef4b 100644 --- a/spec/model/gcp_vm_spec.rb +++ b/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)