Merge "Hash domains by name"

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