Merge "Add the option to only aggregate hosts that are known by openstack to be active"

lib/puppet/provider/nova_aggregate/openstack.rb

@@ -21,7 +21,8 @@ Puppet::Type.type(:nova_aggregate).provide(
           :id => attrs[:id],
           :availability_zone => attrs[:availability_zone],
           :metadata => str2hash(attrs[:properties]),
-          :hosts => string2list(attrs[:hosts]).sort
+          :hosts => string2list(attrs[:hosts]).sort,
+          :filter_hosts => attrs[:filter_hosts]
       )
     end
   end
@@ -39,6 +40,11 @@ Puppet::Type.type(:nova_aggregate).provide(
     end
   end
 
+  def self.get_known_hosts
+    # get list of hosts known to be active from openstack
+    return request('compute service', 'list', ['--service', 'nova-compute']).map{|el| el[:host]}
+  end
+
   def exists?
     @property_hash[:ensure] == :present
   end
@@ -62,8 +68,11 @@ Puppet::Type.type(:nova_aggregate).provide(
       end
     end
     @property_hash = self.class.request('aggregate', 'create', properties)
-
     if not @resource[:hosts].nil? and not @resource[:hosts].empty?
+      # filter host list by known hosts if filter_hosts is set
+      if @resource[:filter_hosts] == :true
+        @resource[:hosts] = @resource[:hosts] & self.class.get_known_hosts()
+      end
       @resource[:hosts].each do |host|
         properties = [@property_hash[:name], host]
         self.class.request('aggregate', 'add host', properties)
@@ -93,15 +102,20 @@ Puppet::Type.type(:nova_aggregate).provide(
   end
 
   def hosts=(value)
-    # remove hosts, which are not present in update
-    (@property_hash[:hosts] - @resource[:hosts]).each do |host|
-      properties = [@property_hash[:id], host]
-      self.class.request('aggregate', 'remove host', properties)
+    # filter host list by known hosts if filter_hosts is set
+    if @resource[:filter_hosts] == :true
+      value &= self.class.get_known_hosts()
     end
-    # add new hosts
-    (@resource[:hosts] - @property_hash[:hosts]).each do |host|
-      properties = [@property_hash[:id], host]
-      self.class.request('aggregate', 'add host', properties)
+    if not @property_hash[:hosts].nil?
+      # remove hosts that are not present in update
+      (@property_hash[:hosts] - value).each do |host|
+        self.class.request('aggregate', 'remove host', [@property_hash[:id], host])
+      end
+      # add hosts that are not already present
+      (value - @property_hash[:hosts]).each do |host|
+        self.class.request('aggregate', 'add host', [@property_hash[:id], host])
+      end
     end
   end
+
 end

lib/puppet/type/nova_aggregate.rb

@@ -23,6 +23,11 @@
 #    Name for the new aggregate
 #    Required
 #
+#  [*filter_hosts*]
+#    A boolean-y value to toggle whether only hosts known to be active by
+#    openstack should be aggregated. i.e. "true" or "false"
+#    Optional, defaults to "false"
+#
 #  [*availability_zone*]
 #    The availability zone. ie "zone1"
 #    Optional
@@ -60,6 +65,12 @@ Puppet::Type.newtype(:nova_aggregate) do
     end
   end
 
+  newparam(:filter_hosts) do
+    desc 'Toggle to filter given hosts so that only known nova-compute service hosts are added to the aggregate'
+    defaultto :false
+    newvalues(:true, :false)
+  end
+
   newproperty(:id) do
     desc 'The unique Id of the aggregate'
     validate do |v|

spec/unit/provider/nova_aggregate/openstack_spec.rb

@@ -97,4 +97,105 @@ hosts="[u\'example\']"
 
     end
   end
+
+
+  describe 'managing aggregates with filter_hosts toggled' do
+
+    # instantiation attributes for the provider with filter_hosts set.
+    let(:aggregate_attrs) do
+      {
+         :name              => 'just',
+         :availability_zone => 'simple',
+         :hosts             => ['known'],
+         :ensure            => 'present',
+         :metadata          => 'nice=cookie',
+         :filter_hosts      => 'true'
+      }
+    end
+
+    let(:resource) do
+      Puppet::Type::Nova_aggregate.new(aggregate_attrs)
+    end
+
+    let(:provider) do
+      provider_class.new(resource)
+    end
+
+    it_behaves_like 'authenticated with environment variables' do
+
+      # create an aggregate and actually aggregate hosts to it
+      describe 'create aggregate and add/remove hosts with filter_hosts toggled' do
+
+        it 'creates aggregate with filter_hosts toggled' do
+
+          provider.class.stubs(:get_known_hosts)
+            .returns(['known', 'known_too'])
+
+          # these expectations are the actual tests that check the provider's behaviour
+          # and make sure only known hosts ('known' is the only known host) will be
+          # aggregated.
+
+          provider_class.expects(:openstack)
+            .with('aggregate', 'create', '--format', 'shell', ['just', '--zone', 'simple', "--property", "nice=cookie"])
+            .once
+            .returns('name="just"
+id="just"
+availability_zone="simple"
+properties="{u\'nice\': u\'cookie\'}"
+hosts="[]"
+')
+
+          provider_class.expects(:openstack)
+            .with('aggregate', 'add host', ['just', 'known'])
+            .once
+            .returns('name="just"
+id="just"
+availability_zone="simple"
+properties="{u\'nice\': u\'cookie\'}"
+hosts="[u\'known\']"
+')
+
+          provider_class.expects(:openstack)
+            .with('aggregate', 'add host', ['just', 'known_too'])
+            .once
+            .returns('name="just"
+id="just"
+availability_zone="simple"
+properties="{u\'nice\': u\'cookie\'}"
+hosts="[u\'known\', u\'known_too\']"
+')
+
+          provider_class.expects(:openstack)
+            .with('aggregate', 'remove host', ['just', 'known'])
+            .once
+            .returns('name="just"
+id="just"
+availability_zone="simple"
+properties="{u\'nice\': u\'cookie\'}"
+hosts="[u\'known_too\']"
+')
+
+          # this creates a provider with the attributes defined above as 'aggregate_attrs'
+          # and tries to add some hosts and then to remove some hosts.
+          # the hosts will be filtered against known active hosts and the expectations
+          # described above are the actual tests that check the provider's behaviour
+
+          provider.create
+          property_hash = provider.instance_variable_get(:@property_hash)
+          property_hash[:hosts] = ['known']
+          provider.instance_variable_set(:@property_hash, property_hash)
+
+          provider.hosts = ['known', 'known_too', 'unknown']
+          property_hash = provider.instance_variable_get(:@property_hash)
+          property_hash[:hosts] = ['known', 'known_too']
+          provider.instance_variable_set(:@property_hash, property_hash)
+
+          provider.hosts = ['known_too']
+
+        end
+      end
+
+    end
+  end
+
 end

spec/unit/type/nova_aggregate_spec.rb

@@ -30,4 +30,13 @@ describe Puppet::Type.type(:nova_aggregate) do
     }
     expect { Puppet::Type.type(:nova_aggregate).new(incorrect_input) }.to raise_error(Puppet::ResourceError, /availability zone must be a String/)
   end
+
+  it 'should raise error if non-boolean-y input for filter_hosts' do
+    incorrect_input = {
+      :name => 'new_aggr',
+      :filter_hosts => 'some non boolean-y value',
+    }
+    expect { Puppet::Type.type(:nova_aggregate).new(incorrect_input) }.to raise_error(Puppet::ResourceError, /Valid values are true, false/)
+  end
+
 end