nova_flavor: Add the new project_name property

The nova_flavor resource has been providing the project property which
accepts both project name and id. However this implementation results
in broken idempotency with project name used.

This change introduces a separate project_name property, so that users
can use project name with proper idempotency.

Closes-Bug: #1790795
Change-Id: Idee4af6931b8cf4a21d88f4cd38fe83468ec8efa

lib/puppet/provider/nova_flavor/openstack.rb

@@ -32,11 +32,26 @@ Puppet::Type.type(:nova_flavor).provide(
       prop_opts << props_to_s(@resource[:properties])
       self.class.request('flavor', 'set', prop_opts)
     end
+
     if @resource[:project] and @resource[:project] != ''
       proj_opts = [@resource[:name]]
       proj_opts << '--project' << @resource[:project]
       self.class.request('flavor', 'set', proj_opts)
+
+      project = self.class.request('project', 'show', @resource[:project])
+      @property_hash[:project_name] = project[:name]
+      @property_hash[:project] = project[:id]
+
+    elsif @resource[:project_name] and @resource[:project_name] != ''
+      proj_opts = [@resource[:name]]
+      proj_opts << '--project' << @resource[:project_name]
+      self.class.request('flavor', 'set', proj_opts)
+
+      project = self.class.request('project', 'show', @resource[:project_name])
+      @property_hash[:project_name] = project[:name]
+      @property_hash[:project] = project[:id]
     end
+
     @property_hash[:ensure] = :present
   end
 
@@ -73,6 +88,10 @@ Puppet::Type.type(:nova_flavor).provide(
     @project_flush[:project] = value
   end
 
+  def project_name=(value)
+    @project_flush[:project_name] = value
+  end
+
   def self.instances
     request('flavor', 'list', ['--long', '--all']).collect do |attrs|
       project = request('flavor', 'show', [attrs[:id], '-c', 'access_project_ids'])
@@ -89,20 +108,31 @@ Puppet::Type.type(:nova_flavor).provide(
         project_value = access_project_ids
       end
       project_value = project_value.gsub('\'', '')
+
+      if project_value != ''
+        project = request('project', 'show', project_value)
+        project_id = project[:id]
+        project_name = project[:name]
+      else
+        project_id = ''
+        project_name = ''
+      end
+
       properties = Hash[attrs[:properties].scan(/(\S+)='([^']*)'/)] rescue nil
       new(
-          :ensure      => :present,
+        :ensure       => :present,
-          :name        => attrs[:name],
+        :name         => attrs[:name],
-          :id          => attrs[:id],
+        :id           => attrs[:id],
-          :ram         => attrs[:ram],
+        :ram          => attrs[:ram],
-          :disk        => attrs[:disk],
+        :disk         => attrs[:disk],
-          :ephemeral   => attrs[:ephemeral],
+        :ephemeral    => attrs[:ephemeral],
-          :vcpus       => attrs[:vcpus],
+        :vcpus        => attrs[:vcpus],
-          :is_public   => attrs[:is_public].downcase.chomp == 'true'? true : false,
+        :is_public    => attrs[:is_public].downcase.chomp == 'true'? true : false,
-          :swap        => attrs[:swap],
+        :swap         => attrs[:swap],
-          :rxtx_factor => attrs[:rxtx_factor],
+        :rxtx_factor  => attrs[:rxtx_factor],
-          :properties  => properties,
+        :properties   => properties,
-          :project     => project_value
+        :project      => project_id,
+        :project_name => project_name,
       )
     end
   end
@@ -124,14 +154,26 @@ Puppet::Type.type(:nova_flavor).provide(
       self.class.request('flavor', 'set', opts)
       @property_flush.clear
     end
+
     unless @project_flush.empty?
-      opts = [@resource[:name]]
+      if @project_flush[:project]
-      unless @project_flush[:project] == ''
+        if @property_hash[:project] and @property_hash[:project] != ''
-        opts << '--project' << @project_flush[:project]
+          opts = [@resource[:name], '--project', @property_hash[:project]]
-        self.class.request('flavor', 'set', opts)
+          self.class.request('flavor', 'unset', opts)
-      else
+        end
-        opts << '--project' << @property_hash[:project]
+        if @project_flush[:project] != ''
-        self.class.request('flavor', 'unset', opts)
+          opts = [@resource[:name], '--project', @project_flush[:project]]
+          self.class.request('flavor', 'set', opts)
+        end
+      elsif @project_flush[:project_name]
+        if @property_hash[:project_name] and @property_hash[:project_name] != ''
+          opts = [@resource[:name], '--project', @property_hash[:project_name]]
+          self.class.request('flavor', 'unset', opts)
+        end
+        if @project_flush[:project_name] != ''
+          opts = [@resource[:name], '--project', @project_flush[:project_name]]
+          self.class.request('flavor', 'set', opts)
+        end
       end
       @project_flush.clear
     end

lib/puppet/type/nova_flavor.rb

@@ -44,9 +44,15 @@
 #    Optional
 #
 #  [*project*]
-#    Set flavor access to project (name or ID).
+#    Set flavor access to project (ID).
 #    If you set this option, take care to set is_public to false.
 #    Optional
+#
+#  [*project_name*]
+#    Set flavor access to project (name).
+#    If you set this option, take care to set is_public to false.
+#    Optional
+#
 require 'puppet'
 
 Puppet::Type.newtype(:nova_flavor) do
@@ -60,6 +66,10 @@ Puppet::Type.newtype(:nova_flavor) do
     ['nova::service::end']
   end
 
+  autorequire(:keystone_tenant) do
+    [self[:project_name]] if (self[:project_name] and self[:project_name] != '')
+  end
+
   newparam(:name, :namevar => true) do
     desc 'Name for the flavor'
     validate do |value|
@@ -116,8 +126,11 @@ Puppet::Type.newtype(:nova_flavor) do
   end
 
   newproperty(:project) do
-    desc 'Set flavor access to project (name or ID).'
+    desc 'Set flavor access to project (ID).'
-    defaultto('')
+  end
+
+  newproperty(:project_name) do
+    desc 'Set flavor access to project (Name).'
   end
 
   newproperty(:properties) do
@@ -144,6 +157,13 @@ Puppet::Type.newtype(:nova_flavor) do
     unless self[:name]
       raise(ArgumentError, 'Name must be set')
     end
+
+    if self[:project] && self[:project_name]
+      raise(Puppet::Error, <<-EOT
+Please provide a value for only one of project_name and project.
+EOT
+      )
+    end
   end
 
 end

releasenotes/notes/nova_flavor-project_name-fbab75b57ad9d4b3.yaml

@@ -0,0 +1,6 @@
+---
+features:
+  - |
+    The ``nova_flavor`` resource now supports the ``project_name`` property.
+    This property should be used instead of the ``project`` property when
+    project name is used instead of project id.

spec/acceptance/nova_wsgi_apache_spec.rb

@@ -23,14 +23,23 @@ describe 'basic nova' do
         require           => Class['nova::api'],
       }
 
-      nova_flavor { 'test_flavor':
+      nova_flavor { 'public_flavor':
-        ensure  => present,
+        ensure => present,
-        name    => 'test_flavor',
+        name   => 'public_flavor',
-        id      => '9999',
+        id     => '42',
-        ram     => '512',
+        ram    => '512',
-        disk    => '1',
+        disk   => '1',
-        vcpus   => '1',
+        vcpus  => '1',
-        require => [ Class['nova::api'], Class['nova::keystone::auth'] ],
+      }
+      nova_flavor { 'private_flavor':
+        ensure       => present,
+        name         => 'private_flavor',
+        id           => '43',
+        ram          => '512',
+        disk         => '1',
+        vcpus        => '1',
+        is_public    => 'False',
+        project_name => 'services'
       }
       EOS
 
@@ -72,7 +81,8 @@ describe 'basic nova' do
     describe 'nova flavor' do
       it 'should create new flavor' do
         command('openstack --os-identity-api-version 3 --os-username nova --os-password a_big_secret --os-tenant-name services --os-user-domain-name Default --os-project-domain-name Default --os-auth-url http://127.0.0.1:5000/v3 flavor list') do |r|
-          expect(r.stdout).to match(/test_flavor/)
+          expect(r.stdout).to match(/public_flavor/)
+          expect(r.stdout).to match(/private_flavor/)
         end
       end
     end

spec/unit/provider/nova_flavor/openstack_spec.rb

@@ -27,13 +27,11 @@ describe provider_class do
     end
 
     describe '#create' do
-      it 'creates flavor' do
+      context 'with defaults' do
-        provider.class.stubs(:openstack)
+        it 'creates flavor' do
-                      .with('flavor', 'list', ['--long', '--all'])
+          provider.class.expects(:openstack)
-                      .returns('"ID", "Name", "RAM", "Disk", "Ephemeral", "VCPUs", "Is Public", "Swap", "RXTX Factor", "Properties"')
-        provider.class.stubs(:openstack)
             .with('flavor', 'create', '--format', 'shell', ['example', '--public', '--id', '1', '--ram', '512', '--disk', '1', '--vcpus', '1'])
-                .returns('os-flv-disabled:disabled="False"
+            .returns('os-flv-disabled:disabled="False"
 os-flv-ext-data:ephemeral="0"
 disk="1"
 id="1"
@@ -43,8 +41,81 @@ ram="512"
 rxtx_factor="1.0"
 swap=""
 vcpus="1"')
-        provider.create
+          provider.create
-        expect(provider.exists?).to be_truthy
+          expect(provider.exists?).to be_truthy
+        end
+      end
+
+      context 'with project' do
+        before do
+          flavor_attrs.merge!(
+            :project => '3073e17b-fb7f-4524-bdcd-c54bc70e9da9'
+          )
+        end
+
+        it 'creates flavor' do
+          provider.class.expects(:openstack)
+            .with('flavor', 'create', '--format', 'shell', ['example', '--public', '--id', '1', '--ram', '512', '--disk', '1', '--vcpus', '1'])
+            .returns('os-flv-disabled:disabled="False"
+os-flv-ext-data:ephemeral="0"
+disk="1"
+id="1"
+name="example"
+os-flavor-access:is_public="True"
+ram="512"
+rxtx_factor="1.0"
+swap=""
+vcpus="1"')
+          provider.class.expects(:openstack)
+            .with('flavor', 'set', ['example', '--project', '3073e17b-fb7f-4524-bdcd-c54bc70e9da9'])
+          provider.class.expects(:openstack)
+            .with('project', 'show', '--format', 'shell', '3073e17b-fb7f-4524-bdcd-c54bc70e9da9')
+            .returns('enabled="True"
+name="admin"
+id="3073e17b-fb7f-4524-bdcd-c54bc70e9da9"
+domain_id="domain_one_id"
+')
+          provider.create
+          expect(provider.exists?).to be_truthy
+          expect(provider.project).to eq('3073e17b-fb7f-4524-bdcd-c54bc70e9da9')
+          expect(provider.project_name).to eq('admin')
+        end
+      end
+
+      context 'with project_name' do
+        before do
+          flavor_attrs.merge!(
+            :project_name => 'admin'
+          )
+        end
+
+        it 'creates flavor with project_name' do
+          provider.class.expects(:openstack)
+            .with('flavor', 'create', '--format', 'shell', ['example', '--public', '--id', '1', '--ram', '512', '--disk', '1', '--vcpus', '1'])
+            .returns('os-flv-disabled:disabled="False"
+os-flv-ext-data:ephemeral="0"
+disk="1"
+id="1"
+name="example"
+os-flavor-access:is_public="True"
+ram="512"
+rxtx_factor="1.0"
+swap=""
+vcpus="1"')
+          provider.class.expects(:openstack)
+            .with('flavor', 'set', ['example', '--project', 'admin'])
+          provider.class.expects(:openstack)
+            .with('project', 'show', '--format', 'shell', 'admin')
+            .returns('enabled="True"
+name="admin"
+id="3073e17b-fb7f-4524-bdcd-c54bc70e9da9"
+domain_id="domain_one_id"
+')
+          provider.create
+          expect(provider.exists?).to be_truthy
+          expect(provider.project).to eq('3073e17b-fb7f-4524-bdcd-c54bc70e9da9')
+          expect(provider.project_name).to eq('admin')
+        end
       end
     end
 
@@ -57,6 +128,26 @@ vcpus="1"')
         expect(provider.exists?).to be_falsey
       end
     end
+
+    describe '#flush' do
+      context '.project' do
+        it 'updates flavor' do
+          provider.class.expects(:openstack)
+            .with('flavor', 'set', ['example', '--project', '3073e17b-fb7f-4524-bdcd-c54bc70e9da9'])
+          provider.project = '3073e17b-fb7f-4524-bdcd-c54bc70e9da9'
+          provider.flush
+        end
+      end
+
+      context '.project_name' do
+        it 'updates flavor' do
+          provider.class.expects(:openstack)
+            .with('flavor', 'set', ['example', '--project', 'admin'])
+          provider.project_name = 'admin'
+          provider.flush
+        end
+      end
+    end
   end
 end