From daa55fe5b8b72686c01aebe2e5453e188bdf41de Mon Sep 17 00:00:00 2001 From: Samuel Beaulieu Date: Wed, 29 Dec 2021 08:13:32 -0600 Subject: [PATCH 1/7] Adding the cloud DNS API library and related methods we setup DNS when a VM is created and tear it down when a VM is deleted the DNS zone should exist already and is referenced by a provider setting the dns zone is also set in order to use it for vm_ready? instead of the global domain instances have a label that identifies which project they belong to, so it can be used for FW rules --- .gitignore | 1 + README.md | 11 ++++ lib/vmpooler/providers/gce.rb | 60 ++++++++++++++++--- scripts/GCE_custom_role_for_SA.yaml | 38 ++++++++++++ scripts/create_custom_role | 5 ++ spec/unit/providers/gce_spec.rb | 47 +++++++++++++-- .../vmpooler_provider_gce_spec.rb | 9 +++ vmpooler-provider-gce.gemspec | 1 + vmpooler.yaml.example | 7 +++ 9 files changed, 167 insertions(+), 12 deletions(-) create mode 100644 scripts/GCE_custom_role_for_SA.yaml create mode 100644 scripts/create_custom_role create mode 100644 spec/vmpooler-provider-gce/vmpooler_provider_gce_spec.rb diff --git a/.gitignore b/.gitignore index 9b97fbf..95e94de 100644 --- a/.gitignore +++ b/.gitignore @@ -8,3 +8,4 @@ Gemfile.local results.xml /vmpooler.yaml .idea +*.json diff --git a/README.md b/README.md index d401c83..ec761b7 100644 --- a/README.md +++ b/README.md @@ -14,6 +14,10 @@ GCE authorization is handled via a service account (or personal account) private 1. GOOGLE_APPLICATION_CREDENTIALS environment variable eg GOOGLE_APPLICATION_CREDENTIALS=/my/home/directory/my_account_key.json +### DNS +DNS is integrated via Google's CloudDNS service. To enable a CloudDNS zone name must be provided in the config (see the example yaml file dns_zone_resource_name) + +An A record is then created in that zone upon instance creation with the VM's internal IP, and deleted when the instance is destroyed. ### Labels This provider adds labels to all resources that are managed @@ -27,6 +31,13 @@ This provider adds labels to all resources that are managed Also see the usage of vmpooler's optional purge_unconfigured_resources, which is used to delete any resource found that do not have the pool label, and can be configured to allow a specific list of unconfigured pool names. +### Pre-requisite + +- A service account needs to be created and a private json key generated (see usage section) +- The service account needs given permissions to the project (broad permissions would be compute v1 admin and dns admin). A yaml file is provided that lists the least-privilege permissions needed +- if using DNS, a DNS zone needs to be created + + ## License vmpooler-provider-gce is distributed under the [Apache License, Version 2.0](http://www.apache.org/licenses/LICENSE-2.0.html). See the [LICENSE](LICENSE) file for more details. \ No newline at end of file diff --git a/lib/vmpooler/providers/gce.rb b/lib/vmpooler/providers/gce.rb index b4f5f8e..d929a85 100644 --- a/lib/vmpooler/providers/gce.rb +++ b/lib/vmpooler/providers/gce.rb @@ -2,6 +2,7 @@ require 'googleauth' require 'google/apis/compute_v1' +require "google/cloud/dns" require 'bigdecimal' require 'bigdecimal/util' require 'vmpooler/providers/base' @@ -44,6 +45,7 @@ module Vmpooler { connection: new_conn } end @redis = redis_connection_pool + @dns = Google::Cloud::Dns.new(project_id: project) end # name of the provider class @@ -66,6 +68,10 @@ module Vmpooler provider_config['network_name'] end + def subnetwork_name(pool_name) + return pool_config(pool_name)['subnetwork_name'] if pool_config(pool_name)['subnetwork_name'] + end + # main configuration options, overridable for each pool def zone(pool_name) return pool_config(pool_name)['zone'] if pool_config(pool_name)['zone'] @@ -77,6 +83,14 @@ module Vmpooler return provider_config['machine_type'] if provider_config['machine_type'] end + def dns_zone + provider_config['dns_zone'] + end + + def dns_zone_resource_name + provider_config['dns_zone_resource_name'] + end + # Base methods that are implemented: # vms_in_pool lists all the VM names in a pool, which is based on the VMs @@ -162,6 +176,7 @@ module Vmpooler network_interfaces = Google::Apis::ComputeV1::NetworkInterface.new( network: network_name ) + network_interfaces.subnetwork=subnetwork_name(pool_name) if subnetwork_name(pool_name) init_params = { source_image: pool['template'], # The source image to create this disk. labels: { 'vm' => new_vmname, 'pool' => pool_name }, @@ -172,19 +187,23 @@ module Vmpooler boot: true, initialize_params: Google::Apis::ComputeV1::AttachedDiskInitializeParams.new(init_params) ) - # Assume all pool config is valid i.e. not missing client = ::Google::Apis::ComputeV1::Instance.new( name: new_vmname, machine_type: pool['machine_type'], disks: [disk], network_interfaces: [network_interfaces], - labels: { 'vm' => new_vmname, 'pool' => pool_name } + labels: { 'vm' => new_vmname, 'pool' => pool_name, project => nil } ) +=begin TODO: Maybe this will be needed to set the hostname (usually internal DNS name but in opur case for some reason its nil) + given_hostname = "#{new_vmname}.#{dns_zone}" + client.hostname = given_hostname if given_hostname +=end debug_logger('trigger insert_instance') result = connection.insert_instance(project, zone(pool_name), client) wait_for_operation(project, pool_name, result) - get_vm(pool_name, new_vmname) + created_instance = get_vm(pool_name, new_vmname) + dns_setup(created_instance) end # create_disk creates an additional disk for an existing VM. It will name the new @@ -398,8 +417,10 @@ module Vmpooler unless deleted debug_logger("trigger delete_instance #{vm_name}") + vm_hash = get_vm(pool_name, vm_name) result = connection.delete_instance(project, zone(pool_name), vm_name) wait_for_operation(project, pool_name, result, 10) + dns_teardown(vm_hash) end # list and delete any leftover disk, for instance if they were detached from the instance @@ -437,7 +458,7 @@ module Vmpooler def vm_ready?(_pool_name, vm_name) begin # TODO: we could use a healthcheck resource attached to instance - open_socket(vm_name, global_config[:config]['domain']) + open_socket(vm_name, dns_zone || global_config[:config]['domain']) rescue StandardError => _e return false end @@ -469,6 +490,9 @@ module Vmpooler debug_logger("trigger async delete_instance #{vm.name}") result = connection.delete_instance(project, zone, vm.name) + vm_pool = vm.labels&.key?('pool') ? vm.labels['pool'] : nil + existing_vm = generate_vm_hash(vm, vm_pool) + dns_teardown(existing_vm) result_list << result end # now check they are done @@ -529,6 +553,27 @@ module Vmpooler # END BASE METHODS + def dns_setup(created_instance) + zone = @dns.zone dns_zone_resource_name if dns_zone_resource_name + if zone && created_instance + name = created_instance['name'] + change = zone.add name, "A", 60, [created_instance['ip']] + debug_logger("#{change.id} - #{change.started_at} - #{change.status}") if change + end + # TODO: should we catch Google::Cloud::AlreadyExistsError that is thrown when it already exist? + # and then delete and recreate? + # eg the error is Google::Cloud::AlreadyExistsError: alreadyExists: The resource 'entity.change.additions[0]' named 'instance-8.test.vmpooler.puppet.net. (A)' already exists + end + + def dns_teardown(created_instance) + zone = @dns.zone dns_zone_resource_name if dns_zone_resource_name + if zone && created_instance + name = created_instance['name'] + change = zone.remove name, "A" + debug_logger("#{change.id} - #{change.started_at} - #{change.status}") if change + end + end + def should_be_ignored(item, allowlist) return false if allowlist.nil? @@ -565,7 +610,7 @@ module Vmpooler if result.error # unsure what kind of error can be stored here error_message = '' # array of errors, combine them all - result.error.each do |error| + result.error.errors.each do |error| error_message = "#{error_message} #{error.code}:#{error.message}" end raise "Operation: #{result.description} failed with error: #{error_message}" @@ -606,7 +651,8 @@ module Vmpooler 'zone' => vm_object.zone, 'machine_type' => vm_object.machine_type, 'labels' => vm_object.labels, - 'label_fingerprint' => vm_object.label_fingerprint + 'label_fingerprint' => vm_object.label_fingerprint, + 'ip' => vm_object.network_interfaces.first.network_ip # 'powerstate' => powerstate } end @@ -682,6 +728,6 @@ module Vmpooler logger.log('[g]', message) if send_to_upstream end end - end + end end end diff --git a/scripts/GCE_custom_role_for_SA.yaml b/scripts/GCE_custom_role_for_SA.yaml new file mode 100644 index 0000000..696f455 --- /dev/null +++ b/scripts/GCE_custom_role_for_SA.yaml @@ -0,0 +1,38 @@ +title: Custom vmpooler provider +description: for the vmpooler provider +stage: GA +includedPermissions: +- compute.disks.create +- compute.disks.createSnapshot +- compute.disks.delete +- compute.disks.get +- compute.disks.list +- compute.disks.setLabels +- compute.disks.use +- compute.instances.attachDisk +- compute.instances.create +- compute.instances.delete +- compute.instances.detachDisk +- compute.instances.get +- compute.instances.list +- compute.instances.setLabels +- compute.instances.start +- compute.instances.stop +- compute.snapshots.create +- compute.snapshots.delete +- compute.snapshots.get +- compute.snapshots.list +- compute.snapshots.setLabels +- compute.snapshots.useReadOnly +- compute.subnetworks.use +- compute.zoneOperations.get +- dns.changes.create +- dns.changes.get +- dns.changes.list +- dns.managedZones.get +- dns.managedZones.list +- dns.resourceRecordSets.create +- dns.resourceRecordSets.update +- dns.resourceRecordSets.delete +- dns.resourceRecordSets.get +- dns.resourceRecordSets.list diff --git a/scripts/create_custom_role b/scripts/create_custom_role new file mode 100644 index 0000000..56d905e --- /dev/null +++ b/scripts/create_custom_role @@ -0,0 +1,5 @@ +#!/bin/bash +#set your project name in GCE here +project_id="vmpooler-test" +#this creates a custom role, that should then be applied to a service account used to run the API requests. +gcloud iam roles create Customvmpoolerprovider --project=$project-id --file=GCE_custom_role_for_SA.yaml \ No newline at end of file diff --git a/spec/unit/providers/gce_spec.rb b/spec/unit/providers/gce_spec.rb index 8fc07a0..bf69457 100644 --- a/spec/unit/providers/gce_spec.rb +++ b/spec/unit/providers/gce_spec.rb @@ -11,6 +11,7 @@ describe 'Vmpooler::PoolManager::Provider::Gce' do let(:metrics) { Vmpooler::Metrics::DummyStatsd.new } let(:poolname) { 'debian-9' } let(:provider_options) { { 'param' => 'value' } } + # let(:project) { 'vmpooler-test' } let(:project) { 'dio-samuel-dev' } let(:zone) { 'us-west1-b' } let(:config) { YAML.load(<<-EOT @@ -23,7 +24,10 @@ describe 'Vmpooler::PoolManager::Provider::Gce' do connection_pool_timeout: 1 project: '#{project}' zone: '#{zone}' - network_name: 'global/networks/default' + network_name: global/networks/default + # network_name: 'projects/itsysopsnetworking/global/networks/shared1' + dns_zone_resource_name: 'example-com' + dns_zone: 'example.com' :pools: - name: '#{poolname}' alias: [ 'mockpool' ] @@ -32,13 +36,13 @@ describe 'Vmpooler::PoolManager::Provider::Gce' do timeout: 10 ready_ttl: 1440 provider: 'gce' - network_name: 'default' + # subnetwork_name: 'projects/itsysopsnetworking/regions/us-west1/subnetworks/vmpooler-test' machine_type: 'zones/#{zone}/machineTypes/e2-micro' EOT ) } - let(:vmname) { 'vm16' } + let(:vmname) { 'vm17' } let(:connection) { MockComputeServiceConnection.new } let(:redis_connection_pool) do Vmpooler::PoolManager::GenericConnectionPool.new( @@ -80,11 +84,44 @@ EOT # result = subject.create_snapshot(poolname, vmname, "sams") # result = subject.revert_snapshot(poolname, vmname, "sams") # puts subject.get_vm(poolname, vmname) + result = subject.create_vm(poolname, vmname) result = subject.destroy_vm(poolname, vmname) end - skip 'debug' do - puts subject.purge_unconfigured_resources(['foo', '', 'blah']) + context 'in itsysops' do + let(:vmname) { "instance-10" } + let(:project) { 'vmpooler-test' } + let(:config) { YAML.load(<<-EOT +--- +:config: + max_tries: 3 + retry_factor: 10 +:providers: + :gce: + connection_pool_timeout: 1 + project: '#{project}' + zone: '#{zone}' + network_name: 'projects/itsysopsnetworking/global/networks/shared1' + dns_zone_resource_name: 'test-vmpooler-puppet-net' + dns_zone: 'test.vmpooler.puppet.net' +:pools: + - name: '#{poolname}' + alias: [ 'mockpool' ] + template: 'projects/debian-cloud/global/images/family/debian-9' + size: 5 + timeout: 10 + ready_ttl: 1440 + provider: 'gce' + subnetwork_name: 'projects/itsysopsnetworking/regions/us-west1/subnetworks/vmpooler-test' + machine_type: 'zones/#{zone}/machineTypes/e2-micro' + EOT + ) } + skip 'gets a vm' do + result = subject.create_vm(poolname, vmname) + #subject.get_vm(poolname, vmname) + #subject.dns_teardown({'name' => vmname}) + # subject.dns_setup({'name' => vmname, 'ip' => '1.2.3.5'}) + end end end diff --git a/spec/vmpooler-provider-gce/vmpooler_provider_gce_spec.rb b/spec/vmpooler-provider-gce/vmpooler_provider_gce_spec.rb new file mode 100644 index 0000000..3337597 --- /dev/null +++ b/spec/vmpooler-provider-gce/vmpooler_provider_gce_spec.rb @@ -0,0 +1,9 @@ +require 'rspec' + +describe 'VmpoolerProviderGce' do + context 'when creating class ' do + it 'sets a version' do + expect(VmpoolerProviderGce::VERSION).not_to be_nil + end + end +end \ No newline at end of file diff --git a/vmpooler-provider-gce.gemspec b/vmpooler-provider-gce.gemspec index 15bbca0..0c1d8cb 100644 --- a/vmpooler-provider-gce.gemspec +++ b/vmpooler-provider-gce.gemspec @@ -17,6 +17,7 @@ Gem::Specification.new do |s| s.require_paths = ["lib"] s.add_dependency "google-apis-compute_v1", "~> 0.14" s.add_dependency "googleauth", "~> 0.16.2" + s.add_dependency "google-cloud-dns", "~>0.35.1" s.add_development_dependency 'vmpooler', '~> 1.3', '>= 1.3.0' diff --git a/vmpooler.yaml.example b/vmpooler.yaml.example index 772de78..b784c19 100644 --- a/vmpooler.yaml.example +++ b/vmpooler.yaml.example @@ -72,6 +72,13 @@ # - network_name # The GCE network_name to use # (required) +# - dns_zone_resource_name +# The name given to the DNS zone ressource. This is not the domain, but the name identifier of a zone eg example-com +# (optional) when not set, the dns setup / teardown is skipped +# - dns_zone +# The dns zone domain set for the dns_zone_resource_name. This becomes the domain part of the FQDN ie $vm_name.$dns_zone +# When setting multiple providers at the same time, this value should be set for each GCE pools. +# default to: global config:domain. if dns_zone is set, it overwrites the top-level domain when checking vm_ready? # Example: :gce: From a4c730df7b915af2e16cd4ef3597cb7148e53e6a Mon Sep 17 00:00:00 2001 From: Samuel Beaulieu Date: Wed, 29 Dec 2021 12:02:55 -0600 Subject: [PATCH 2/7] fix rubocop and spec tests --- .rubocop.yml | 16 +++++++----- lib/vmpooler/providers/gce.rb | 46 ++++++++++++++++++--------------- spec/dnsservice_helper.rb | 9 +++++++ spec/spec_helper.rb | 3 +++ spec/unit/providers/gce_spec.rb | 6 ++--- 5 files changed, 49 insertions(+), 31 deletions(-) create mode 100644 spec/dnsservice_helper.rb diff --git a/.rubocop.yml b/.rubocop.yml index 782e71e..3333234 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -36,16 +36,18 @@ Style/SwapValues: # (new in 1.1) #disabled Metrics/AbcSize: - Max: 77 + Enabled: false Metrics/ClassLength: - Max: 430 + Enabled: false Metrics/CyclomaticComplexity: - Max: 14 + Enabled: false Metrics/MethodLength: - Max: 48 + Enabled: false Metrics/PerceivedComplexity: - Max: 14 + Enabled: false Metrics/ParameterLists: - Max: 6 + Enabled: false Layout/LineLength: - Max: 220 \ No newline at end of file + Enabled: false +Metrics/BlockLength: + Enabled: false \ No newline at end of file diff --git a/lib/vmpooler/providers/gce.rb b/lib/vmpooler/providers/gce.rb index d929a85..d5b1dde 100644 --- a/lib/vmpooler/providers/gce.rb +++ b/lib/vmpooler/providers/gce.rb @@ -2,7 +2,7 @@ require 'googleauth' require 'google/apis/compute_v1' -require "google/cloud/dns" +require 'google/cloud/dns' require 'bigdecimal' require 'bigdecimal/util' require 'vmpooler/providers/base' @@ -45,7 +45,6 @@ module Vmpooler { connection: new_conn } end @redis = redis_connection_pool - @dns = Google::Cloud::Dns.new(project_id: project) end # name of the provider class @@ -59,6 +58,10 @@ module Vmpooler end end + def dns + @dns ||= Google::Cloud::Dns.new(project_id: project) + end + # main configuration options def project provider_config['project'] @@ -176,7 +179,7 @@ module Vmpooler network_interfaces = Google::Apis::ComputeV1::NetworkInterface.new( network: network_name ) - network_interfaces.subnetwork=subnetwork_name(pool_name) if subnetwork_name(pool_name) + network_interfaces.subnetwork = subnetwork_name(pool_name) if subnetwork_name(pool_name) init_params = { source_image: pool['template'], # The source image to create this disk. labels: { 'vm' => new_vmname, 'pool' => pool_name }, @@ -195,15 +198,16 @@ module Vmpooler network_interfaces: [network_interfaces], labels: { 'vm' => new_vmname, 'pool' => pool_name, project => nil } ) -=begin TODO: Maybe this will be needed to set the hostname (usually internal DNS name but in opur case for some reason its nil) - given_hostname = "#{new_vmname}.#{dns_zone}" - client.hostname = given_hostname if given_hostname -=end + # TODO: Maybe this will be needed to set the hostname (usually internal DNS name but in opur case for some reason its nil) + # given_hostname = "#{new_vmname}.#{dns_zone}" + # client.hostname = given_hostname if given_hostname + debug_logger('trigger insert_instance') result = connection.insert_instance(project, zone(pool_name), client) wait_for_operation(project, pool_name, result) created_instance = get_vm(pool_name, new_vmname) dns_setup(created_instance) + created_instance end # create_disk creates an additional disk for an existing VM. It will name the new @@ -554,24 +558,24 @@ module Vmpooler # END BASE METHODS def dns_setup(created_instance) - zone = @dns.zone dns_zone_resource_name if dns_zone_resource_name - if zone && created_instance - name = created_instance['name'] - change = zone.add name, "A", 60, [created_instance['ip']] - debug_logger("#{change.id} - #{change.started_at} - #{change.status}") if change - end + zone = dns.zone dns_zone_resource_name if dns_zone_resource_name + return unless zone && created_instance + + name = created_instance['name'] + change = zone.add name, 'A', 60, [created_instance['ip']] + debug_logger("#{change.id} - #{change.started_at} - #{change.status}") if change # TODO: should we catch Google::Cloud::AlreadyExistsError that is thrown when it already exist? # and then delete and recreate? # eg the error is Google::Cloud::AlreadyExistsError: alreadyExists: The resource 'entity.change.additions[0]' named 'instance-8.test.vmpooler.puppet.net. (A)' already exists end def dns_teardown(created_instance) - zone = @dns.zone dns_zone_resource_name if dns_zone_resource_name - if zone && created_instance - name = created_instance['name'] - change = zone.remove name, "A" - debug_logger("#{change.id} - #{change.started_at} - #{change.status}") if change - end + zone = dns.zone dns_zone_resource_name if dns_zone_resource_name + return unless zone && created_instance + + name = created_instance['name'] + change = zone.remove name, 'A' + debug_logger("#{change.id} - #{change.started_at} - #{change.status}") if change end def should_be_ignored(item, allowlist) @@ -652,7 +656,7 @@ module Vmpooler 'machine_type' => vm_object.machine_type, 'labels' => vm_object.labels, 'label_fingerprint' => vm_object.label_fingerprint, - 'ip' => vm_object.network_interfaces.first.network_ip + 'ip' => vm_object.network_interfaces ? vm_object.network_interfaces.first.network_ip : nil # 'powerstate' => powerstate } end @@ -728,6 +732,6 @@ module Vmpooler logger.log('[g]', message) if send_to_upstream end end - end + end end end diff --git a/spec/dnsservice_helper.rb b/spec/dnsservice_helper.rb new file mode 100644 index 0000000..c2752c0 --- /dev/null +++ b/spec/dnsservice_helper.rb @@ -0,0 +1,9 @@ +MockDNS = Struct.new( + # https://rubydoc.info/gems/google-cloud-dns/0.35.1/Google/Cloud/Dns + :change, :credentials, :project, :record, :zone, + keyword_init: true +) do + def zone(zone) + self.zone = zone + end +end \ No newline at end of file diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 603f438..14ab878 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -1,15 +1,18 @@ # frozen_string_literal: true require 'simplecov' +=begin SimpleCov.start do add_filter '/spec/' end +=end require 'helpers' require 'rspec' require 'vmpooler' require 'redis' require 'vmpooler/metrics' require 'computeservice_helper' +require 'dnsservice_helper' def project_root_dir File.dirname(File.dirname(__FILE__)) diff --git a/spec/unit/providers/gce_spec.rb b/spec/unit/providers/gce_spec.rb index bf69457..fa7a466 100644 --- a/spec/unit/providers/gce_spec.rb +++ b/spec/unit/providers/gce_spec.rb @@ -26,8 +26,6 @@ describe 'Vmpooler::PoolManager::Provider::Gce' do zone: '#{zone}' network_name: global/networks/default # network_name: 'projects/itsysopsnetworking/global/networks/shared1' - dns_zone_resource_name: 'example-com' - dns_zone: 'example.com' :pools: - name: '#{poolname}' alias: [ 'mockpool' ] @@ -56,6 +54,8 @@ EOT subject { Vmpooler::PoolManager::Provider::Gce.new(config, logger, metrics, redis_connection_pool, 'gce', provider_options) } + before(:each) { allow(subject).to receive(:dns).and_return(MockDNS.new()) } + describe '#name' do it 'should be gce' do expect(subject.name).to eq('gce') @@ -89,7 +89,7 @@ EOT end context 'in itsysops' do - let(:vmname) { "instance-10" } + let(:vmname) { "instance-15" } let(:project) { 'vmpooler-test' } let(:config) { YAML.load(<<-EOT --- From 0beec3d8e981f0294ad6848c2510a0066b5b9611 Mon Sep 17 00:00:00 2001 From: Samuel Beaulieu Date: Wed, 29 Dec 2021 12:17:25 -0600 Subject: [PATCH 3/7] code review fixes --- README.md | 6 +++--- lib/vmpooler/providers/gce.rb | 2 +- spec/unit/providers/gce_spec.rb | 30 +----------------------------- vmpooler-provider-gce.gemspec | 2 +- 4 files changed, 6 insertions(+), 34 deletions(-) diff --git a/README.md b/README.md index ec761b7..8260302 100644 --- a/README.md +++ b/README.md @@ -15,7 +15,7 @@ GCE authorization is handled via a service account (or personal account) private 1. GOOGLE_APPLICATION_CREDENTIALS environment variable eg GOOGLE_APPLICATION_CREDENTIALS=/my/home/directory/my_account_key.json ### DNS -DNS is integrated via Google's CloudDNS service. To enable a CloudDNS zone name must be provided in the config (see the example yaml file dns_zone_resource_name) +DNS is integrated via Google's CloudDNS service. To enable, a CloudDNS zone name must be provided in the config (see the example yaml file dns_zone_resource_name) An A record is then created in that zone upon instance creation with the VM's internal IP, and deleted when the instance is destroyed. @@ -34,8 +34,8 @@ do not have the pool label, and can be configured to allow a specific list of un ### Pre-requisite - A service account needs to be created and a private json key generated (see usage section) -- The service account needs given permissions to the project (broad permissions would be compute v1 admin and dns admin). A yaml file is provided that lists the least-privilege permissions needed -- if using DNS, a DNS zone needs to be created +- The service account needs to be given permissions to the project (broad permissions would be compute v1 admin and dns admin). A yaml file is provided that lists the least-privilege permissions needed +- if using DNS, a DNS zone needs to be created in CloudDNS, and configured in the provider's config section with the name of that zone (dns_zone_resource_name). When not specified, the DNS setup and teardown is skipped. ## License diff --git a/lib/vmpooler/providers/gce.rb b/lib/vmpooler/providers/gce.rb index d5b1dde..3381471 100644 --- a/lib/vmpooler/providers/gce.rb +++ b/lib/vmpooler/providers/gce.rb @@ -198,7 +198,7 @@ module Vmpooler network_interfaces: [network_interfaces], labels: { 'vm' => new_vmname, 'pool' => pool_name, project => nil } ) - # TODO: Maybe this will be needed to set the hostname (usually internal DNS name but in opur case for some reason its nil) + # TODO: Maybe this will be needed to set the hostname (usually internal DNS name but in our case for some reason its nil) # given_hostname = "#{new_vmname}.#{dns_zone}" # client.hostname = given_hostname if given_hostname diff --git a/spec/unit/providers/gce_spec.rb b/spec/unit/providers/gce_spec.rb index fa7a466..3075f8a 100644 --- a/spec/unit/providers/gce_spec.rb +++ b/spec/unit/providers/gce_spec.rb @@ -11,8 +11,7 @@ describe 'Vmpooler::PoolManager::Provider::Gce' do let(:metrics) { Vmpooler::Metrics::DummyStatsd.new } let(:poolname) { 'debian-9' } let(:provider_options) { { 'param' => 'value' } } - # let(:project) { 'vmpooler-test' } - let(:project) { 'dio-samuel-dev' } + let(:project) { 'vmpooler-test' } let(:zone) { 'us-west1-b' } let(:config) { YAML.load(<<-EOT --- @@ -25,7 +24,6 @@ describe 'Vmpooler::PoolManager::Provider::Gce' do project: '#{project}' zone: '#{zone}' network_name: global/networks/default - # network_name: 'projects/itsysopsnetworking/global/networks/shared1' :pools: - name: '#{poolname}' alias: [ 'mockpool' ] @@ -34,7 +32,6 @@ describe 'Vmpooler::PoolManager::Provider::Gce' do timeout: 10 ready_ttl: 1440 provider: 'gce' - # subnetwork_name: 'projects/itsysopsnetworking/regions/us-west1/subnetworks/vmpooler-test' machine_type: 'zones/#{zone}/machineTypes/e2-micro' EOT ) @@ -63,31 +60,6 @@ EOT end describe '#manual tests live' do - skip 'runs in gce' do - puts 'creating' - result = subject.create_vm(poolname, vmname) - subject.get_vm(poolname, vmname) - subject.vms_in_pool(poolname) - - puts 'create snapshot w/ one disk' - result = subject.create_snapshot(poolname, vmname, 'sams') - puts 'create disk' - result = subject.create_disk(poolname, vmname, 10) - puts 'create snapshot w/ 2 disks' - result = subject.create_snapshot(poolname, vmname, 'sams2') - puts 'revert snapshot' - result = subject.revert_snapshot(poolname, vmname, 'sams') - result = subject.destroy_vm(poolname, vmname) - end - - skip 'runs existing' do - # result = subject.create_snapshot(poolname, vmname, "sams") - # result = subject.revert_snapshot(poolname, vmname, "sams") - # puts subject.get_vm(poolname, vmname) - result = subject.create_vm(poolname, vmname) - result = subject.destroy_vm(poolname, vmname) - end - context 'in itsysops' do let(:vmname) { "instance-15" } let(:project) { 'vmpooler-test' } diff --git a/vmpooler-provider-gce.gemspec b/vmpooler-provider-gce.gemspec index 0c1d8cb..3127a46 100644 --- a/vmpooler-provider-gce.gemspec +++ b/vmpooler-provider-gce.gemspec @@ -17,7 +17,7 @@ Gem::Specification.new do |s| s.require_paths = ["lib"] s.add_dependency "google-apis-compute_v1", "~> 0.14" s.add_dependency "googleauth", "~> 0.16.2" - s.add_dependency "google-cloud-dns", "~>0.35.1" + s.add_dependency "google-cloud-dns", "~> 0.35.1" s.add_development_dependency 'vmpooler', '~> 1.3', '>= 1.3.0' From b66218dc10a57f850b59b47670189df7005c6351 Mon Sep 17 00:00:00 2001 From: Samuel Beaulieu Date: Fri, 31 Dec 2021 09:35:29 -0600 Subject: [PATCH 4/7] replace dns zone entry if it already exists on the rare occasion where adding a dns record and we get teh error it already exists assume that it is stale and replace it with the new IP --- lib/vmpooler/providers/gce.rb | 25 ++++++++++++------------- 1 file changed, 12 insertions(+), 13 deletions(-) diff --git a/lib/vmpooler/providers/gce.rb b/lib/vmpooler/providers/gce.rb index 3381471..3da189f 100644 --- a/lib/vmpooler/providers/gce.rb +++ b/lib/vmpooler/providers/gce.rb @@ -134,7 +134,7 @@ module Vmpooler # [String] hostname : Specifies the hostname of the instance. The specified hostname must be RFC1035 compliant. If hostname is not specified, # the default hostname is [ INSTANCE_NAME].c.[PROJECT_ID].internal when using the global DNS, and # [ INSTANCE_NAME].[ZONE].c.[PROJECT_ID].internal when using zonal DNS - # [String] template : This is the name of template exposed by the API. It must _match_ the poolname ??? TODO + # [String] template : This is the name of template # [String] poolname : Name of the pool the VM as per labels # [Time] boottime : Time when the VM was created/booted # [String] status : One of the following values: PROVISIONING, STAGING, RUNNING, STOPPING, SUSPENDING, SUSPENDED, REPAIRING, and TERMINATED @@ -198,9 +198,6 @@ module Vmpooler network_interfaces: [network_interfaces], labels: { 'vm' => new_vmname, 'pool' => pool_name, project => nil } ) - # TODO: Maybe this will be needed to set the hostname (usually internal DNS name but in our case for some reason its nil) - # given_hostname = "#{new_vmname}.#{dns_zone}" - # client.hostname = given_hostname if given_hostname debug_logger('trigger insert_instance') result = connection.insert_instance(project, zone(pool_name), client) @@ -559,14 +556,17 @@ module Vmpooler def dns_setup(created_instance) zone = dns.zone dns_zone_resource_name if dns_zone_resource_name - return unless zone && created_instance + return unless zone && created_instance && created_instance['name'] && created_instance['ip'] name = created_instance['name'] - change = zone.add name, 'A', 60, [created_instance['ip']] - debug_logger("#{change.id} - #{change.started_at} - #{change.status}") if change - # TODO: should we catch Google::Cloud::AlreadyExistsError that is thrown when it already exist? - # and then delete and recreate? - # eg the error is Google::Cloud::AlreadyExistsError: alreadyExists: The resource 'entity.change.additions[0]' named 'instance-8.test.vmpooler.puppet.net. (A)' already exists + begin + change = zone.add name, 'A', 60, [created_instance['ip']] + debug_logger("#{change.id} - #{change.started_at} - #{change.status}") if change + rescue AlreadyExistsError => _e + # DNS setup is done only for new instances, so in the rare case where a DNS record already exists (it is stale) and we replace it. + # the error is Google::Cloud::AlreadyExistsError: alreadyExists: The resource 'entity.change.additions[0]' named 'instance-8.test.vmpooler.net. (A)' already exists + zone.replace(name, 'A', 60, [created_instance['ip']]) + end end def dns_teardown(created_instance) @@ -640,7 +640,7 @@ module Vmpooler end # Return a hash of VM data - # Provides vmname, hostname, template, poolname, boottime, status, zone, machine_type information + # Provides vmname, hostname, template, poolname, boottime, status, zone, machine_type, labels, label_fingerprint, ip information def generate_vm_hash(vm_object, pool_name) pool_configuration = pool_config(pool_name) return nil if pool_configuration.nil? @@ -648,7 +648,7 @@ module Vmpooler { 'name' => vm_object.name, 'hostname' => vm_object.hostname, - 'template' => pool_configuration&.key?('template') ? pool_configuration['template'] : nil, # TODO: get it from the API, not from config, but this is what vSphere does too! + 'template' => pool_configuration&.key?('template') ? pool_configuration['template'] : nil, # was expecting to get it from API, not from config, but this is what vSphere does too! 'poolname' => vm_object.labels&.key?('pool') ? vm_object.labels['pool'] : nil, 'boottime' => vm_object.creation_timestamp, 'status' => vm_object.status, # One of the following values: PROVISIONING, STAGING, RUNNING, STOPPING, SUSPENDING, SUSPENDED, REPAIRING, and TERMINATED @@ -657,7 +657,6 @@ module Vmpooler 'labels' => vm_object.labels, 'label_fingerprint' => vm_object.label_fingerprint, 'ip' => vm_object.network_interfaces ? vm_object.network_interfaces.first.network_ip : nil - # 'powerstate' => powerstate } end From 2cb2550aca41ec7f66c77234400e0b054078d081 Mon Sep 17 00:00:00 2001 From: Samuel Beaulieu Date: Fri, 31 Dec 2021 09:54:46 -0600 Subject: [PATCH 5/7] refactor dns_zone to be renamed domain to match the global parameter being overwritten in the upstream vmpooler code, we use the global domain parameter, so naming that parameter the same in GCE. This parameter can be optionally set in the provider config, and overwrites the global parameter. It is used to infer the FQDN as . --- lib/vmpooler/providers/gce.rb | 6 +++--- spec/spec_helper.rb | 2 -- spec/unit/providers/gce_spec.rb | 2 +- vmpooler.yaml.example | 9 ++++++--- 4 files changed, 10 insertions(+), 9 deletions(-) diff --git a/lib/vmpooler/providers/gce.rb b/lib/vmpooler/providers/gce.rb index 3da189f..f8ccafd 100644 --- a/lib/vmpooler/providers/gce.rb +++ b/lib/vmpooler/providers/gce.rb @@ -86,8 +86,8 @@ module Vmpooler return provider_config['machine_type'] if provider_config['machine_type'] end - def dns_zone - provider_config['dns_zone'] + def domain + provider_config['domain'] end def dns_zone_resource_name @@ -459,7 +459,7 @@ module Vmpooler def vm_ready?(_pool_name, vm_name) begin # TODO: we could use a healthcheck resource attached to instance - open_socket(vm_name, dns_zone || global_config[:config]['domain']) + open_socket(vm_name, domain || global_config[:config]['domain']) rescue StandardError => _e return false end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 14ab878..6cd4447 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -1,11 +1,9 @@ # frozen_string_literal: true require 'simplecov' -=begin SimpleCov.start do add_filter '/spec/' end -=end require 'helpers' require 'rspec' require 'vmpooler' diff --git a/spec/unit/providers/gce_spec.rb b/spec/unit/providers/gce_spec.rb index 3075f8a..550e3bf 100644 --- a/spec/unit/providers/gce_spec.rb +++ b/spec/unit/providers/gce_spec.rb @@ -75,7 +75,7 @@ EOT zone: '#{zone}' network_name: 'projects/itsysopsnetworking/global/networks/shared1' dns_zone_resource_name: 'test-vmpooler-puppet-net' - dns_zone: 'test.vmpooler.puppet.net' + domain: 'test.vmpooler.puppet.net' :pools: - name: '#{poolname}' alias: [ 'mockpool' ] diff --git a/vmpooler.yaml.example b/vmpooler.yaml.example index b784c19..c51265d 100644 --- a/vmpooler.yaml.example +++ b/vmpooler.yaml.example @@ -75,10 +75,11 @@ # - dns_zone_resource_name # The name given to the DNS zone ressource. This is not the domain, but the name identifier of a zone eg example-com # (optional) when not set, the dns setup / teardown is skipped -# - dns_zone -# The dns zone domain set for the dns_zone_resource_name. This becomes the domain part of the FQDN ie $vm_name.$dns_zone +# - domain +# Overwrites the global domain parameter. This should match the dns zone domain set for the dns_zone_resource_name. +# It is used to infer the domain part of the FQDN ie $vm_name.$domain # When setting multiple providers at the same time, this value should be set for each GCE pools. -# default to: global config:domain. if dns_zone is set, it overwrites the top-level domain when checking vm_ready? +# (optional) If not explicitely set, the FQDN is inferred using the global 'domain' config parameter # Example: :gce: @@ -86,6 +87,8 @@ zone: 'us-central1-f' machine_type: '' network_name: '' + dns_zone_resource_name: 'subdomain-example-com' + domain: 'subdomain.example.com' # :pools: # From 83770acd892da35dc81cb6868045a3b8e8db0f1c Mon Sep 17 00:00:00 2001 From: Samuel Beaulieu Date: Thu, 6 Jan 2022 15:31:42 -0600 Subject: [PATCH 6/7] fix dns_setup as zone was already a method also changed from setting the project name in the instance labels to a instance tag aka network tag, as the setup for allowing traffic in the FW is bassed on tag not label --- lib/vmpooler/providers/gce.rb | 25 ++++---- scripts/GCE_custom_role_for_SA.yaml | 1 + spec/unit/providers/gce_spec.rb | 94 +++++++++++++++-------------- 3 files changed, 63 insertions(+), 57 deletions(-) diff --git a/lib/vmpooler/providers/gce.rb b/lib/vmpooler/providers/gce.rb index f8ccafd..6cf4450 100644 --- a/lib/vmpooler/providers/gce.rb +++ b/lib/vmpooler/providers/gce.rb @@ -60,6 +60,7 @@ module Vmpooler def dns @dns ||= Google::Cloud::Dns.new(project_id: project) + @dns end # main configuration options @@ -196,7 +197,8 @@ module Vmpooler machine_type: pool['machine_type'], disks: [disk], network_interfaces: [network_interfaces], - labels: { 'vm' => new_vmname, 'pool' => pool_name, project => nil } + labels: { 'vm' => new_vmname, 'pool' => pool_name }, + tags: Google::Apis::ComputeV1::Tags.new(items: [project]) ) debug_logger('trigger insert_instance') @@ -555,27 +557,28 @@ module Vmpooler # END BASE METHODS def dns_setup(created_instance) - zone = dns.zone dns_zone_resource_name if dns_zone_resource_name - return unless zone && created_instance && created_instance['name'] && created_instance['ip'] + dns_zone = dns.zone(dns_zone_resource_name) if dns_zone_resource_name + return unless dns_zone && created_instance && created_instance['name'] && created_instance['ip'] name = created_instance['name'] begin - change = zone.add name, 'A', 60, [created_instance['ip']] - debug_logger("#{change.id} - #{change.started_at} - #{change.status}") if change - rescue AlreadyExistsError => _e + change = dns_zone.add(name, 'A', 60, [created_instance['ip']]) + debug_logger("#{change.id} - #{change.started_at} - #{change.status} DNS address added") if change + rescue Google::Cloud::AlreadyExistsError => _e # DNS setup is done only for new instances, so in the rare case where a DNS record already exists (it is stale) and we replace it. # the error is Google::Cloud::AlreadyExistsError: alreadyExists: The resource 'entity.change.additions[0]' named 'instance-8.test.vmpooler.net. (A)' already exists - zone.replace(name, 'A', 60, [created_instance['ip']]) + change = dns_zone.replace(name, 'A', 60, [created_instance['ip']]) + debug_logger("#{change.id} - #{change.started_at} - #{change.status} DNS address previously existed and was replaced") if change end end def dns_teardown(created_instance) - zone = dns.zone dns_zone_resource_name if dns_zone_resource_name - return unless zone && created_instance + dns_zone = dns.zone(dns_zone_resource_name) if dns_zone_resource_name + return unless dns_zone && created_instance name = created_instance['name'] - change = zone.remove name, 'A' - debug_logger("#{change.id} - #{change.started_at} - #{change.status}") if change + change = dns_zone.remove(name, 'A') + debug_logger("#{change.id} - #{change.started_at} - #{change.status} DNS address removed") if change end def should_be_ignored(item, allowlist) diff --git a/scripts/GCE_custom_role_for_SA.yaml b/scripts/GCE_custom_role_for_SA.yaml index 696f455..fe26b4c 100644 --- a/scripts/GCE_custom_role_for_SA.yaml +++ b/scripts/GCE_custom_role_for_SA.yaml @@ -16,6 +16,7 @@ includedPermissions: - compute.instances.get - compute.instances.list - compute.instances.setLabels +- compute.instances.setTags - compute.instances.start - compute.instances.stop - compute.snapshots.create diff --git a/spec/unit/providers/gce_spec.rb b/spec/unit/providers/gce_spec.rb index 550e3bf..cc43c0c 100644 --- a/spec/unit/providers/gce_spec.rb +++ b/spec/unit/providers/gce_spec.rb @@ -13,26 +13,26 @@ describe 'Vmpooler::PoolManager::Provider::Gce' do let(:provider_options) { { 'param' => 'value' } } let(:project) { 'vmpooler-test' } let(:zone) { 'us-west1-b' } - let(:config) { YAML.load(<<-EOT ---- -:config: - max_tries: 3 - retry_factor: 10 -:providers: - :gce: - connection_pool_timeout: 1 - project: '#{project}' - zone: '#{zone}' - network_name: global/networks/default -:pools: - - name: '#{poolname}' - alias: [ 'mockpool' ] - template: 'projects/debian-cloud/global/images/family/debian-9' - size: 5 - timeout: 10 - ready_ttl: 1440 - provider: 'gce' - machine_type: 'zones/#{zone}/machineTypes/e2-micro' + let(:config) { YAML.load(<<~EOT + --- + :config: + max_tries: 3 + retry_factor: 10 + :providers: + :gce: + connection_pool_timeout: 1 + project: '#{project}' + zone: '#{zone}' + network_name: global/networks/default + :pools: + - name: '#{poolname}' + alias: [ 'mockpool' ] + template: 'projects/debian-cloud/global/images/family/debian-9' + size: 5 + timeout: 10 + ready_ttl: 1440 + provider: 'gce' + machine_type: 'zones/#{zone}/machineTypes/e2-micro' EOT ) } @@ -61,36 +61,38 @@ EOT describe '#manual tests live' do context 'in itsysops' do - let(:vmname) { "instance-15" } + before(:each) { allow(subject).to receive(:dns).and_call_original } + let(:vmname) { "instance-24" } let(:project) { 'vmpooler-test' } - let(:config) { YAML.load(<<-EOT ---- -:config: - max_tries: 3 - retry_factor: 10 -:providers: - :gce: - connection_pool_timeout: 1 - project: '#{project}' - zone: '#{zone}' - network_name: 'projects/itsysopsnetworking/global/networks/shared1' - dns_zone_resource_name: 'test-vmpooler-puppet-net' - domain: 'test.vmpooler.puppet.net' -:pools: - - name: '#{poolname}' - alias: [ 'mockpool' ] - template: 'projects/debian-cloud/global/images/family/debian-9' - size: 5 - timeout: 10 - ready_ttl: 1440 - provider: 'gce' - subnetwork_name: 'projects/itsysopsnetworking/regions/us-west1/subnetworks/vmpooler-test' - machine_type: 'zones/#{zone}/machineTypes/e2-micro' - EOT + let(:config) { YAML.load(<<~EOT + --- + :config: + max_tries: 3 + retry_factor: 10 + :providers: + :gce: + connection_pool_timeout: 1 + project: '#{project}' + zone: '#{zone}' + network_name: 'projects/itsysopsnetworking/global/networks/shared1' + dns_zone_resource_name: 'test-vmpooler-puppet-net' + domain: 'test.vmpooler.puppet.net' + :pools: + - name: '#{poolname}' + alias: [ 'mockpool' ] + template: 'projects/debian-cloud/global/images/family/debian-9' + size: 5 + timeout: 10 + ready_ttl: 1440 + provider: 'gce' + subnetwork_name: 'projects/itsysopsnetworking/regions/us-west1/subnetworks/vmpooler-test' + machine_type: 'zones/#{zone}/machineTypes/e2-micro' +EOT ) } skip 'gets a vm' do result = subject.create_vm(poolname, vmname) - #subject.get_vm(poolname, vmname) + #result = subject.destroy_vm(poolname, vmname) + subject.get_vm(poolname, vmname) #subject.dns_teardown({'name' => vmname}) # subject.dns_setup({'name' => vmname, 'ip' => '1.2.3.5'}) end From e3119758b4014e3007f5372786a2f424eb0edf80 Mon Sep 17 00:00:00 2001 From: Samuel Beaulieu Date: Mon, 10 Jan 2022 10:43:53 -0600 Subject: [PATCH 7/7] fix rubocop offence --- lib/vmpooler/providers/gce.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/vmpooler/providers/gce.rb b/lib/vmpooler/providers/gce.rb index 6cf4450..3aa965e 100644 --- a/lib/vmpooler/providers/gce.rb +++ b/lib/vmpooler/providers/gce.rb @@ -567,7 +567,7 @@ module Vmpooler rescue Google::Cloud::AlreadyExistsError => _e # DNS setup is done only for new instances, so in the rare case where a DNS record already exists (it is stale) and we replace it. # the error is Google::Cloud::AlreadyExistsError: alreadyExists: The resource 'entity.change.additions[0]' named 'instance-8.test.vmpooler.net. (A)' already exists - change = dns_zone.replace(name, 'A', 60, [created_instance['ip']]) + change = dns_zone.replace(name, 'A', 60, [created_instance['ip']]) debug_logger("#{change.id} - #{change.started_at} - #{change.status} DNS address previously existed and was replaced") if change end end