-
Notifications
You must be signed in to change notification settings - Fork 305
[Provisioner] Vagrant setup script #268
base: master
Are you sure you want to change the base?
Conversation
Tested on Windows 10 and this is working very nice. For Windows, the following instructions are important:
Procedure for Windows (but works equally well for Linux)
mkdir lightbulb
cd lightbulb
wget -o Vagrantfile https://github.com/ansible/lightbulb/blob/master/tools/vagrant_lab_setup/Vagrantfile
vagrant up
vagrant ssh And everything should be up and running ! You can now run the below and all hosts should be addressable out-of-the-box: yum install -y ansible
ansible -m ping all |
We now have two options here to make it work identically on Linux and Windows (and macOS)
Both would work fine. |
readonly workshop_user=vagrant | ||
|
||
readonly workshop_key="/home/${workshop_user}/.ssh/id_rsa" | ||
readonly workshop_inventory="/home/${workshop_user}/hosts.ini" |
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.
I think this will conflict with #265 that @dagwieers submitted.
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.
Agreed, but merge this one first and I'll resolve the conflicts...
|
||
[defaults] | ||
|
||
inventory = ./hosts.ini |
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.
Likewise #264 conflict?
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.
Agreed, but merge this one first and I'll resolve the conflicts...
Vagrantfile
Outdated
install_ansible_centos() { | ||
info "CentOS: installing Ansible from the EPEL repository" | ||
yum -y install epel-release | ||
yum -y install git ansible |
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.
The first Ansible Essentials workshop assignment is to install ansible
itself -- by putting this function in we are making that pointless. I want users of this content to experience install ansible and how easy it is.
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.
That's fine, I agree.
Ok, the Ansible installation was removed from the script. |
LGTM, can we merge this ? |
@bertvv and @dagwieers: I was starting to merge this, but this doesn't seem to work properly. First time I run
I think the last commit didn't go far enough removing ansible from being installed. I find some other issues I was going to fix after merging. When I rerun Given this hiccup, I'm going to do another code review to address those to see if we can address those before merging. |
@tima The removal of ansible from the installation process broke some other stuff. We tested this thoroughly before that last commit. Shouldn't be too hard to get the other ansible references removed. |
|
||
} | ||
|
||
# Ansible installation |
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 seems like cruft with the automatic installation of ansible removed now.
ansible --version | ||
} | ||
|
||
is_ansible_installed() { |
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 seems like cruft with the automatic installation of ansible removed now.
fi | ||
} | ||
|
||
# Install Ansible on a Fedora system. |
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.
We aren't supporting Fedora at this time. Please remove.
yum -y install git | ||
} | ||
|
||
# Install Ansible on a recent Ubuntu distribution, from the PPA |
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.
We aren't supporting Ubuntu at this time. Please remove.
[defaults] | ||
|
||
inventory = ./hosts.ini | ||
forks = 50 |
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 is too high for this lab environment (I know that in the source material from another use case) -- it should be 10 or removed entirely.
"--memory", "#$NODEMEM", | ||
"--cpus", 1 | ||
] | ||
vb.customize ['modifyvm', :id, '--groups', '/lightbulb_workshop'] |
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.
Question why "ligthbulb_workshop"? I'd prefer just sticking with "lightbulb".
|
||
# Install Ansible on a CentOS system from EPEL | ||
install_ansible_centos() { | ||
info "CentOS: installing Ansible from the EPEL repository" |
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.
We're not installing Ansible.
main() { | ||
info "Bootstrapping Ansible on host ${HOSTNAME}." | ||
|
||
ensure_ansible_installed |
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.
Function seems misnamed now.
# | ||
|
||
main() { | ||
info "Bootstrapping Ansible on host ${HOSTNAME}." |
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 should read something like "Bootstrapping Lightbulb lab environment on host..." since we aren't installing Ansible.
elif [ -f '/etc/lsb-release' ]; then | ||
|
||
# Debian-based distributions | ||
grep DISTRIB_ID '/etc/lsb-release' \ |
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.
We are only supporting centos7 and RHEL7 at this time.
@dagwieers: Yes I figure as much, but it was enough that it stopped me from merging at the moment to work with you to get it straight. |
ansible ansible_host=10.42.0.2 ansible_connection=local | ||
|
||
[web] | ||
node-1 ansible_host=10.42.0.6 |
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.
I'd add integration with vagrant-hostmanager
plugin instead of hardcoding IPs
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.
Thanks for the suggestion. These fixed static IP addresses are used throughout the content that we want to keep it simple and static like this to demonstrate the static inventory features of Ansible.
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.
Yes, but I had to track all occurrences of them once, when I had some issues with conflicting networking. I'd not want to do this again as patching vagrantfile makes it harder to sync with upstream etc. IMHO this should be easily configurable, but now it's painful sometimes, as opposed to "just works" approach.
@@ -6,6 +6,196 @@ $NODEMEM=256 | |||
# Overwrite host locale in ssh session |
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.
I'd move changable options to a separate config like in https://github.com/openstack-dev/devstack-vagrant/blob/master/Vagrantfile#L7-L12
It's very useful to not have to edit local copy of vagrantfile and/or track down all the places where the value is hardcoded
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.
Something to consider though we are trying to keep this experience as straight forward and simple as possible. We purposefully do not want to add too many knobs and switches to avoid issues getting to the task at hand learning how to automate with Ansible. By curl
'ing a single Vagrantfile followed by a vagrant up
then vagrant ssh
it can't get much easier than that.
Ideally the whole bootstrap process would be done by Ansible and not the bash script and embedded files process that @bertvv and @dagwieers are introducing here, but in streamlining this process it makes the most sense.
Hope that reasoning makes sense.
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.
Sounds good, but it won't make it more complicated for user if there would be sane defaults and it will work without config, but support changing things in the independent file seamlessly.
You're talking about public switches, but if they're not explicitly advertised to users it shouldn't be harmful and will enable ones who needs to really do changes make them in a comfortable way.
@@ -47,7 +237,9 @@ Vagrant.configure("2") do |cluster| | |||
"--memory", "2048", |
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.
--name
fails for me when I cloned the fork to other folder and tried to have a separate set of VMs.
I would suggest adding support for some (configurable) prefixes.
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.
I'll let @bertvv and @dagwieers and others weigh in on this idea. Perhaps an ENV variable with a default value? Again, we want to err on the side of making this as simple and "foolproof" as possible to get up and running. Our goal is to teach Ansible automation skill, not Vagrant ones. ;)
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.
Right, env var or config with defaults is what I'd expect. Consider supporting dotenv in case of env vars.
fi | ||
|
||
info "Ansible version" | ||
ansible --version |
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 fails, because ansible is not being installed during the flow.
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.
Right -- that's the line I hacked out to test the rest of this. I trust @dagwieers and @bertvv will pull that out when they get around to addressing the issues that have been raised.
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.
There's mention of using vi or vim
in the second guide. only vi
is available. it's better to install vim
in vagrantfile as well.
This looks cluttered to me. Why not create a lightbulb box with Packer and work with that? download Vagrantfile any way you like...
|
@webknjaz Adding vim would be a good thing -- I actually thought centos7 had that sort of knowledge isn't my thing. There are a few other tools we install in the AWS provisioned boxes I'd like to see added to the list (wget, nano, tree, python-pip) in addition to some other VM definitions that go dropped during renovations (ansible/tower, a ubuntu web node for testing multi-os roles, and a haproxy VM for doing a rolling update exercise/demo like #158). Feel free to file an enhancement issue that we can come back to. @bbaassssiiee Good point. Definitely something to consider since that would streamline this process at the expense of some addition maintenance and bandwidth. Feel free to file an enhancement issue for us to revisit after we get thru some of these other things here. |
@tima why is it labeled |
@webknjaz Windows was in reference to this: #268 (comment) |
@bertvv this now needs conflict resolution |
pinging @bertvv and @dagwieers |
@bertvv @dagwieers ping ;) |
I cannot make modifications to this branch. |
IMO We should have done this in Ghent when everyone was together and we were testing this. Now we lost momentum, it conflicts with other changes and things go stale. Short feedback loop wins. |
Noted. I agree with you @dagwieers and ideally we would have so this feedback loop would have been kept short. Unfortunately getting it done in Ghent in-person wasn't an option under the circumstances so we have to go from here. Do you have a better way of reaching @bertvv than here? Would it be better to start this PR over, inserting the init script features in the current Vagrantfile rather than attempt to resolve the conflicts? |
Hi, what is the status of this pull request. |
cc @dfederlein |
Lightbulb is currently not making any changes in regards to provisioning the labs/examples while we regroup and redesign. We will soon publish new materials on this and update the contributor guide accordingly. Basically, we'll be removing the provisioning, including vagrant, by separating provisioning out into a role that lives on galaxy so everyone can point to their own provisioner and use the content. We'll then only focus on the content and have spec sheets for the labs so people can design their provisioning accordingly. |
any updates ? |
This is some code to automate the setup of a Vagrant environment. After
vagrant up
, log in on the control node withvagrant ssh
. Ansible should be installed, as well as the lightbulb repository. The VMsnode-[123]
can immediately be controlled from the control node.