diff --git a/lib/hosting/gcp_apis.rb b/lib/hosting/gcp_apis.rb index 59415b3f6..9588f2ed9 100644 --- a/lib/hosting/gcp_apis.rb +++ b/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"]) diff --git a/model/gcp_vm.rb b/model/gcp_vm.rb index 37606e5fe..470bf298d 100644 --- a/model/gcp_vm.rb +++ b/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 diff --git a/spec/lib/hosting/gcp_apis_spec.rb b/spec/lib/hosting/gcp_apis_spec.rb index 8a9f4a0de..dd54f8cec 100644 --- a/spec/lib/hosting/gcp_apis_spec.rb +++ b/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"}) diff --git a/spec/model/gcp_vm_spec.rb b/spec/model/gcp_vm_spec.rb index 2bec8ef4b..f848b058a 100644 --- a/spec/model/gcp_vm_spec.rb +++ b/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)