-
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
Support installation of Neutron ZVM Driver #27
Conversation
end | ||
|
||
def downgrade ta, td, a, d | ||
a.delete('zvm') |
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/StringLiterals: Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.
Beside the style offenses LGTM |
"zvm_xcat_server": { "type": "str", "required": true }, | ||
"zvm_xcat_username": { "type": "str", "required": true }, | ||
"zvm_xcat_password": { "type": "str", "required": true } | ||
}}, |
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.
will these 3 parameters then be taken out of nova? you have mentioned some day IIRC that nova could use these attributes from neutron, as neutron is required by nova anyway.
end | ||
|
||
def downgrade ta, td, a, d | ||
a.delete('zvm') |
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/StringLiterals: Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping. (https://github.com/bbatsov/ruby-style-guide#consistent-string-literals)
2764ed4
to
a994df7
Compare
mode "0640" | ||
variables( | ||
zvm: neutron[:neutron][:zvm] | ||
) |
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/ClosingParenthesisIndentation: Indent )
the same as the start of the line where (
is.
4f7db4e
to
9f71e0e
Compare
aac4462
to
24aa3ff
Compare
|
||
# XXX define the xcat management network when needed | ||
if true | ||
external_networks += ["xcatvsw1"] |
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.
test failed with
undefined method `to_hash' for nil:NilClass
in cookbooks/neutron/libraries/helpers.rb
13: external_networks.each do |net|
14>> networks[net] = node["network"]["networks"][net].to_hash
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.
not that it fixes the above error, but stylewise maybe use external_networks.push("xcatvsw1")
@@ -13,8 +13,14 @@ | |||
# limitations under the License. | |||
# | |||
|
|||
zvm_compute_node = search(:node, "roles:nova-compute-zvm") || [] | |||
use_zvm = not zvm_compute_node.empty? |
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.
unexpected token tIDENTIFIER
(Using Ruby 2.3 parser; configure using TargetRubyVersion
parameter, under AllCops
)
@@ -169,6 +179,9 @@ | |||
#TODO(vuntz): temporarily disable the hyperv mechanism since we're lacking networking-hyperv from stackforge | |||
#ml2_mechanism_drivers = node[:neutron][:ml2_mechanism_drivers].dup.push("hyperv") | |||
ml2_mechanism_drivers = node[:neutron][:ml2_mechanism_drivers].dup | |||
if zvm_compute_node.first |
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.
if use_zvm
...
+1 (assuming you tested it and it works) |
@@ -13,8 +13,14 @@ | |||
# limitations under the License. | |||
# | |||
|
|||
zvm_compute_node = search(:node, "roles:nova-compute-zvm") || [] | |||
use_zvm = !zvm_compute_node.empty? |
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.
In case the PR gets updated again (no need to update it just for that):
I know it doesn't change anything to the code path here, but for better readability, I would move the node[:neutron][:networking_plugin] == "ml2"
test to be part of use_zvm
.
@@ -112,6 +115,9 @@ | |||
# ML2 configuration: L2 agent and L3 agent | |||
if neutron[:neutron][:networking_plugin] == "ml2" | |||
ml2_mech_drivers = neutron[:neutron][:ml2_mechanism_drivers] | |||
if use_zvm && node.roles.include? "nova-compute-zvm" |
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.
unexpected token tSTRING
(Using Ruby 2.3 parser; configure using TargetRubyVersion
parameter, under AllCops
)
Allow deploying with the ZVM L2 driver.
pkgs = node[:neutron][:platform][:pkgs] + node[:neutron][:platform][:pkgs_fwaas] | ||
pkgs += node[:neutron][:platform][:pkgs_lbaas] if node[:neutron][:use_lbaas] | ||
if use_zvm | ||
pkgs << node[:neutron][:platform][:zvm_agent_pkg] if node[:neutron][:networking_plugin] == "ml2" |
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.
No need for the if node[:neutron][:networking_plugin] == "ml2"
anymore here.
I'm happy to see this merged and I can create a followup PR for the nitpicks if preferred. +1 |
LGTM +1 |
Support installation of Neutron ZVM Driver
|
+1 |
Remove unused grafana_group from cluster template
Remove unused grafana_group from cluster template
Remove unused grafana_group from cluster template
Remove unused grafana_group from cluster template
Remove unused grafana_group from cluster template
Remove unused grafana_group from cluster template
* Finish integration of Ansible installer * Set credentials in command line * Fix violations * Fix violations 2 * Add support for multiple networks in single node * Pass missing variables to ansible * Fix hound violations * Use monasca.yml as entry point for ansible installer * Fix paths and influxdb url * Remove agent nodes from inventory file * Remove default grafana config file (crowbar#22) * Fix suse (crowbar#23) * Register monasca endpoints in Keystone * Fix kibana url * Rearrange helper functions * Remove unused group (crowbar#27) * Remove unused grafana_group from cluster template * Disable deprecation_warnings for Ansible (crowbar#34) * They are outputted to STDERR and break applying cookbook. * Address review comments (crowbar#35)
* Finish integration of Ansible installer * Set credentials in command line * Fix violations * Fix violations 2 * Add support for multiple networks in single node * Pass missing variables to ansible * Fix hound violations * Use monasca.yml as entry point for ansible installer * Fix paths and influxdb url * Remove agent nodes from inventory file * Remove default grafana config file (crowbar#22) * Fix suse (crowbar#23) * Register monasca endpoints in Keystone * Fix kibana url * Rearrange helper functions * Remove unused group (crowbar#27) * Remove unused grafana_group from cluster template * Disable deprecation_warnings for Ansible (crowbar#34) * They are outputted to STDERR and break applying cookbook. * Address review comments (crowbar#35)
Allow deploying with the ZVM L2 driver.