Add methods for configuring and starting ansible inside #13584

carbonin · 2017-01-19T15:56:38Z

This PR mainly fleshes out the EmbeddedAnsible class created in #13435

This adds things like configuring the appliance to run the ansible service and methods for managing that service.

Also added here is a way to share the keys and passwords needed by the embedded ansible service across servers in case of role failover.

This is the general shape of the API that will be used by the worker being created in #13551

https://www.pivotaltracker.com/story/show/137048481

jrafanie · 2017-01-23T20:12:22Z

lib/embedded_ansible.rb

@@ -11,13 +17,114 @@ def self.enabled?
  end

  def self.running?
-    # TODO
-    false
+    services.map { |service| LinuxAdmin::Service.new(service).running? }.inject(:&)


Yeah, not sure why I did it that way ... all? is probably the better way to go ...

jrafanie · 2017-01-23T20:15:26Z

app/models/miq_database.rb

+
+    auth.password = password
+    auth.save!
+  end


Maybe not for this PR, but the repetition of the ansible_ prefix for the methods and the hardcoded authtypes make me think we should have a ansible thing mixed in here or a separate class so we can have constants and less code here. Maybe we can start with making constants out of the hardcoded values?

Yes, seems weird to have so many "ansible" specific references in MiqDatabase. It may seem like overkill for now, but maybe a new model for "appliance services" or something like that where ansible could be on of them. Just spitballing here...

jrafanie · 2017-01-23T20:19:39Z

lib/embedded_ansible.rb

+      rabbitmq_vhost=tower
+      rabbitmq_username=tower
+      rabbitmq_password='#{rabbitmq_password}'
+      rabbitmq_cookie=cookiemonster


lol, that's their default, right?

Yup, pulled this all from the default file 😄

jrafanie · 2017-01-23T20:25:45Z

lib/embedded_ansible.rb

+    with_inventory_file do |inventory_file_path|
+      setup_params = {
+        :e => "minimum_var_space=0",
+        :k => "packages,migrations,supervisor",


Would it make sense to put this list into a explaining constant/variable/method so it's obvious why one has supervisor in the list and the one below doesn't?

jrafanie · 2017-01-23T20:28:50Z

lib/embedded_ansible.rb

+    end
+  end
+
+  def self.stop


Does it make sense to name this method "disable"? I think we can assume disabling a service should stop it but I don't think you can assume stopping a service should disable it.

👍 Do you think we should also have a .stop?

jrafanie · 2017-01-23T20:37:57Z

lib/embedded_ansible.rb

+      file.close
+      yield(file.path)
+    ensure
+      file.unlink


I believe the file.close is done automatically if you use Tempfile.new with a block. Are you trying to avoid having the GC do the unlink through a finalizer?

In other words, why not:

Tempfile.open("miq_inventory") do |file| file.write(inventory_file_contents) yield(file.path) end

I feel like I ran into an issue where the contents of the tempfile were not synced to the filesystem unless I did the #close. So when we then do a File.read("temp/file/path") we didn't get the proper contents.

May have made that up though.

I had to do this extra #close to get the .configured? specs to pass, for example.

Interesting, the link is now dead. I'm ok with doing this if we really need make sure the file is gone. I think the difference is the unlink could be delayed until the Tempfile object is GC'd so the file could stay around longer than expected.

I think we would want this file gone ASAP as it will contain passwords.

These methods run the setup playbook with particular arguments to accomplish configuration and startup at different times. Also added stop and running? methods to control and monitor the services running on the server

These methods will be used to store authentication information and shared keys that will be used whenever we start the ansible role on a server.

https://www.pivotaltracker.com/story/show/137048481

This determines if the service is configured based on the status of the secret key.

This allows us to run nginx alongside apache withou hacking an edit on the configuration file at install time.

The firewall task opens the ports we specify to use for nginx. We want to control the appliance's firewall and we don't want nginx to be able to get through.

This avoids unpleasant quoting and escaping issues within the inventory file.

There's no reason for this to be in the console or elsewhere. We can manage the creation of the database the same way we manage the secret key. Create it if we think we need to. We are using the presence of the database password to determine if the database needs to be created.

The `.stop` method now just stops the services and the `.disable` method will stop then disable each service.

Also fix a brakeman falure by quoting the password for SQL when creating the awx role.

miq-bot · 2017-01-25T23:17:18Z

Checked commits carbonin/manageiq@d0873c5~...d277d40 with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0
4 files checked, 0 offenses detected
Everything looks good. 🏆

jrafanie · 2017-01-26T20:49:12Z

lib/embedded_ansible.rb

+  private_class_method :playbook_extra_variables
+
+  def self.run_setup_script(params)
+    with_inventory_file do |inventory_file_path|


Let's wrap this with a flock or something to avoid the possibility of the worker running more than one of these at once.

We can do this in a followup PR.

jrafanie

We pounded this PR through some manual test runs and found some minor things for follow up PRs but it's all good.

👍

carbonin added core enhancement labels Jan 19, 2017

carbonin assigned gtanzillo Jan 19, 2017

carbonin requested review from jrafanie and gtanzillo January 19, 2017 15:56

carbonin added the wip label Jan 19, 2017

miq-bot added the dependencies label Jan 19, 2017

carbonin force-pushed the ansible_inside branch 3 times, most recently from 9cbd33a to 1146f08 Compare January 20, 2017 22:04

jrafanie reviewed Jan 23, 2017

View reviewed changes

carbonin force-pushed the ansible_inside branch 3 times, most recently from 492343c to b6d6ae8 Compare January 24, 2017 14:21

carbonin mentioned this pull request Jan 24, 2017

Prerequisites for running ansible inside the appliance ManageIQ/manageiq-gems-pending#42

Merged

carbonin force-pushed the ansible_inside branch 4 times, most recently from 303bc69 to bcfb98e Compare January 25, 2017 14:46

carbonin mentioned this pull request Jan 25, 2017

Remove the awx database creation ManageIQ/manageiq-gems-pending#45

Merged

carbonin force-pushed the ansible_inside branch 3 times, most recently from 54ffac9 to b96d3f6 Compare January 25, 2017 22:14

carbonin added 2 commits January 25, 2017 18:10

Remove private .installed? used in only one place

d0873c5

Add additional methods to the EmbeddedAnsible class

d163bfd

These methods run the setup playbook with particular arguments to accomplish configuration and startup at different times. Also added stop and running? methods to control and monitor the services running on the server

carbonin added 12 commits January 25, 2017 18:10

Add ansible credential handling methods to MiqDatabase

9c93d80

These methods will be used to store authentication information and shared keys that will be used whenever we start the ansible role on a server.

Set and use saved authentications when configuring ansible service

1ff2509

https://www.pivotaltracker.com/story/show/137048481

Add EmbeddedAnsible.configured? method

396a785

This determines if the service is configured based on the status of the secret key.

Use the extra variables to specify the ports nginx should listen on

d5de23d

This allows us to run nginx alongside apache withou hacking an edit on the configuration file at install time.

Don't run the firewall task

5f23b6a

The firewall task opens the ports we specify to use for nginx. We want to control the appliance's firewall and we don't want nginx to be able to get through.

Use SecureRandom.hex over SecureRandom.base64

847c92e

This avoids unpleasant quoting and escaping issues within the inventory file.

Add ansible database password handling to MiqDatabase class

7261e12

Move ansible auth methods to a mixin and constantize the auth types

4b8d78f

DRY up ansible authentications mixin

b55ca89

Split the stop and disable functionality into separate methods

7e97c7e

The `.stop` method now just stops the services and the `.disable` method will stop then disable each service.

Use base64 instead of hex for passwords because security

d277d40

Also fix a brakeman falure by quoting the password for SQL when creating the awx role.

carbonin force-pushed the ansible_inside branch from b96d3f6 to d277d40 Compare January 25, 2017 23:10

carbonin changed the title ~~[WIP] Add methods for configuring and starting ansible inside~~ Add methods for configuring and starting ansible inside Jan 26, 2017

carbonin removed dependencies wip labels Jan 26, 2017

jrafanie reviewed Jan 26, 2017

View reviewed changes

jrafanie approved these changes Jan 26, 2017

View reviewed changes

jrafanie merged commit b3ade89 into ManageIQ:master Jan 26, 2017

jrafanie added this to the Sprint 53 Ending Jan 30, 2017 milestone Jan 26, 2017

carbonin mentioned this pull request Feb 2, 2017

Create ansible tower provider mixin #13733

Merged

carbonin deleted the ansible_inside branch March 7, 2017 16:33

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add methods for configuring and starting ansible inside #13584

Add methods for configuring and starting ansible inside #13584

carbonin commented Jan 19, 2017

jrafanie Jan 23, 2017

carbonin Jan 23, 2017

jrafanie Jan 23, 2017

gtanzillo Jan 23, 2017

jrafanie Jan 23, 2017 •

edited

Loading

carbonin Jan 23, 2017

jrafanie Jan 23, 2017

jrafanie Jan 23, 2017

carbonin Jan 23, 2017

jrafanie Jan 23, 2017

carbonin Jan 23, 2017

carbonin Jan 24, 2017 •

edited

Loading

jrafanie Jan 24, 2017

carbonin Jan 24, 2017

miq-bot commented Jan 25, 2017

jrafanie Jan 26, 2017

jrafanie Jan 26, 2017

jrafanie left a comment

Add methods for configuring and starting ansible inside #13584

Add methods for configuring and starting ansible inside #13584

Conversation

carbonin commented Jan 19, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jrafanie Jan 23, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

carbonin Jan 24, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

miq-bot commented Jan 25, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jrafanie left a comment

Choose a reason for hiding this comment

jrafanie Jan 23, 2017 •

edited

Loading

carbonin Jan 24, 2017 •

edited

Loading