openstack
/
cookbook-openstackclient
Code Issues Proposed changes

Migrate to using converge_by and fix idempotency issues 

- Migrate away from using the log resource to using converge_by. This finally
  fixes idempotency issues we were having with all of these resources
- Fix :delete action in openstack_domain by disabling the domain prior to
  destroying it
- Add missing references to new_resource parameters
- Fix :delete action in openstack_endpoint resource
- Add domain_name property in openstack_project resource. This is required and
  would throw a deprecation warning in the backend system.
- Fix logic around user grant/revoke actions
- Cookstyle fixes

Depends-On: https://review.opendev.org/c/openstack/openstack-chef/+/838415
Change-Id: I33601eb5595c794a9a025417ed3bc88cfa6cfaf0
Signed-off-by: Lance Albertson <lance@osuosl.org>
This commit is contained in:
Lance Albertson 2022-04-18 16:49:28 -07:00
parent 9276352d02
commit 2402afd670
22 changed files with 215 additions and 257 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
Rakefile
View File

@ -1,6 +1,6 @@
#
# Copyright:: 2016-2021, cloudbau GmbH
# Copyright:: 2016-2022, cloudbau GmbH
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.

4
libraries/openstack_base.rb
View File

@ -1,7 +1,7 @@
#
# Copyright:: 2016-2021, cloudbau GmbH
# Copyright:: 2019-2021, Oregon State University
# Copyright:: 2016-2022, cloudbau GmbH
# Copyright:: 2019-2022, Oregon State University
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.

19
libraries/openstack_domain.rb
View File

@ -1,6 +1,6 @@
#
# Copyright:: 2016-2021, cloudbau GmbH
# Copyright:: 2016-2022, cloudbau GmbH
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
@ -28,21 +28,20 @@ module OpenstackclientCookbook
action :create do
domain = new_resource.connection.domains.find { |u| u.id == new_resource.domain_name } ||
new_resource.connection.domains.find { |u| u.name == new_resource.domain_name }
if domain
log "Domain with name: \"#{new_resource.domain_name}\" already exists"
else
new_resource.connection.domains.create(
name: new_resource.domain_name
)
unless domain
converge_by "creating domain #{new_resource.domain_name}" do
new_resource.connection.domains.create(name: new_resource.domain_name)
end
end
end
action :delete do
domain = new_resource.connection.domains.find { |u| u.name == new_resource.domain_name }
if domain
domain.destroy
else
log "Domain with name: \"#{new_resource.domain_name}\" doesn't exist"
converge_by "deleting domain #{new_resource.domain_name}" do
domain.update(enabled: false)
domain.destroy
end
end
end
end

32
libraries/openstack_endpoint.rb
View File

@ -1,6 +1,6 @@
#
# Copyright:: 2016-2021, cloudbau GmbH
# Copyright:: 2016-2022, cloudbau GmbH
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
@ -35,28 +35,28 @@ module OpenstackclientCookbook
e.service_id == service.id && e.interface == new_resource.interface
end
if endpoint
log "#{new_resource.interface}_endpoint for \"#{new_resource.service_name}\" already exists"
else
new_resource.connection.endpoints.create(
interface: new_resource.interface,
url: new_resource.url,
service_id: service.id,
name: new_resource.endpoint_name,
region: new_resource.region
)
unless endpoint
converge_by "creating endpoint #{new_resource.endpoint_name}" do
new_resource.connection.endpoints.create(
interface: new_resource.interface,
url: new_resource.url,
service_id: service.id,
name: new_resource.endpoint_name,
region: new_resource.region
)
end
end
end
action :delete do
service = new_resource.connection.services.find { |s| s.name == service_name }
service = new_resource.connection.services.find { |s| s.name == new_resource.service_name }
endpoint = new_resource.connection.endpoints.find do |e|
e.service_id == service.id && e.interface == interface
e.service_id == service.id && e.interface == new_resource.interface
end
if endpoint
new_resource.connection.endpoints.destroy(endpoint.id)
else
log "#{new_resource.interface}_endpoint for \"#{new_resource.service_name}\" doesn't exist"
converge_by "deleting endpoint #{new_resource.endpoint_name}" do
endpoint.destroy
end
end
end
end

42
libraries/openstack_project.rb
View File

@ -1,6 +1,6 @@
#
# Copyright:: 2016-2021, cloudbau GmbH
# Copyright:: 2016-2022, cloudbau GmbH
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
@ -21,25 +21,45 @@ module OpenstackclientCookbook
provides :openstack_project
property :project_name, String, name_property: true
property :domain_name, String, default: 'Default'
property :connection_params, Hash, required: true
default_action :create
action :create do
project = new_resource.connection.projects.find { |p| p.name == new_resource.project_name }
if project
log "Project with name: \"#{new_resource.project_name}\" already exists"
else
new_resource.connection.projects.create name: new_resource.project_name
domain = new_resource.connection.domains.find { |d| d.name == new_resource.domain_name }
project = new_resource.connection.projects.find do |p|
(p.name == new_resource.project_name) && (domain ? p.domain_id == domain.id : {})
end
if !project && domain
converge_by "creating project #{new_resource.project_name} in domain #{new_resource.domain_name}" do
new_resource.connection.projects.create(
name: new_resource.project_name,
domain_id: domain.id
)
end
elsif !project
converge_by "creating project #{new_resource.project_name}" do
new_resource.connection.projects.create(
name: new_resource.project_name
)
end
end
end
action :delete do
project = new_resource.connection.projects.find { |p| p.name == new_resource.project_name }
if project
project.destroy
else
log "Project with name: \"#{new_resource.project_name}\" doesn't exist"
domain = new_resource.connection.domains.find { |d| d.name == new_resource.domain_name }
project = new_resource.connection.projects.find do |p|
(p.name == new_resource.project_name) && (domain ? p.domain_id == domain.id : {})
end
if project && domain
converge_by "deleting project #{new_resource.project_name} in domain #{new_resource.domain_name}" do
project.destroy
end
elsif project
converge_by "deleting project #{new_resource.project_name}" do
project.destroy
end
end
end
end

16
libraries/openstack_role.rb
View File

@ -1,6 +1,6 @@
#
# Copyright:: 2016-2021, cloudbau GmbH
# Copyright:: 2016-2022, cloudbau GmbH
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
@ -27,19 +27,19 @@ module OpenstackclientCookbook
action :create do
role = new_resource.connection.roles.find { |r| r.name == new_resource.role_name }
if role
log "Role with name: \"#{new_resource.role_name}\" already exists"
else
new_resource.connection.roles.create name: new_resource.role_name
unless role
converge_by "creating role #{new_resource.role_name}" do
new_resource.connection.roles.create name: new_resource.role_name
end
end
end
action :delete do
role = new_resource.connection.roles.find { |r| r.name == new_resource.role_name }
if role
role.destroy
else
log "Role with name: \"#{new_resource.role_name}\" doesn't exist"
converge_by "deleting role #{new_resource.role_name}" do
role.destroy
end
end
end
end

22
libraries/openstack_service.rb
View File

@ -1,6 +1,6 @@
#
# Copyright:: 2016-2021, cloudbau GmbH
# Copyright:: 2016-2022, cloudbau GmbH
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
@ -28,22 +28,22 @@ module OpenstackclientCookbook
action :create do
service = new_resource.connection.services.find { |s| s.name == new_resource.service_name }
if service
log "Service with name: \"#{new_resource.service_name}\" already exists"
else
new_resource.connection.services.create(
name: new_resource.service_name,
type: new_resource.type
)
unless service
converge_by "creating service #{new_resource.service_name}" do
new_resource.connection.services.create(
name: new_resource.service_name,
type: new_resource.type
)
end
end
end
action :delete do
service = new_resource.connection.services.find { |s| s.name == new_resource.service_name }
if service
service.destroy
else
log "Service with name: \"#{new_resource.service_name}\" doesn't exist"
converge_by "creating service #{new_resource.service_name}" do
service.destroy
end
end
end
end

69
libraries/openstack_user.rb
View File

@ -1,5 +1,5 @@
#
# Copyright:: 2016-2021, cloudbau GmbH
# Copyright:: 2016-2022, cloudbau GmbH
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
@ -38,23 +38,25 @@ module OpenstackclientCookbook
new_resource.user_name,
domain ? { domain_id: domain.id } : {}
).first
if user
log "User with name: \"#{new_resource.user_name}\" already exists"
elsif domain
new_resource.connection.users.create(
name: new_resource.user_name,
domain_id: domain.id,
email: new_resource.email,
default_project_id: project ? project.id : nil,
password: new_resource.password
)
else
new_resource.connection.users.create(
name: new_resource.user_name,
email: new_resource.email,
default_project_id: project ? project.id : nil,
password: new_resource.password
)
if !user && domain
converge_by "creating user #{new_resource.user_name} in domain #{new_resource.domain_name}" do
new_resource.connection.users.create(
name: new_resource.user_name,
domain_id: domain.id,
email: new_resource.email,
default_project_id: project ? project.id : nil,
password: new_resource.password
)
end
elsif !user
converge_by "creating user #{new_resource.user_name}" do
new_resource.connection.users.create(
name: new_resource.user_name,
email: new_resource.email,
default_project_id: project ? project.id : nil,
password: new_resource.password
)
end
end
end
@ -65,9 +67,9 @@ module OpenstackclientCookbook
domain ? { domain_id: domain.id } : {}
).first
if user
user.destroy
else
log "User with name: \"#{new_resource.user_name}\" doesn't exist"
converge_by "deleting user #{new_resource.user_name}" do
user.destroy
end
end
end
@ -82,7 +84,11 @@ module OpenstackclientCookbook
new_resource.user_name,
domain ? { domain_id: domain.id } : {}
).first
project.grant_role_to_user role.id, user.id if role && project && user
if user && role && domain && project && !user.check_role(role.id)
converge_by "granting role #{new_resource.role_name} to user #{new_resource.user_name}" do
project.grant_role_to_user role.id, user.id
end
end
end
action :revoke_role do
@ -95,7 +101,12 @@ module OpenstackclientCookbook
(p.name == new_resource.project_name) && (domain ? p.domain_id == domain.id : {})
end
role = new_resource.connection.roles.find { |r| r.name == new_resource.role_name }
project.revoke_role_from_user role.id, user.id if role && project && user
if user && role && project && user.check_role(role.id)
converge_by "revoking role #{new_resource.role_name} to user #{new_resource.user_name}" do
project.revoke_role_from_user role.id, user.id
end
end
end
# Grant a role in a domain
@ -109,7 +120,11 @@ module OpenstackclientCookbook
domain ? { domain_id: domain.id } : {}
).first
role = new_resource.connection.roles.find { |r| r.name == new_resource.role_name }
user.grant_role role.id if role && domain && user
if user && role && domain && !user.check_role(role.id)
converge_by "granting role #{new_resource.role_name} to user #{new_resource.user_name} in domain #{new_resource.domain_name}" do
user.grant_role role.id
end
end
end
action :revoke_domain do
@ -119,7 +134,11 @@ module OpenstackclientCookbook
domain ? { domain_id: domain.id } : {}
).first
role = new_resource.connection.roles.find { |r| r.name == new_resource.role_name }
user.revoke_role role.id if role && domain && user
if user && role && domain && user.check_role(role.id)
converge_by "revoking role #{new_resource.role_name} to user #{new_resource.user_name} in domain #{new_resource.domain_name}" do
user.revoke_role role.id
end
end
end
end
end

2
spec/cookbooks/openstackclient_test/metadata.rb
View File

@ -1,6 +1,6 @@
#
# Copyright:: 2016-2021, cloudbau GmbH
# Copyright:: 2016-2022, cloudbau GmbH
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.

2
spec/cookbooks/openstackclient_test/recipes/domain.rb
View File

@ -1,6 +1,6 @@
#
# Copyright:: 2016-2021, cloudbau GmbH
# Copyright:: 2016-2022, cloudbau GmbH
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.

2
spec/cookbooks/openstackclient_test/recipes/endpoint.rb
View File

@ -1,6 +1,6 @@
#
# Copyright:: 2016-2021, cloudbau GmbH
# Copyright:: 2016-2022, cloudbau GmbH
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.

2
spec/cookbooks/openstackclient_test/recipes/project.rb
View File

@ -1,6 +1,6 @@
#
# Copyright:: 2016-2021, cloudbau GmbH
# Copyright:: 2016-2022, cloudbau GmbH
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.

2
spec/cookbooks/openstackclient_test/recipes/role.rb
View File

@ -1,6 +1,6 @@
#
# Copyright:: 2016-2021, cloudbau GmbH
# Copyright:: 2016-2022, cloudbau GmbH
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.

2
spec/cookbooks/openstackclient_test/recipes/service.rb
View File

@ -1,6 +1,6 @@
#
# Copyright:: 2016-2021, cloudbau GmbH
# Copyright:: 2016-2022, cloudbau GmbH
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.

2
spec/cookbooks/openstackclient_test/recipes/user.rb
View File

@ -1,6 +1,6 @@
#
# Copyright:: 2016-2021, cloudbau GmbH
# Copyright:: 2016-2022, cloudbau GmbH
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.

31
spec/domain_spec.rb
View File

@ -1,6 +1,6 @@
#
# Copyright:: 2016-2021, cloudbau GmbH
# Copyright:: 2016-2022, cloudbau GmbH
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
@ -34,6 +34,7 @@ describe 'openstackclient_test::domain' do
let(:found_domain) do
double :find,
update: { enabled: false },
destroy: true
end
@ -68,18 +69,6 @@ describe 'openstackclient_test::domain' do
expect(chef_run).to delete_openstack_domain('mydomain')
end
it do
expect(chef_run).to write_log(
'Domain with name: "mydomain" doesn\'t exist'
)
end
it do
expect(chef_run).not_to write_log(
'Domain with name: "mydomain" already exists'
)
end
it do
expect(domains_empty).to receive(:create)
.with(name: 'mydomain')
@ -116,23 +105,15 @@ describe 'openstackclient_test::domain' do
expect(chef_run).to delete_openstack_domain('mydomain')
end
it do
expect(chef_run).not_to write_log(
'Domain with name: "mydomain" doesn\'t exist'
)
end
it do
expect(chef_run).to write_log(
'Domain with name: "mydomain" already exists'
)
end
it do
expect(domains_populated).not_to receive(:create)
chef_run
end
it do
expect(found_domain).to receive(:update).with(enabled: false)
chef_run
end
it do
expect(found_domain).to receive(:destroy)
chef_run

37
spec/endpoint_spec.rb
View File

@ -1,6 +1,6 @@
#
# Copyright:: 2016-2021, cloudbau GmbH
# Copyright:: 2016-2022, cloudbau GmbH
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
@ -37,11 +37,17 @@ describe 'openstackclient_test::endpoint' do
find: nil
end
let(:found_endpoint) do
double :find,
id: 2,
destroy: true
end
let(:endpoints_populated) do
double :endpoints,
create: true,
destroy: true,
find: double(id: 2)
find: found_endpoint
end
context 'no endpoints defined' do
@ -67,18 +73,6 @@ describe 'openstackclient_test::endpoint' do
end
%w(public internal admin).each do |interface|
it do
expect(chef_run).to write_log(
"#{interface}_endpoint for \"myservice\" doesn't exist"
)
end
it do
expect(chef_run).not_to write_log(
"#{interface}_endpoint for \"myservice\" already exists"
)
end
it do
expect(chef_run).to create_openstack_endpoint("myendpoint_#{interface}")
end
@ -125,18 +119,6 @@ describe 'openstackclient_test::endpoint' do
chef_run
end
%w(public internal admin).each do |interface|
it do
expect(chef_run).not_to write_log(
"#{interface}_endpoint for \"myservice\" doesn't exist"
)
end
it do
expect(chef_run).to write_log(
"#{interface}_endpoint for \"myservice\" already exists"
)
end
it do
expect(chef_run).to create_openstack_endpoint("myendpoint_#{interface}")
end
@ -158,8 +140,7 @@ describe 'openstackclient_test::endpoint' do
end
it do
expect(endpoints_populated).to receive(:destroy)
.with(2)
expect(found_endpoint).to receive(:destroy)
chef_run
end
end

52
spec/project_spec.rb
View File

@ -1,6 +1,6 @@
#
# Copyright:: 2016-2021, cloudbau GmbH
# Copyright:: 2016-2022, cloudbau GmbH
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
@ -25,6 +25,26 @@ describe 'openstackclient_test::project' do
runner.converge(described_recipe)
end
let(:domains_empty) do
double :domains,
create: true,
destroy: true,
find: nil
end
let(:found_domain) do
double :find,
update: { enabled: false },
destroy: true
end
let(:domains_populated) do
double :domains,
create: true,
destroy: true,
find: found_domain
end
let(:projects_empty) do
double :projects,
create: true,
@ -47,7 +67,8 @@ describe 'openstackclient_test::project' do
context 'no projects defined' do
let(:connection_dub) do
double :fog_connection,
projects: projects_empty
projects: projects_empty,
domains: domains_empty
end
before do
@ -68,18 +89,6 @@ describe 'openstackclient_test::project' do
expect(chef_run).to delete_openstack_project('myproject')
end
it do
expect(chef_run).to write_log(
'Project with name: "myproject" doesn\'t exist'
)
end
it do
expect(chef_run).not_to write_log(
'Project with name: "myproject" already exists'
)
end
it do
expect(projects_empty).to receive(:create)
.with(
@ -97,7 +106,8 @@ describe 'openstackclient_test::project' do
context 'projects defined' do
let(:connection_dub) do
double :fog_connection,
projects: projects_populated
projects: projects_populated,
domains: domains_populated
end
before do
@ -118,18 +128,6 @@ describe 'openstackclient_test::project' do
expect(chef_run).to delete_openstack_project('myproject')
end
it do
expect(chef_run).not_to write_log(
'Project with name: "myproject" doesn\'t exist'
)
end
it do
expect(chef_run).to write_log(
'Project with name: "myproject" already exists'
)
end
it do
expect(projects_empty).not_to receive(:create)
chef_run

26
spec/role_spec.rb
View File

@ -1,6 +1,6 @@
#
# Copyright:: 2016-2021, cloudbau GmbH
# Copyright:: 2016-2022, cloudbau GmbH
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
@ -68,18 +68,6 @@ describe 'openstackclient_test::role' do
expect(chef_run).to delete_openstack_role('myrole')
end
it do
expect(chef_run).to write_log(
'Role with name: "myrole" doesn\'t exist'
)
end
it do
expect(chef_run).not_to write_log(
'Role with name: "myrole" already exists'
)
end
it do
expect(roles_empty).to receive(:create)
.with(
@ -118,18 +106,6 @@ describe 'openstackclient_test::role' do
expect(chef_run).to delete_openstack_role('myrole')
end
it do
expect(chef_run).not_to write_log(
'Role with name: "myrole" doesn\'t exist'
)
end
it do
expect(chef_run).to write_log(
'Role with name: "myrole" already exists'
)
end
it do
expect(roles_empty).not_to receive(:create)
chef_run

26
spec/service_spec.rb
View File

@ -1,6 +1,6 @@
#
# Copyright:: 2016-2021, cloudbau GmbH
# Copyright:: 2016-2022, cloudbau GmbH
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
@ -68,18 +68,6 @@ describe 'openstackclient_test::service' do
expect(chef_run).to delete_openstack_service('myservice')
end
it do
expect(chef_run).to write_log(
'Service with name: "myservice" doesn\'t exist'
)
end
it do
expect(chef_run).not_to write_log(
'Service with name: "myservice" already exists'
)
end
it do
expect(services_empty).to receive(:create)
.with(
@ -119,18 +107,6 @@ describe 'openstackclient_test::service' do
expect(chef_run).to delete_openstack_service('myservice')
end
it do
expect(chef_run).not_to write_log(
'Service with name: "myservice" doesn\'t exist'
)
end
it do
expect(chef_run).to write_log(
'Service with name: "myservice" already exists'
)
end
it do
expect(services_empty).not_to receive(:create)
chef_run

2
spec/spec_helper.rb
View File

@ -1,6 +1,6 @@
#
# Copyright:: 2016-2021, cloudbau GmbH
# Copyright:: 2016-2022, cloudbau GmbH
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.

78
spec/user_spec.rb
View File

@ -1,6 +1,6 @@
#
# Copyright:: 2016-2021, cloudbau GmbH
# Copyright:: 2016-2022, cloudbau GmbH
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
@ -37,6 +37,16 @@ describe 'openstackclient_test::user' do
id: 4,
destroy: true,
grant_role: true,
check_role: false,
revoke_role: true
end
let(:found_user_revoke) do
double :find,
id: 4,
destroy: true,
grant_role: true,
check_role: true,
revoke_role: true
end
@ -47,6 +57,13 @@ describe 'openstackclient_test::user' do
find_by_name: [ found_user ]
end
let(:users_populated_revoke) do
double :user,
create: true,
destroy: true,
find_by_name: [ found_user_revoke ]
end
let(:found_role) do
double :find,
id: 3
@ -121,18 +138,6 @@ describe 'openstackclient_test::user' do
expect(chef_run).to delete_openstack_user('myuser')
end
it do
expect(chef_run).to write_log(
'User with name: "myuser" doesn\'t exist'
)
end
it do
expect(chef_run).not_to write_log(
'User with name: "myuser" already exists'
)
end
it do
expect(users_empty).to receive(:create)
.with(
@ -193,18 +198,6 @@ describe 'openstackclient_test::user' do
expect(chef_run).to delete_openstack_user('myuser')
end
it do
expect(chef_run).not_to write_log(
'User with name: "myuser" doesn\'t exist'
)
end
it do
expect(chef_run).to write_log(
'User with name: "myuser" already exists'
)
end
it do
expect(chef_run).to grant_role_openstack_user('myuser')
end
@ -237,22 +230,37 @@ describe 'openstackclient_test::user' do
chef_run
end
it do
expect(found_project).to receive(:revoke_role_from_user)
.with(3, 4)
chef_run
end
it do
expect(found_user).to receive(:grant_role)
.with(3)
chef_run
end
it do
expect(found_user).to receive(:revoke_role)
.with(3)
chef_run
context 'revoked' do
let(:connection_dub) do
double :fog_connection,
users: users_populated_revoke,
domains: domains_populated,
roles: roles_populated,
projects: projects_populated
end
before do
allow_any_instance_of(OpenstackclientCookbook::OpenstackUser)
.to receive(:connection).and_return(connection_dub)
end
it do
expect(found_user_revoke).to receive(:revoke_role)
.with(3)
chef_run
end
it do
expect(found_project).to receive(:revoke_role_from_user)
.with(3, 4)
chef_run
end
end
end
end