From b7e9437a77ea0f1c31fb0d5e8bbcede859bdbe5a Mon Sep 17 00:00:00 2001 From: Kamjar Gerami Date: Fri, 4 Dec 2015 10:05:07 +0100 Subject: [PATCH] #3539 - Fixed syntax, language and logic according to comments made by reviewer in PR #6639 --- .../provisioners/ansible/provisioner/base.rb | 14 ++++++------- templates/locales/en.yml | 2 +- .../provisioners/ansible/provisioner_test.rb | 3 +-- .../v2/provisioning/ansible_intro.html.md | 21 ++++++------------- 4 files changed, 15 insertions(+), 25 deletions(-) diff --git a/plugins/provisioners/ansible/provisioner/base.rb b/plugins/provisioners/ansible/provisioner/base.rb index cf88266f6..291df8213 100644 --- a/plugins/provisioners/ansible/provisioner/base.rb +++ b/plugins/provisioners/ansible/provisioner/base.rb @@ -12,6 +12,8 @@ module VagrantPlugins class Base < Vagrant.plugin("2", :provisioner) + RANGE_PATTERN = %r{(?:\[[a-z]:[a-z]\]|\[[0-9]+?:[0-9]+?\])}.freeze + protected def initialize(machine, config) @@ -113,9 +115,7 @@ module VagrantPlugins inventory_groups = "" # Verify if host range patterns exist and warn - if config.groups.any? do |gm| - gm.to_s[/(?:\[[a-z]:[a-z]\]|\[[0-9]+?:[0-9]+?\])/] - end + if config.groups.any? { |gm| gm.to_s[RANGE_PATTERN] } @machine.ui.warn(I18n.t("vagrant.provisioners.ansible.ansible_host_pattern_detected")) end @@ -141,10 +141,10 @@ module VagrantPlugins defined_groups << gname inventory_groups += "\n[#{gname}]\n" gmembers.each do |gm| - # TODO : Expand and validate host range patterns - # against @inventory_machines list before adding them - # otherwise abort with an error message - if gm[/(?:\[[a-z]:[a-z]\]|\[[0-9]+?:[0-9]+?\])/] + # TODO : Expand and validate host range patterns + # against @inventory_machines list before adding them + # otherwise abort with an error message + if gm[RANGE_PATTERN] inventory_groups += "#{gm}\n" end inventory_groups += "#{gm}\n" if @inventory_machines.include?(gm.to_sym) diff --git a/templates/locales/en.yml b/templates/locales/en.yml index 6fbd4ee79..e3382002d 100755 --- a/templates/locales/en.yml +++ b/templates/locales/en.yml @@ -2066,7 +2066,7 @@ en: ansible: ansible_host_pattern_detected: |- - Vagrant has detected a host range pattern in the ansible.groups option. + Vagrant has detected a host range pattern in the groups option. Vagrant doesn't fully check the validity of these parameters! Please check http://docs.ansible.com/ansible/intro_inventory.html#hosts-and-groups diff --git a/test/unit/plugins/provisioners/ansible/provisioner_test.rb b/test/unit/plugins/provisioners/ansible/provisioner_test.rb index 1fec0faff..c97cdb5ed 100644 --- a/test/unit/plugins/provisioners/ansible/provisioner_test.rb +++ b/test/unit/plugins/provisioners/ansible/provisioner_test.rb @@ -310,7 +310,6 @@ VF expect(inventory_content).to include("[group6]\n#{machine.name}\n\n") # Accept host range patterns - expect(inventory_content).to match(/(?:\[[a-z]:[a-z]\]|\[[0-9]+?:[0-9]+?\])/) expect(inventory_content).to include("[group4]\nmachine[1:2]\nmachine[a:f]\n") # Don't mix group names and host names @@ -343,7 +342,7 @@ VF # Single string syntax expect(inventory_content).to include("[group3:vars]\nstringvar1=stringvalue1\nstringvar2=stringvalue2\n") - } + } end end diff --git a/website/docs/source/v2/provisioning/ansible_intro.html.md b/website/docs/source/v2/provisioning/ansible_intro.html.md index 6871030d3..1db717912 100644 --- a/website/docs/source/v2/provisioning/ansible_intro.html.md +++ b/website/docs/source/v2/provisioning/ansible_intro.html.md @@ -156,8 +156,8 @@ Vagrant.configure(2) do |config| ansible.groups = { "group1" => ["machine1"], "group2" => ["machine2"], - "group3" => ["machine[01:50]"], - "group4" => ["machine-[a:d]"], + "group3" => ["machine[1:2]"], + "group4" => ["other_node-[a:d]"], "all_groups:children" => ["group1", "group2"], "group1:vars" => {"variable1" => 9, "variable2" => "example"} @@ -173,14 +173,6 @@ Vagrant would generate an inventory file that might look like: machine1 ansible_ssh_host=127.0.0.1 ansible_ssh_port=2200 ansible_ssh_user='vagrant' ansible_ssh_private_key_file='/home/.../.vagrant/machines/machine1/virtualbox/private_key' machine2 ansible_ssh_host=127.0.0.1 ansible_ssh_port=2222 ansible_ssh_user='vagrant' ansible_ssh_private_key_file='/home/.../.vagrant/machines/machine2/virtualbox/private_key' -machine01 ansible_ssh_host=127.0.0.1 ansible_ssh_port=2223 ansible_ssh_user='vagrant' ansible_ssh_private_key_file='/home/.../.vagrant/machines/machine01/virtualbox/private_key' -machine02 ansible_ssh_host=127.0.0.1 ansible_ssh_port=2224 ansible_ssh_user='vagrant' ansible_ssh_private_key_file='/home/.../.vagrant/machines/machine02/virtualbox/private_key' -machine03 ansible_ssh_host=127.0.0.1 ansible_ssh_port=2225 ansible_ssh_user='vagrant' ansible_ssh_private_key_file='/home/.../.vagrant/machines/machine03/virtualbox/private_key' -machine04 ansible_ssh_host=127.0.0.1 ansible_ssh_port=2226 ansible_ssh_user='vagrant' ansible_ssh_private_key_file='/home/.../.vagrant/machines/machine04/virtualbox/private_key' -machine-a ansible_ssh_host=127.0.0.1 ansible_ssh_port=2227 ansible_ssh_user='vagrant' ansible_ssh_private_key_file='/home/.../.vagrant/machines/machine-a/virtualbox/private_key' -machine-b ansible_ssh_host=127.0.0.1 ansible_ssh_port=2228 ansible_ssh_user='vagrant' ansible_ssh_private_key_file='/home/.../.vagrant/machines/machine-b/virtualbox/private_key' -machine-c ansible_ssh_host=127.0.0.1 ansible_ssh_port=2229 ansible_ssh_user='vagrant' ansible_ssh_private_key_file='/home/.../.vagrant/machines/machine-c/virtualbox/private_key' -machine-d ansible_ssh_host=127.0.0.1 ansible_ssh_port=2230 ansible_ssh_user='vagrant' ansible_ssh_private_key_file='/home/.../.vagrant/machines/machine-d/virtualbox/private_key' [group1] machine1 @@ -189,10 +181,10 @@ machine1 machine2 [group3] -machine[01:50] +machine[1:2] [group4] -machine-[a:d] +other_node-[a:d] [all_groups:children] group1 @@ -208,10 +200,9 @@ variable2=example - Prior to Vagrant 1.7.3, the `ansible_ssh_private_key_file` variable was not set in generated inventory, but passed as command line argument to `ansible-playbook` command. - The generation of group variables blocks (e.g. `[group1:vars]`) is only possible since Vagrant 1.8.0. Note however that setting variables directly in the inventory is not the [preferred practice in Ansible](http://docs.ansible.com/intro_inventory.html#splitting-out-host-and-group-specific-data). If possible, group (or host) variables should be set in `YAML` files stored in the `group_vars/` or `host_vars/` directories in the playbook (or inventory) directory instead. - Unmanaged machines and undefined groups are not added to the inventory, to avoid useless Ansible errors (e.g. *unreachable host* or *undefined child group*) - - [Host range patterns (numeric and alphabetic ranges)](http://docs.ansible.com/ansible/intro_inventory.html#hosts-and-groups) will not be validated by vagrant. Hosts will be added as group members to the inventory anyway, this might lead to errors in Ansible (e.q *unreachable host*). + - [Host range patterns (numeric and alphabetic ranges)](http://docs.ansible.com/ansible/intro_inventory.html#hosts-and-groups) will not be validated by Vagrant. Hosts will be added as group members to the inventory anyway, this might lead to errors in Ansible (e.g *unreachable host*). -For example, `machine3`, `group3` and `group1:vars` in the example below would not be added to the generated inventory file, -`machine-[a:d]` and `machine[01:50]` would be (even thought there are no hosts named machine50): +For example, `machine3` and `group3` in the example below would not be added to the generated inventory file: ``` ansible.groups = {