-
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
[4.0][neutron][Cisco ACI]: Multiple VMM domain support (SOC - 10471) #2228
base: stable/4.0
Are you sure you want to change the base?
Conversation
This commit provides changes in plugin packages and config files needed for integration of SOC with ACI 4.1 and higher versions. ACI 4.1 uses a slightly different set of plugin packages and configs for integration with OpenStack. This includes: - python-gbpclient renamed to python-group-based-policy-client - ovs-bridge-name in opflex-agent-ovs.conf removed - addition of int-bridge-name and access-bridge-name in opflex-agent-ovs.conf - Renaming of agent-ovs to opflex-agent For uniformity, the template for opflex-agent-ovs.conf is now renamed from 10-opflex-agent-ovs.conf.erb to opflex-agent-ovs.conf.erb - The neutron template schema and json templates are updated to provide integration_bridge and access_bridge details with default values. The corresponding migration scripts are also updated. (cherry picked from commit cb5347d)
spec/neutron_l3_ha_service_spec.rb
Outdated
puts "MIGRATING AGENTS" | ||
sleep 1000 | ||
end | ||
EOF |
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.
Naming/HeredocDelimiterNaming: Use meaningful heredoc delimiters. (https://github.com/bbatsov/ruby-style-guide#heredoc-delimiters)
spec/neutron_l3_ha_service_spec.rb
Outdated
exit 0 | ||
end | ||
exit 2 if ARGV.include? "--l3-agent-check" | ||
EOF |
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.
Naming/HeredocDelimiterNaming: Use meaningful heredoc delimiters. (https://github.com/bbatsov/ruby-style-guide#heredoc-delimiters)
spec/neutron_l3_ha_service_spec.rb
Outdated
exit 0 | ||
end | ||
exit 2 if ARGV.include? "--l3-agent-check" | ||
EOF |
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.
Naming/HeredocDelimiterNaming: Use meaningful heredoc delimiters. (https://github.com/bbatsov/ruby-style-guide#heredoc-delimiters)
spec/neutron_l3_ha_service_spec.rb
Outdated
with_tmpdir do |tmpdir| | ||
tmpdir.write_script "somescript", <<-EOF | ||
puts "hi" | ||
EOF |
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.
Naming/HeredocDelimiterNaming: Use meaningful heredoc delimiters. (https://github.com/bbatsov/ruby-style-guide#heredoc-delimiters)
spec/neutron_l3_ha_service_spec.rb
Outdated
STDOUT.flush | ||
STDERR.flush | ||
sleep 1000 | ||
EOF |
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.
Naming/HeredocDelimiterNaming: Use meaningful heredoc delimiters. (https://github.com/bbatsov/ruby-style-guide#heredoc-delimiters)
= select_field %w(ha storage mode), :collection => :ha_storage_mode_for_rabbitmq, "data-showit" => ["drbd", "shared"].join(";"), "data-showit-target" => "#drbd_storage_container;#shared_storage_container", "data-showit-direct" => "true" | ||
= boolean_field %w(cluster), "data-hideit" => "true", "data-hideit-target" => "#ha_storage_container", "data-hideit-direct" => "true" | ||
|
||
#ha_storage_container |
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.
id
attribute must be in lisp-case
= boolean_field %w(cluster), "data-hideit" => "true", "data-hideit-target" => "#ha_storage_container", "data-hideit-direct" => "true" | ||
|
||
#ha_storage_container | ||
= select_field %w(ha storage mode), :collection => :ha_storage_mode_for_rabbitmq, "data-showit" => ["drbd", "shared"].join(";"), "data-showit-target" => "#drbd_storage_container;#shared_storage_container", "data-showit-direct" => "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.
Line is too long. [244/150]
|
||
def save_config_to_databag(old_role, role) | ||
Rails.logger.debug("#{@bc_name} save_config_to_databag: entering") | ||
if role.nil? |
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/ConditionalAssignment: Use the return of the conditional for variable assignment and comparison.
chef/roles/mysql-server.rb
Outdated
@@ -0,0 +1,5 @@ | |||
name "mysql-server" |
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.
Naming/FileName: The name of this source file (mysql-server.rb) should use snake_case. (https://github.com/bbatsov/ruby-style-guide#snake-case-files)
role.default_attributes["database"]["mysql"]["server_root_password"] = (old_role && old_role.default_attributes["database"]["mysql"]["server_root_password"]) || random_password | ||
role.default_attributes["database"]["mysql"]["server_repl_password"] = (old_role && old_role.default_attributes["database"]["mysql"]["server_repl_password"]) || random_password | ||
if db_enabled["mysql"]["ha"] | ||
role.default_attributes["database"]["mysql"]["sstuser_password"] = (old_role && old_role.default_attributes["database"]["mysql"]["sstuser_password"]) || random_password |
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/LineLength: Line is too long. [176/100] (https://github.com/SUSE/style-guides/blob/master/Ruby.md#metricslinelength)
5d0b3ad
to
4ee7a94
Compare
@@ -0,0 +1,15 @@ | |||
def upgrade(tattr, tdep, attr, dep) | |||
unless attr["apic"].key?("apic_vmms") |
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 unless 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)
# distributed dhcp and metadata cannot work since these | ||
# functions conflict with vcenter functionality. | ||
if acivmms.find { |vmm| vmm[:vmm_type].downcase == "vmware"} | ||
apic_optimized_dhcp = 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.
Layout/TrailingWhitespace: Trailing whitespace detected. (https://github.com/bbatsov/ruby-style-guide#no-trailing-whitespace)
# If using VMWare vcenter as one of the compute hosts. | ||
# distributed dhcp and metadata cannot work since these | ||
# functions conflict with vcenter functionality. | ||
if acivmms.find { |vmm| vmm[:vmm_type].downcase == "vmware"} |
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.
Performance/Casecmp: Use casecmp instead of downcase ==. (https://github.com/JuanitoFatas/fast-ruby#stringcasecmp-vs-stringdowncase---code)
Layout/SpaceInsideBlockBraces: Space missing inside }.
@@ -32,6 +32,9 @@ | |||
default[:neutron][:metadata_agent_config_file] = "/etc/neutron/neutron-metadata-agent.conf.d/100-metadata_agent.conf" | |||
default[:neutron][:ml2_config_file] = "/etc/neutron/neutron.conf.d/110-ml2.conf" | |||
default[:neutron][:nsx_config_file] = "/etc/neutron/neutron.conf.d/110-nsx.conf" | |||
default[:neutron][:ml2_cisco_config_file] = "/etc/neutron/neutron.conf.d/115-ml2_cisco.conf" | |||
default[:neutron][:ml2_cisco_apic_config_file] = "/etc/neutron/neutron.conf.d/115-ml2_cisco_apic.conf" |
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/LineLength: Line is too long. [102/100] (https://github.com/SUSE/style-guides/blob/master/Ruby.md#metricslinelength)
A Single ACI fabric can support multiple VMM domains. Each VMM domain can be governed by a different controller (Eg: VMWare vCenter or OpenStack or MicroSoft SCVMM). Several production data centers tend to use multiple VMM domains and expect to be able to monitor and control network policies from a single ACI fabric. Integration of OpenStack with such a setup requires crowbar to provide parameters specific to each VMM domain. This commit adds the additional parameters and logic to validate and send these to the correct config location. The changes now allow to provide "Vmware" or "OpenStack" as the VMM type. Multiple entries of either types are possible. - Also added "ssl_mode" as a configurable parameter which is needed to be in "encrypted" mode if ESXi is used as compute. Other use-cases may need to change it as required and hence included it as a configurable parameter within the opflex node structure. (cherry picked from commit 1f16436)
4ee7a94
to
dacd4ee
Compare
@@ -0,0 +1,15 @@ | |||
def upgrade(tattr, tdep, attr, dep) | |||
unless attr["apic"].key?("apic_vmms") |
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 unless 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)
@@ -99,7 +99,7 @@ | |||
else | |||
cisco_apic_link_action = "delete" | |||
end | |||
link "/etc/neutron/neutron-server.conf.d/100-ml2_conf_cisco_apic.ini.conf" do | |||
link "#{node[:neutron][:platform][:ml2_cisco_apic_config_file]}" do |
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/UnneededInterpolation: Prefer to_s over string interpolation.
@@ -85,7 +85,7 @@ | |||
else | |||
cisco_nexus_link_action = "delete" | |||
end | |||
link "/etc/neutron/neutron-server.conf.d/100-ml2_conf_cisco.ini.conf" do | |||
link "#{node[:neutron][:platform][:ml2_cisco_config_file]}" do |
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/UnneededInterpolation: Prefer to_s over string interpolation.
Backport of #2227 , not done yet. |
This PR enables the following feature for Cisco ACI:
Allows Crowbar configuration for enabling multiple VMM domain features for ACI. It was painful for the customer to change in the config file manually and avoid the chef-client from overriding the config. Both KVM and VMWare based VMM domains can be configured using this feature.
Each [apic_vmdom:<vmm_domain_name>] corresponds to a VMM configuration. In these sections, [apic] configurations can be overridden for more granular infrastructure sharing.
What is configured in the [apic] sharing will be the default used in case a more specific configuration is missing for the domain.
For example:
[apic_vmdom:soc_kvm_domain]
vlan_ranges=1000:2000
[apic_vmdom:soc_vmware_domain]
apic_vmm_type=vmware
In case of a VMWare based VMM domain, the respective VMM domain MUST be created in APIC prior to configuring in neutron. For KVM, neutron will create the VMM domain if not already created.
Note: The intended target of this PR is Cloud 7 and is updated here due to the standard process being followed for all PRs (master-update followed by cloud 7 backport). The tests were only done for Cloud 7 based deployments.