From ebed6ed2f1b5aeb1b3b85e6c403625d613d8acf6 Mon Sep 17 00:00:00 2001 From: Bob van den Heuvel Date: Wed, 17 May 2017 17:31:00 +0200 Subject: [PATCH 1/4] Place the class in a namespace --- .../action/read_transport_info.rb | 66 +++++++------ .../action/retrieve_public_ip_port_spec.rb | 95 +++++++++++++++++++ 2 files changed, 131 insertions(+), 30 deletions(-) create mode 100644 spec/vagrant-cloudstack/action/retrieve_public_ip_port_spec.rb diff --git a/lib/vagrant-cloudstack/action/read_transport_info.rb b/lib/vagrant-cloudstack/action/read_transport_info.rb index 9cd36c27..73b8120d 100644 --- a/lib/vagrant-cloudstack/action/read_transport_info.rb +++ b/lib/vagrant-cloudstack/action/read_transport_info.rb @@ -1,38 +1,44 @@ -class ReadTransportInfo - def initialize - @public_port_fieldname = 'pf_public_port' - end +module VagrantPlugins + module Cloudstack + module Action + class ReadTransportInfo + def initialize + @public_port_fieldname = 'pf_public_port' + end - def retrieve_public_ip_port(cloudstack, domain_config, machine) - pf_ip_address_id = domain_config.pf_ip_address_id - pf_ip_address = domain_config.pf_ip_address - pf_public_port = domain_config.send(@public_port_fieldname) + def retrieve_public_ip_port(cloudstack, domain_config, machine) + pf_ip_address_id = domain_config.pf_ip_address_id + pf_ip_address = domain_config.pf_ip_address + pf_public_port = domain_config.send(@public_port_fieldname) - if pf_public_port.nil? - pf_public_port_file = machine.data_dir.join(@public_port_fieldname) - if pf_public_port_file.file? - File.read(pf_public_port_file).each_line do |line| - pf_public_port = line.strip - end - domain_config.send("#{@public_port_fieldname}=", pf_public_port) - end - end + if pf_public_port.nil? + pf_public_port_file = machine.data_dir.join(@public_port_fieldname) + if pf_public_port_file.file? + File.read(pf_public_port_file).each_line do |line| + pf_public_port = line.strip + end + domain_config.send("#{@public_port_fieldname}=", pf_public_port) + end + end - if not pf_ip_address and pf_ip_address_id and pf_public_port - begin - response = cloudstack.list_public_ip_addresses({:id => pf_ip_address_id}) - rescue Fog::Compute::Cloudstack::Error => e - raise Errors::FogError, :message => e.message - end + if not pf_ip_address and pf_ip_address_id and pf_public_port + begin + response = cloudstack.list_public_ip_addresses({:id => pf_ip_address_id}) + rescue Fog::Compute::Cloudstack::Error => e + raise Errors::FogError, :message => e.message + end - if (response['listpublicipaddressesresponse']['count']).zero? - @logger.info("IP address #{pf_ip_address_id} not exists.") - env[:ui].info(I18n.t("IP address #{pf_ip_address_id} not exists.")) - pf_ip_address = nil - else - pf_ip_address = response['listpublicipaddressesresponse']['publicipaddress'][0]['ipaddress'] + if (response['listpublicipaddressesresponse']['count']).zero? + @logger.info("IP address #{pf_ip_address_id} not exists.") + env[:ui].info(I18n.t("IP address #{pf_ip_address_id} not exists.")) + pf_ip_address = nil + else + pf_ip_address = response['listpublicipaddressesresponse']['publicipaddress'][0]['ipaddress'] + end + end + return pf_ip_address, pf_public_port + end end end - return pf_ip_address, pf_public_port end end diff --git a/spec/vagrant-cloudstack/action/retrieve_public_ip_port_spec.rb b/spec/vagrant-cloudstack/action/retrieve_public_ip_port_spec.rb new file mode 100644 index 00000000..d3052716 --- /dev/null +++ b/spec/vagrant-cloudstack/action/retrieve_public_ip_port_spec.rb @@ -0,0 +1,95 @@ +require 'spec_helper' +require 'vagrant-cloudstack/action/read_ssh_info' +require 'vagrant-cloudstack/config' + +describe VagrantPlugins::Cloudstack::Action::ReadSSHInfo do + let(:action) {VagrantPlugins::Cloudstack::Action::ReadSSHInfo.new(nil, nil)} + + describe '#fetch_nic_ip_address' do + subject {action.fetch_nic_ip_address(nics, domain_config)} + + let(:nics) do + [ + {'networkid' => 'networkid1', 'networkname' => 'networkname1', 'ipaddress' => '127.0.0.1'}, + {'networkid' => 'networkid2', 'networkname' => 'networkname2', 'ipaddress' => '127.0.0.2'}, + {'networkid' => 'networkid3', 'networkname' => 'networkname3', 'ipaddress' => '127.0.0.3'}, + ] + end + + let(:ssh_network_id) {Vagrant::Plugin::V2::Config::UNSET_VALUE} + let(:ssh_network_name) {Vagrant::Plugin::V2::Config::UNSET_VALUE} + + let(:domain_config) do + config = VagrantPlugins::Cloudstack::Config.new + config.domain_config :cloudstack do |cloudstack| + cloudstack.ssh_network_id = ssh_network_id + cloudstack.ssh_network_name = ssh_network_name + end + config.finalize! + config.get_domain_config(:cloudstack) + end + + context 'without neither ssh_network_id and ssh_network_name' do + it {should eq '127.0.0.1' + } + end + + context 'with ssh_network_id' do + context 'when exists in nics' do + let(:ssh_network_id) {'networkid2' + } + + it {should eq '127.0.0.2' + } + end + + context 'when not exists in nics' do + let(:ssh_network_id) {'unknown' + } + + it {should eq '127.0.0.1' + } + end + end + + context 'with ssh_network_id' do + context 'when exists in nics' do + let(:ssh_network_name) {'networkname3' + } + + it {should eq '127.0.0.3' + } + end + + context 'when not exists in nics' do + let(:ssh_network_name) {'unknown' + } + + it {should eq '127.0.0.1' + } + end + end + + context 'with both ssh_network_id and ssh_network_name' do + context 'when exists in nics' do + let(:ssh_network_id) {'networkid2' + } + let(:ssh_network_name) {'networkname3' + } + + it {should eq '127.0.0.2' + } + end + + context 'when not exists in nics' do + let(:ssh_network_id) {'unknown' + } + let(:ssh_network_name) {'unknown' + } + + it {should eq '127.0.0.1' + } + end + end + end +end From b84dd0ff61bcc411c72dabfa588493564cb016f0 Mon Sep 17 00:00:00 2001 From: Bob van den Heuvel Date: Wed, 17 May 2017 17:32:20 +0200 Subject: [PATCH 2/4] Add unit test for retrieve_public_ip_port_spec --- .../action/retrieve_public_ip_port_spec.rb | 133 +++++++++--------- 1 file changed, 66 insertions(+), 67 deletions(-) diff --git a/spec/vagrant-cloudstack/action/retrieve_public_ip_port_spec.rb b/spec/vagrant-cloudstack/action/retrieve_public_ip_port_spec.rb index d3052716..f294aa09 100644 --- a/spec/vagrant-cloudstack/action/retrieve_public_ip_port_spec.rb +++ b/spec/vagrant-cloudstack/action/retrieve_public_ip_port_spec.rb @@ -1,94 +1,93 @@ require 'spec_helper' -require 'vagrant-cloudstack/action/read_ssh_info' +require 'vagrant-cloudstack/action/read_transport_info' require 'vagrant-cloudstack/config' +require 'fog' -describe VagrantPlugins::Cloudstack::Action::ReadSSHInfo do - let(:action) {VagrantPlugins::Cloudstack::Action::ReadSSHInfo.new(nil, nil)} +describe VagrantPlugins::Cloudstack::Action::ReadTransportInfo do + let(:action) {VagrantPlugins::Cloudstack::Action::ReadTransportInfo.new } - describe '#fetch_nic_ip_address' do - subject {action.fetch_nic_ip_address(nics, domain_config)} + describe '#retrieve_public_ip_port' do + subject { action.retrieve_public_ip_port(cloudstack_compute, domain_config, machine) } - let(:nics) do - [ - {'networkid' => 'networkid1', 'networkname' => 'networkname1', 'ipaddress' => '127.0.0.1'}, - {'networkid' => 'networkid2', 'networkname' => 'networkname2', 'ipaddress' => '127.0.0.2'}, - {'networkid' => 'networkid3', 'networkname' => 'networkname3', 'ipaddress' => '127.0.0.3'}, - ] - end + let(:cloudstack_compute) { double('Fog::Compute::Cloudstack') } + let(:machine) { double('Vagrant::Machine')} + + let(:data_dir) { double('Pathname') } + let(:pf_public_port_file) { double('Pathname') } - let(:ssh_network_id) {Vagrant::Plugin::V2::Config::UNSET_VALUE} - let(:ssh_network_name) {Vagrant::Plugin::V2::Config::UNSET_VALUE} + let(:pf_ip_address) { 'ip_address_in_config' } + let(:pf_ip_address_from_server) { 'ip_address_from_server' } + let(:pf_ip_address_id) { 'ID of ip_address_in_config' } + let(:pf_public_port) { 'public_port_in_config' } + let(:pf_public_port_from_file) { 'public_port_from_file' } let(:domain_config) do config = VagrantPlugins::Cloudstack::Config.new - config.domain_config :cloudstack do |cloudstack| - cloudstack.ssh_network_id = ssh_network_id - cloudstack.ssh_network_name = ssh_network_name + config.domain_config :cloudstack do |cfg| + cfg.pf_ip_address = pf_ip_address + cfg.pf_public_port = pf_public_port + cfg.pf_ip_address_id = pf_ip_address_id end config.finalize! config.get_domain_config(:cloudstack) end - context 'without neither ssh_network_id and ssh_network_name' do - it {should eq '127.0.0.1' - } - end - - context 'with ssh_network_id' do - context 'when exists in nics' do - let(:ssh_network_id) {'networkid2' - } - - it {should eq '127.0.0.2' - } - end - - context 'when not exists in nics' do - let(:ssh_network_id) {'unknown' - } - - it {should eq '127.0.0.1' - } + context 'without both ip address and port in config' do + it 'retrieves those configured values' do + should eq [pf_ip_address, pf_public_port] end end - context 'with ssh_network_id' do - context 'when exists in nics' do - let(:ssh_network_name) {'networkname3' - } - - it {should eq '127.0.0.3' - } - end + context 'port not configured' do + let(:pf_public_port) { nil } - context 'when not exists in nics' do - let(:ssh_network_name) {'unknown' - } + it 'retrieves the active port stored on filesystem' do + expect(machine).to receive(:data_dir).and_return(data_dir) + expect(data_dir).to receive(:join).and_return(pf_public_port_file) + expect(pf_public_port_file).to receive(:file?).and_return(true) + expect(File).to receive(:read).and_return(pf_public_port_from_file) - it {should eq '127.0.0.1' - } + expect(subject).to eq [pf_ip_address, pf_public_port_from_file] end end - context 'with both ssh_network_id and ssh_network_name' do - context 'when exists in nics' do - let(:ssh_network_id) {'networkid2' - } - let(:ssh_network_name) {'networkname3' + context 'only ID of ip address specified (and public port)' do + let(:pf_ip_address) { nil } + + it 'resolves, and returns, the ip address from the ID' do + response = { + 'listpublicipaddressesresponse' => { + 'count' =>1, + 'publicipaddress' =>[{ + 'id' => pf_ip_address_id, + 'ipaddress' => pf_ip_address_from_server, + 'allocated' => '2016-05-06T13:58:04+0200', + 'zoneid' => 'UUID', + 'zonename' => 'Name', + 'issourcenat' =>false, + 'account' => 'Name', + 'domainid' => 'UUID', + 'domain' => 'Name', + 'forvirtualnetwork' =>true, + 'isstaticnat' =>false, + 'issystem' =>false, + 'associatednetworkid' => 'UUID', + 'associatednetworkname' => 'Name', + 'networkid' => 'UUID', + 'aclid' => 'UUID', + 'state' => 'Allocated', + 'physicalnetworkid' => 'UUID', + 'vpcid' => 'UUID', + 'tags' =>[], + 'isportable' =>false + }] + } } + expect(cloudstack_compute).to receive(:list_public_ip_addresses) + .with(:id => pf_ip_address_id) + .and_return(response) - it {should eq '127.0.0.2' - } - end - - context 'when not exists in nics' do - let(:ssh_network_id) {'unknown' - } - let(:ssh_network_name) {'unknown' - } - - it {should eq '127.0.0.1' - } + expect(subject).to eq [pf_ip_address_from_server, pf_public_port] end end end From 1cb2c83095d9050daa225b2953993e79bbe8b3a2 Mon Sep 17 00:00:00 2001 From: Bob van den Heuvel Date: Thu, 18 May 2017 20:18:11 +0200 Subject: [PATCH 3/4] Add unit test for run_instance This covers a basic VM deployment --- lib/vagrant-cloudstack/action/run_instance.rb | 10 +- .../action/run_instance_spec.rb | 170 ++++++++++++++++++ 2 files changed, 175 insertions(+), 5 deletions(-) create mode 100644 spec/vagrant-cloudstack/action/run_instance_spec.rb diff --git a/lib/vagrant-cloudstack/action/run_instance.rb b/lib/vagrant-cloudstack/action/run_instance.rb index 696e5aa5..40990475 100644 --- a/lib/vagrant-cloudstack/action/run_instance.rb +++ b/lib/vagrant-cloudstack/action/run_instance.rb @@ -178,11 +178,11 @@ def sanitize_domain_config def configure_networking enable_static_nat_rules - evaluate_pf_private_port - evaluate_pf_private_rdp_port - - - create_port_forwardings + unless @pf_ip_address.is_undefined? + evaluate_pf_private_port + evaluate_pf_private_rdp_port + create_port_forwardings + end # First create_port_forwardings, # as it may generate 'pf_public_port' or 'pf_public_rdp_port', # which after this may need a firewall rule diff --git a/spec/vagrant-cloudstack/action/run_instance_spec.rb b/spec/vagrant-cloudstack/action/run_instance_spec.rb new file mode 100644 index 00000000..66e6898b --- /dev/null +++ b/spec/vagrant-cloudstack/action/run_instance_spec.rb @@ -0,0 +1,170 @@ +require 'spec_helper' +require 'vagrant-cloudstack/action/run_instance' +require 'vagrant-cloudstack/config' +require 'vagrant-cloudstack/provider' + +require 'vagrant' +require 'fog' + + +ZONE_NAME = 'Zone Name' +ZONE_ID = 'Zone UUID' +SERVICE_OFFERING_NAME = 'Service Offering Name' +SERVICE_OFFERING_ID = 'Service Offering UUID' +TEMPLATE_NAME = 'Template Name' +TEMPLATE_ID = 'Template UUID' +NETWORK_NAME = 'Network Name' +NETWORK_ID = 'Network UUID' +DISPLAY_NAME = 'Display Name' + +SERVER_ID = 'Server UUID' +NETWORK_TYPE = 'Advanced' +SECURITY_GROUPS_ENABLED = false + + +describe VagrantPlugins::Cloudstack::Action::RunInstance do + let(:action) { VagrantPlugins::Cloudstack::Action::RunInstance.new(app, env) } + + + describe '#run_instance in advanced zone' do + subject { action.call(env) } + let(:app) { double('Vagrant::Action::Warden')} + + let(:provider_config) do + config = VagrantPlugins::Cloudstack::Config.new + config.domain_config :cloudstack do |cfg| + cfg.zone_name = ZONE_NAME + cfg.network_name = NETWORK_NAME + cfg.service_offering_name = SERVICE_OFFERING_NAME + cfg.template_name = TEMPLATE_NAME + cfg.ssh_key = '/path/to/ssh/key/file' + cfg.display_name = DISPLAY_NAME + end + config.finalize! + config.get_domain_config(:cloudstack) + end + + let(:machine) { double('Vagrant::Machine') } + + let(:cloudstack_zone) { + instance_double('Fog::Compute::Cloudstack::Zone', + id: ZONE_ID, + name: ZONE_NAME, + network_type: NETWORK_TYPE, + security_groups_enabled: SECURITY_GROUPS_ENABLED) + } + let(:cloudstack_compute) { double('Fog::Compute::Cloudstack') } + let(:servers) { double('Fog::Compute::Cloudstack::Servers') } + let(:server) { double('Fog::Compute::Cloudstack::Server') } + let(:ui) { double('Vagrant::UI::Prefixed') } + let(:root_path) { double('Pathname') } + let(:env) do + { + root_path: root_path, + ui: ui, + machine: machine, + cloudstack_compute: cloudstack_compute + } + end + + before(:each) do + allow(app).to receive(:call).and_return(true) + + allow(ui).to receive(:info) + allow(ui).to receive(:warn) + allow(ui).to receive(:detail) + + allow(machine).to receive(:provider_config).and_return(provider_config) + allow(machine).to receive(:id=).with(SERVER_ID) + allow(machine).to receive(:communicate).and_return('') + allow(machine).to receive_message_chain(:communicate, :ready?).and_return(true) + allow(machine).to receive_message_chain(:config, :vm, :box).and_return(TEMPLATE_NAME) + + allow(cloudstack_compute).to receive(:servers).and_return(servers) + allow(cloudstack_compute).to receive(:volumes).and_return([]) + + allow(server).to receive(:id).and_return(SERVER_ID) + allow(server).to receive(:wait_for).and_return(ready = true) + allow(server).to receive(:password_enabled).and_return(false) + + list_zones_response = { + "listzonesresponse"=>{ + "count"=>1, + "zone"=>[ + { + "tags"=>[], + "id"=>ZONE_ID, + "name"=>ZONE_NAME, + "networktype"=>NETWORK_TYPE, + "securitygroupsenabled"=>SECURITY_GROUPS_ENABLED + } + ] + } + } + allow(cloudstack_compute).to receive(:send).with(:list_zones, :available => true ).and_return(list_zones_response) + + list_service_offerings_response = { + "listserviceofferingsresponse"=>{ + "count"=>1, + "serviceoffering"=>[ + { + "id"=>SERVICE_OFFERING_ID, + "name"=>SERVICE_OFFERING_NAME, + "displaytext"=>"Display version of #{SERVICE_OFFERING_NAME}", + } + ] + } + } + allow(cloudstack_compute).to receive(:send).with(:list_service_offerings, listall: true) + .and_return(list_service_offerings_response) + + list_templates_response = { + "listtemplatesresponse"=>{ + "count"=>1, + "template"=>[ + { + "id"=>TEMPLATE_ID, + "name"=>TEMPLATE_NAME + } + ] + } + } + allow(cloudstack_compute).to receive(:send) + .with(:list_templates, zoneid: ZONE_ID, templatefilter: 'executable', listall: true) + .and_return(list_templates_response) + + allow(cloudstack_compute).to receive(:zones).and_return([cloudstack_zone]) + + list_networks = { + "listnetworksresponse"=>{ + "count"=>1, + "network"=>[ + { + "id"=>NETWORK_ID, + "name"=>NETWORK_NAME, + } + ] + } + } + allow(cloudstack_compute).to receive(:send) + .with(:list_networks, {}) + .and_return(list_networks) + + create_servers_parameters = { + :display_name=>DISPLAY_NAME, + :group=>nil, + :zone_id=>ZONE_ID, + :flavor_id=>SERVICE_OFFERING_ID, + :image_id=>TEMPLATE_ID, + "network_ids"=>NETWORK_ID + } + allow(servers).to receive(:create).with(create_servers_parameters).and_return(server) + end + + context 'start a simple VM' do + it 'starts a vm' do + should eq true + end + end + end +end From 09b57dc1b0cd4374801eafdea1291c2afceaa518 Mon Sep 17 00:00:00 2001 From: Bob van den Heuvel Date: Thu, 18 May 2017 21:30:16 +0200 Subject: [PATCH 4/4] Correct urls of badges, add "Better Code" --- README.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index 00b13240..8f5e790b 100644 --- a/README.md +++ b/README.md @@ -1,10 +1,10 @@ # Vagrant Cloudstack Provider -[![Build Status](https://travis-ci.org/MissionCriticalCloud/vagrant-cloudstack.png?branch=master)](https://travis-ci.org/missioncriticalcloud/vagrant-cloudstack) +[![Build Status](https://travis-ci.org/MissionCriticalCloud/vagrant-cloudstack.png?branch=master)](https://travis-ci.org/MissionCriticalCloud/vagrant-cloudstack) [![Gem Version](https://badge.fury.io/rb/vagrant-cloudstack.png)](http://badge.fury.io/rb/vagrant-cloudstack) -[![Code climate](https://codeclimate.com/github/MissionCriticalCloud/vagrant-cloudstack.png)](https://codeclimate.com/github/missioncriticalcloud/vagrant-cloudstack) +[![Code climate](https://codeclimate.com/github/MissionCriticalCloud/vagrant-cloudstack.png)](https://codeclimate.com/github/MissionCriticalCloud/vagrant-cloudstack) [![Coverage Status](https://coveralls.io/repos/github/MissionCriticalCloud/vagrant-cloudstack/badge.svg?branch=master)](https://coveralls.io/github/MissionCriticalCloud/vagrant-cloudstack?branch=master) -[![Gem Version](https://badge.fury.io/rb/vagrant-cloudstack.svg)](http://badge.fury.io/rb/vagrant-cloudstack) +[![BCH compliance](https://bettercodehub.com/edge/badge/MissionCriticalCloud/vagrant-cloudstack?branch=master)](https://bettercodehub.com/) This is a fork of [mitchellh AWS Provider](https://github.com/mitchellh/vagrant-aws/).