Hash domains by name

Currently every project and user resource generates a call to
fetch_domain, which is an unhashed query to the openstack domain show
command. This is very wasteful and does not scale well. This patch takes
the code that hashes domains by id and generates an inverse, a hash of
domains by name. This code is used in place of fetch_domain, which is
removed. Since the only thing used in this hash is the id, which should
be immutable, we should not have issues with the other fields changing
(like description and enabled).

This should provide a performance boost to users who have a lot of users
and projects, especially if everything is in the default domain.

Co-Authored-By: Denis Egorenko <degorenko@mirantis.com>

Change-Id: I99d8fe272aedc0cbc6ad48561ea50a7b7d6cdb1e

lib/puppet/provider/keystone.rb

@@ -111,10 +111,17 @@ class Puppet::Provider::Keystone < Puppet::Provider::Openstack
     @domain_hash[id]
   end
 
-  def self.fetch_domain(domain)
-    request('domain', 'show', domain)
-  rescue Puppet::ExecutionFailure => e
-    raise e unless e.message =~ /No domain with a name or ID/
+  def self.domain_id_from_name(name)
+    unless @domain_hash_name
+      list = request('domain', 'list')
+      @domain_hash_name = Hash[list.collect{|domain| [domain[:name], domain[:id]]}]
+    end
+    unless @domain_hash_name.include?(name)
+      id = request('domain', 'show', name)[:id]
+      err("Could not find domain with name [#{name}]") unless id
+      @domain_hash_name[name] = id
+    end
+    @domain_hash_name[name]
   end
 
   def self.fetch_project(name, domain)
@@ -214,8 +221,7 @@ class Puppet::Provider::Keystone < Puppet::Provider::Openstack
     if domain_name.nil? || domain_name.empty?
       raise(Puppet::Error, "Missing domain name for resource #{name}")
     end
-    domain = fetch_domain(domain_name)
-    domain_id = domain ? domain[:id] : nil
+    domain_id = self.domain_id_from_name(domain_name)
     case domain_id
     when default_domain_id
       name

spec/unit/provider/keystone_spec.rb

@@ -33,26 +33,32 @@ describe Puppet::Provider::Keystone do
     another_class.reset
   end
 
-  describe '#fetch_domain' do
-    it 'should be false if the domain does not exist' do
-      # retry only once.  Not doing this make the test unnecessary
-      # long (1 minute) and retry the command ~20times
-      klass.expects(:request_timeout).returns(0)
+  describe '#domain_id_from_name' do
+    it 'should list all domains when requesting a domain name from an ID' do
       klass.expects(:openstack)
-        .with('domain', 'show', '--format', 'shell', 'no_domain')
-        .times(2)
-        .raises(Puppet::ExecutionFailure, "Execution of '/usr/bin/openstack domain show --format shell no_domain' returned 1: No domain with a name or ID of 'no_domain' exists.")
-      expect(klass.fetch_domain('no_domain')).to be_falsey
-    end
-
-    it 'should return the domain' do
-      klass.expects(:openstack)
-        .with('domain', 'show', '--format', 'shell', 'The Domain')
-        .returns('
-name="The Domain"
-id="the_domain_id"
+           .with('domain', 'list', '--quiet', '--format', 'csv', [])
+           .returns('"ID","Name","Enabled","Description"
+"someid","SomeName",True,"default domain"
 ')
-      expect(klass.fetch_domain('The Domain')).to eq({:name=>"The Domain", :id=>"the_domain_id"})
+      expect(klass.domain_id_from_name('SomeName')).to eq('someid')
+    end
+    it 'should lookup a domain when not found in the hash' do
+      klass.expects(:openstack)
+           .with('domain', 'show', '--format', 'shell', 'NewName')
+           .returns('
+name="NewName"
+id="newid"
+')
+      expect(klass.domain_id_from_name('NewName')).to eq('newid')
+    end
+    it 'should print an error when there is no such domain' do
+      klass.expects(:openstack)
+           .with('domain', 'show', '--format', 'shell', 'doesnotexist')
+           .returns('
+')
+      klass.expects(:err)
+           .with('Could not find domain with name [doesnotexist]')
+      expect(klass.domain_id_from_name('doesnotexist')).to eq(nil)
     end
   end