-
Notifications
You must be signed in to change notification settings - Fork 87
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Start moving away from search queries to static config items for cookbooks #718
Conversation
role.default_attributes["trove"]["keystone_instance"], | ||
"keystone" | ||
) | ||
insecure = insecure || keystone_config["insecure"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Style/SelfAssignment: Use self-assignment shorthand ||=. (https://github.com/bbatsov/ruby-style-guide#self-assignment)
# currently, there's no insecure setting | ||
insecure = false | ||
#use_ssl = role.default_attributes["trove"]["api"]["protocol"] == "https" | ||
#insecure = use_ssl && role.default_attributes["trove"]["ssl"]["insecure"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Style/LeadingCommentSpace: Missing space after #. (https://github.com/bbatsov/ruby-style-guide#hash-space)
else | ||
# currently, there's no insecure setting | ||
insecure = false | ||
#use_ssl = role.default_attributes["trove"]["api"]["protocol"] == "https" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Style/LeadingCommentSpace: Missing space after #. (https://github.com/bbatsov/ruby-style-guide#hash-space)
role.default_attributes["swift"]["keystone_instance"], | ||
"keystone" | ||
) | ||
insecure = insecure || keystone_config["insecure"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Style/SelfAssignment: Use self-assignment shorthand ||=. (https://github.com/bbatsov/ruby-style-guide#self-assignment)
role.default_attributes["sahara"]["keystone_instance"], | ||
"keystone" | ||
) | ||
insecure = insecure || keystone_config["insecure"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Style/SelfAssignment: Use self-assignment shorthand ||=. (https://github.com/bbatsov/ruby-style-guide#self-assignment)
# currently, there's no insecure setting | ||
insecure = false | ||
#use_ssl = role.default_attributes["sahara"]["api"]["protocol"] == "https" | ||
#insecure = use_ssl && role.default_attributes["sahara"]["ssl"]["insecure"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Style/LeadingCommentSpace: Missing space after #. (https://github.com/bbatsov/ruby-style-guide#hash-space)
else | ||
# currently, there's no insecure setting | ||
insecure = false | ||
#use_ssl = role.default_attributes["sahara"]["api"]["protocol"] == "https" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Style/LeadingCommentSpace: Missing space after #. (https://github.com/bbatsov/ruby-style-guide#hash-space)
user = role.default_attributes["rabbitmq"]["user"] | ||
password = role.default_attributes["rabbitmq"]["password"] | ||
# avoid duplicated / | ||
vhost = role.default_attributes["rabbitmq"]["vhost"].sub(/^\/*/, "") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Style/RegexpLiteral: Use %r around regular expression. (https://github.com/SUSE/style-guides/blob/master/Ruby.md#styleregexpliteral)
role.default_attributes["nova"]["keystone_instance"], | ||
"keystone" | ||
) | ||
insecure = insecure || keystone_config["insecure"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Style/SelfAssignment: Use self-assignment shorthand ||=. (https://github.com/bbatsov/ruby-style-guide#self-assignment)
role.default_attributes["neutron"]["keystone_instance"], | ||
"keystone" | ||
) | ||
insecure = insecure || keystone_config["insecure"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Style/SelfAssignment: Use self-assignment shorthand ||=. (https://github.com/bbatsov/ruby-style-guide#self-assignment)
role.default_attributes["manila"]["keystone_instance"], | ||
"keystone" | ||
) | ||
insecure = insecure || keystone_config["insecure"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Style/SelfAssignment: Use self-assignment shorthand ||=. (https://github.com/bbatsov/ruby-style-guide#self-assignment)
role.default_attributes["magnum"]["keystone_instance"], | ||
"keystone" | ||
) | ||
insecure = insecure || keystone_config["insecure"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Style/SelfAssignment: Use self-assignment shorthand ||=. (https://github.com/bbatsov/ruby-style-guide#self-assignment)
# currently, there's no insecure setting | ||
insecure = false | ||
#use_ssl = role.default_attributes["magnum"]["api"]["protocol"] == "https" | ||
#insecure = use_ssl && role.default_attributes["magnum"]["ssl"]["insecure"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Style/LeadingCommentSpace: Missing space after #. (https://github.com/bbatsov/ruby-style-guide#hash-space)
else | ||
# currently, there's no insecure setting | ||
insecure = false | ||
#use_ssl = role.default_attributes["magnum"]["api"]["protocol"] == "https" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Style/LeadingCommentSpace: Missing space after #. (https://github.com/bbatsov/ruby-style-guide#hash-space)
role.default_attributes["heat"]["keystone_instance"], | ||
"keystone" | ||
) | ||
insecure = insecure || keystone_config["insecure"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Style/SelfAssignment: Use self-assignment shorthand ||=. (https://github.com/bbatsov/ruby-style-guide#self-assignment)
role.default_attributes["glance"]["keystone_instance"], | ||
"keystone" | ||
) | ||
insecure = insecure || keystone_config["insecure"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Style/SelfAssignment: Use self-assignment shorthand ||=. (https://github.com/bbatsov/ruby-style-guide#self-assignment)
@logger.debug("Heat apply_role_post_chef_call: leaving") | ||
end | ||
|
||
def save_config_to_databag(old_role, role) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks like a perfect candidate for DRY
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To some extent, yes. But not completely as the path to the attributes is not always the same, and also because some barclamps will have more data in the config later on.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Improved things a bit.
Looking pretty good! |
chef/cookbooks/glance/recipes/api.rb
Outdated
cinder_api_insecure = cinder[:cinder][:api][:protocol] == "https" && cinder[:cinder][:ssl][:insecure] | ||
end | ||
swift_insecure = Config.load("openstack", "swift")["insecure"] || false | ||
cinder_insecure = Config.load("openstack", "cinder")["insecure"] || false |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lint/UselessAssignment: Useless assignment to variable - cinder_insecure. (https://github.com/bbatsov/ruby-style-guide#underscore-unused-vars)
chef/cookbooks/glance/recipes/api.rb
Outdated
cinder_api_insecure = cinder[:cinder][:api][:protocol] == "https" && cinder[:cinder][:ssl][:insecure] | ||
end | ||
swift_insecure = Config.load("openstack", "swift")["insecure"] || false | ||
cinder_insecure = Config.load("openstack", "cinder")["insecure"] || false |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lint/UselessAssignment: Useless assignment to variable - cinder_insecure. (https://github.com/bbatsov/ruby-style-guide#underscore-unused-vars)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, you're wrong...
chef/cookbooks/glance/recipes/api.rb
Outdated
cinder = cinders[0] | ||
cinder_api_insecure = cinder[:cinder][:api][:protocol] == "https" && cinder[:cinder][:ssl][:insecure] | ||
end | ||
swift_insecure = Config.load("openstack", "swift")["insecure"] || false |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lint/UselessAssignment: Useless assignment to variable - swift_insecure. (https://github.com/bbatsov/ruby-style-guide#underscore-unused-vars)
chef/cookbooks/glance/recipes/api.rb
Outdated
cinder = cinders[0] | ||
cinder_api_insecure = cinder[:cinder][:api][:protocol] == "https" && cinder[:cinder][:ssl][:insecure] | ||
end | ||
swift_insecure = Config.load("openstack", "swift")["insecure"] || false |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lint/UselessAssignment: Useless assignment to variable - swift_insecure. (https://github.com/bbatsov/ruby-style-guide#underscore-unused-vars)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, you're wrong...
"ceilometer_config_environment:#{node[:ceilometer][:config][:environment]}" | ||
) | ||
mongodb_nodes = node_search_with_cache("roles:ceilometer-server").select do |n| | ||
n[:ceilometer][:ha][:mongodb][:replica_set][:member] rescue false |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Style/RescueModifier: Avoid using rescue in its modifier form. (https://github.com/bbatsov/ruby-style-guide#no-rescue-modifiers)
"ceilometer_config_environment:#{node[:ceilometer][:config][:environment]}" | ||
) | ||
mongodb_nodes = node_search_with_cache("roles:ceilometer-server").select do |n| | ||
n[:ceilometer][:ha][:mongodb][:replica_set][:member] rescue false |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Style/RescueModifier: Avoid using rescue in its modifier form. (https://github.com/bbatsov/ruby-style-guide#no-rescue-modifiers)
"ceilometer_ha_mongodb_replica_set_member:true AND "\ | ||
"ceilometer_config_environment:#{node[:ceilometer][:config][:environment]}") | ||
members = node_search_with_cache("roles:ceilometer-server").select do |n| | ||
n[:ceilometer][:ha][:mongodb][:replica_set][:member] rescue false |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Style/RescueModifier: Avoid using rescue in its modifier form. (https://github.com/bbatsov/ruby-style-guide#no-rescue-modifiers)
"ceilometer_ha_mongodb_replica_set_member:true AND "\ | ||
"ceilometer_config_environment:#{node[:ceilometer][:config][:environment]}") | ||
members = node_search_with_cache("roles:ceilometer-server").select do |n| | ||
n[:ceilometer][:ha][:mongodb][:replica_set][:member] rescue false |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Style/RescueModifier: Avoid using rescue in its modifier form. (https://github.com/bbatsov/ruby-style-guide#no-rescue-modifiers)
"ceilometer_config_environment:#{node[:ceilometer][:config][:environment]}" | ||
) | ||
db_hosts = node_search_with_cache("roles:ceilometer-server").select do |n| | ||
n[:ceilometer][:ha][:mongodb][:replica_set][:member] rescue false |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Style/RescueModifier: Avoid using rescue in its modifier form. (https://github.com/bbatsov/ruby-style-guide#no-rescue-modifiers)
afa108d
to
f29c043
Compare
@@ -29,8 +29,7 @@ | |||
nova = node | |||
keystone_settings = KeystoneHelper.keystone_settings(node, @cookbook_name) | |||
|
|||
nova_insecure = node[:nova][:ssl][:insecure] | |||
ssl_insecure = keystone_settings["insecure"] || nova_insecure | |||
ssl_insecure = Config.load("openstack", "nova")["insecure"] || false |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Style/IndentationConsistency: Inconsistent indentation detected.
44c97ea
to
bc53167
Compare
Needs a rebase. |
openstack_args_nova = (nova_controller[:nova][:ssl][:enabled] && | ||
nova_controller[:nova][:ssl][:insecure]) || keystone_settings["insecure"] ? "--insecure" : "" | ||
nova_config = Barclamp::Config.load("openstack", "nova", node[:magnum][:nova_instance]) | ||
openstack_args_nova = (nova_config["insecure"] || false) ? "--insecure" : "" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Style/TernaryParentheses: Omit parentheses for ternary conditions.
Lint/LiteralInCondition: Literal false appeared in a condition.
openstack_args_glance = (glance_server[:glance][:api][:protocol] == "https" && | ||
glance_server[:glance][:ssl][:insecure]) || keystone_settings["insecure"] ? "--insecure" : "" | ||
glance_config = Barclamp::Config.load("openstack", "glance", node[:magnum][:glance_instance]) | ||
openstack_args_glance = (glance_config["insecure"] || false) ? "--insecure" : "" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Style/TernaryParentheses: Omit parentheses for ternary conditions.
Lint/LiteralInCondition: Literal false appeared in a condition.
module Openstack | ||
class DataBagConfig | ||
class << self | ||
def insecure(barclamp, role) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Metrics/CyclomaticComplexity: Cyclomatic complexity for insecure is too high. [9/6]
Metrics/PerceivedComplexity: Perceived complexity for insecure is too high. [10/7]
novacmd = "#{novacmd} --os-user-domain-name Default --os-project-domain-name Default" | ||
end | ||
|
||
flavors.keys.each do |id| |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Style/IndentationConsistency: Inconsistent indentation detected.
if ssl_insecure | ||
novacmd = "#{novacmd} --insecure" | ||
end | ||
if keystone_settings["api_version"] != "2.0" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Style/IndentationConsistency: Inconsistent indentation detected.
"vcpu"=>2, | ||
"disk"=>40, | ||
"mem"=>4096} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Style/MultilineHashBraceLayout: Closing hash brace must be on the same line as the last hash element when opening brace is on the same line as the first hash element.
{"name"=>"m1.trusted.medium", | ||
"vcpu"=>2, | ||
"disk"=>40, | ||
"mem"=>4096} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Style/SpaceAroundOperators: Surrounding space missing for operator =>. (https://github.com/bbatsov/ruby-style-guide#spaces-operators)
Style/SpaceInsideHashLiteralBraces: Space inside } missing. (https://github.com/bbatsov/ruby-style-guide#spaces-operators)
10=> | ||
{"name"=>"m1.trusted.medium", | ||
"vcpu"=>2, | ||
"disk"=>40, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Style/SpaceAroundOperators: Surrounding space missing for operator =>. (https://github.com/bbatsov/ruby-style-guide#spaces-operators)
"mem"=>2048}, | ||
10=> | ||
{"name"=>"m1.trusted.medium", | ||
"vcpu"=>2, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Style/SpaceAroundOperators: Surrounding space missing for operator =>. (https://github.com/bbatsov/ruby-style-guide#spaces-operators)
"disk"=>20, | ||
"mem"=>2048}, | ||
10=> | ||
{"name"=>"m1.trusted.medium", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Style/SpaceInsideHashLiteralBraces: Space inside { missing. (https://github.com/bbatsov/ruby-style-guide#spaces-operators)
Style/SpaceAroundOperators: Surrounding space missing for operator =>. (https://github.com/bbatsov/ruby-style-guide#spaces-operators)
All openstack services should provide this, and it happens quite a bit that other services only need this flag about another service. So it's a good candidate to start with.
We can check this out with the data bag config now.
aodh, barbican, ceilometer, magnum, trove and sahara
done |
@@ -35,7 +35,7 @@ def self.fetch_set_az_command_no_arg(node, cookbook_name) | |||
command << "--os-region-name" | |||
command << keystone_settings["endpoint_region"] | |||
|
|||
if keystone_settings["insecure"] || nova_insecure | |||
if ssl_insecure |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Style/IfUnlessModifier: Favor modifier if usage when having a single-line body. Another good alternative is the usage of control flow &&/||. (https://github.com/bbatsov/ruby-style-guide#if-as-a-modifier)
This has been splitted into individual commits and is being reworked again in #1143 so closing this one. |
See crowbar/crowbar-core#947 for the background.