Seed plugin ansible playbooks #17185

jrafanie · 2018-03-21T19:33:51Z

Clean the temp directory before each role activation
Consolidates all gem plugin ansible content
Creates git repo from this content
Creates or updates an existing ansible project for this git repo
- Links it to the default organization
- The project id is retained so we can find by the id to retrieve it in the future

ManageIQ/manageiq-design#40
https://bugzilla.redhat.com/show_bug.cgi?id=1539762

All of this depends on:
#17096
ManageIQ/manageiq-content#254

To test this on downstream appliances:

yum install git patch

vmdb
wget https://github.com/ManageIQ/ManageIQ/manageiq/pull/17096.patch
patch -p1 < 17096.patch

cd `bundle show manageiq-content`
wget https://github.com/ManageIQ/manageiq-content/pull/254.patch
patch -p1 < 254.patch

syncrou · 2018-03-21T20:02:16Z

app/models/embedded_ansible_worker/object_management.rb

+      # ruggedize this
+      `git init`
+      `git add -A`
+      `git commit -m "YOLO Initial Commit"`


@jrafanie - YOLO - nice!

jrafanie · 2018-03-22T22:14:57Z

@syncrou @gmcculloug @gtanzillo other than writing tests and doing cleanup, this is ready for high level review. Am I missing anything?

jrafanie · 2018-03-22T22:27:26Z

@carbonin too of course ^

carbonin

Looks good, just a few minor questions.

carbonin · 2018-03-23T13:08:27Z

app/models/embedded_ansible_worker/object_management.rb

+
+      options              = {}
+      options[:tree]       = index.write_tree(repo)
+      options[:author]     = options[:committer] = { :email => "[email protected]", :name => 'Author', :time => Time.now }


This is weird. Are the things like email required?

Can we make this use the product name or something? Not sure if this gets exposed in our UI anywhere.

Yeah, good question @syncrou @gmcculloug. Do we care?

@jrafanie - I don't think we care. I would just ask that is consistent, but largely because that seems the right thing to do.

On my testing, I received errors on the commit if the email wasn't set, but that obviously would not be a problem here.

I can try not specifying the author and commiter. On CLI, if there's not author/commit it just warns to STDERR but still commits.

Ugh, rugged requires both author and email for git commits 😞

irb(main):002:0> EmbeddedAnsibleWorker.new.ensure_plugin_playbooks_project_seeded(Provider.first, EmbeddedAnsible.new) Rugged::ConfigError: Config value 'user.name' was not found

carbonin · 2018-03-23T13:08:39Z

app/models/embedded_ansible_worker/object_management.rb

+      options              = {}
+      options[:tree]       = index.write_tree(repo)
+      options[:author]     = options[:committer] = { :email => "[email protected]", :name => 'Author', :time => Time.now }
+      options[:message]    = "YOLO Initial Commit"


Again, do we care? I can change it but I left something ridiculous so we remembered to change it.

Same - It doesn't matter, other than being consistent with whatever is picked.

carbonin · 2018-03-23T13:10:39Z

app/models/embedded_ansible_worker/object_management.rb

+    end
+  end
+
+  PLUGIN_PLAYBOOK_PROJECT_NAME = "Default ManageIQ Playbook Project".freeze


Does this get displayed in the UI? If so we should use the product name.

@syncrou @gmcculloug Yeah, I have no idea. This was me just naming it knowing it would change.

@jrafanie, @carbonin - Yes, This name would show up in the services 'repo' list, also in the Automate inline method 'repo' list. So we would want a descriptive name that makes sense to parties expecting to find their seeded playbooks.

@syncrou @gmcculloug naming is hard. Let me know what name you want. 🏃 🚌

I would stick with the convention from the other objects so something like "#{I18n.t("product.name")} Default Project"

carbonin · 2018-03-23T13:13:19Z

app/models/embedded_ansible_worker/object_management.rb

+  end
+
+  def create_playbook_project(connection)
+    connection.api.projects.create!(PLAYBOOK_PROJECT_ATTRIBUTES.to_json)


Does anyone else need this id? Should we save it against the provider like we do for the other initial objects?

@syncrou and I discussed this briefly and it didn't seem that this project would ever get accessed in manageiq after the fact, which is why we're searching by name for the "update" part. If we would need schema changes, we can't really backport that though.

We wouldn't need schema changes. These are built using custom attributes https://github.com/ManageIQ/manageiq/blob/master/app/models/manageiq/providers/embedded_ansible/provider/default_ansible_objects.rb

ok, @gmcculloug should I capture the project id for this?

@syncrou what do you think of this? I can store it if needed.

@jrafanie - I can't think of a reason we need it, if we can rebuild the entire system from the db when enabling the role on another machine (or upgrading ), then it seems to me that id is not needed at all.

jrafanie · 2018-03-23T14:10:18Z

app/models/embedded_ansible_worker/object_management.rb

+
+    #TODO: make this a public api via an attr_reader
+    Vmdb::Plugins.instance.instance_variable_get(:@registered_ansible_content).each do |content|
+      FileUtils.cp_r(Dir.glob("#{content.path}/*"), CONSOLIDATED_PLUGIN_PLAYBOOKS_TEMPDIR)


I think this is wrong. I need to know what namespaced_role_directories and namespaced_playbook_files means so we don't have collisions on content. @syncrou do you have examples of what these values should be for the manageiq-content repo?

@jrafanie - They're namespaced by a ., and that namespace will be completely up to us. e.g. we currently plan on prefixing all miq-core seeded roles with miq-core as the naming is shown here

The namespace collision protection is a documentation and PR reviewer issue. Ansible doesn't care ( mostly ) what the names of the roles are, so we're duplicating what galaxy uses to separate out roles with potential name collisions.

@syncrou

Note, I'm copying from the content.path down. It's assuming that the roles_path is a child directory of the path. I don't know if this is a valid assumption.

irb(main):001:0> Vmdb::Plugins.instance.instance_variable_get(:@registered_ansible_content) => [#<Vmdb::Plugins::AnsibleContent:0x00000001303a58 @roles_path=#<Pathname:/opt/rh/cfme-gemset/bundler/gems/cfme-content-e5c494e59a7b/content/ansible/roles>, @path=#<Pathname:/opt/rh/cfme-gemset/bundler/gems/cfme-content-e5c494e59a7b/content/ansible>>]

I can instead copy them as two separate operations if needed but it was easier to just do cp_r so I don't need to copy recursively by myself.

With this said, this is how the current CONSOLIDATED_PLUGIN_PLAYBOOKS_TEMPDIR looks after this method is complete. It appears to be namespace in that the original filenames are namespaced. As long as this is the expectation, I think we're ok here. Right?

[root@localhost vmdb]# tree /var/lib/awx_consolidated_source/projects/gem_ansible_content/ /var/lib/awx_consolidated_source/projects/gem_ansible_content/ └── roles ├── miq-core.manageiq-automate │ ├── action_plugins │ │ └── manageiq_automate.py │ ├── defaults │ │ └── main.yml │ ├── examples │ ├── handlers │ │ └── main.yml │ ├── library │ │ └── manageiq_automate.py │ ├── LICENSE │ ├── meta │ │ └── main.yml │ ├── README.md │ ├── tasks │ │ └── main.yml │ ├── tests │ │ ├── inventory │ │ └── test.yml │ └── vars │ └── main.yml └── miq-core.manageiq-vmdb ├── action_plugins │ └── manageiq_vmdb.py ├── defaults │ └── main.yml ├── handlers │ └── main.yml ├── library │ └── manageiq_vmdb.py ├── LICENSE ├── meta │ └── main.yml ├── README.md ├── tasks │ └── main.yml ├── tests │ ├── inventory │ └── test.yml └── vars └── main.yml

@jrafanie - That all looks right. If there were playbooks added to this from other repos we would place them under the content.path as well.

Ok, sounds good. Instead of guessing, I'll keep it as is and change when/if we have to. As long as the gems follow the rules and namespace themselves, this should just work.

jrafanie · 2018-03-23T14:10:57Z

app/models/embedded_ansible_worker/object_management.rb

+
+      options              = {}
+      options[:tree]       = index.write_tree(repo)
+      options[:author]     = options[:committer] = { :email => "[email protected]", :name => 'Author', :time => Time.now }


Yeah, good question @syncrou @gmcculloug. Do we care?

jrafanie · 2018-03-23T14:11:47Z

app/models/embedded_ansible_worker/object_management.rb

+      options              = {}
+      options[:tree]       = index.write_tree(repo)
+      options[:author]     = options[:committer] = { :email => "[email protected]", :name => 'Author', :time => Time.now }
+      options[:message]    = "YOLO Initial Commit"


Again, do we care? I can change it but I left something ridiculous so we remembered to change it.

jrafanie · 2018-03-23T14:12:25Z

app/models/embedded_ansible_worker/object_management.rb

+    end
+  end
+
+  PLUGIN_PLAYBOOK_PROJECT_NAME = "Default ManageIQ Playbook Project".freeze


@syncrou @gmcculloug Yeah, I have no idea. This was me just naming it knowing it would change.

jrafanie · 2018-03-23T14:13:57Z

app/models/embedded_ansible_worker/object_management.rb

+  end
+
+  def create_playbook_project(connection)
+    connection.api.projects.create!(PLAYBOOK_PROJECT_ATTRIBUTES.to_json)


@syncrou and I discussed this briefly and it didn't seem that this project would ever get accessed in manageiq after the fact, which is why we're searching by name for the "update" part. If we would need schema changes, we can't really backport that though.

syncrou · 2018-03-26T17:47:25Z

app/models/embedded_ansible_worker/object_management.rb

+
+    commit_git_plugin_content
+
+    if project = existing_plugin_playbook_project(connection)


@jrafanie did you mean to use == here?

No. I can split the assignment and conditional to appease the rubocop.

syncrou · 2018-03-26T17:48:52Z

app/models/embedded_ansible_worker/object_management.rb

+  private
+
+  CONSOLIDATED_PLUGIN_PLAYBOOKS_TEMPDIR = Pathname.new("/var/lib/awx_consolidated_source/projects/gem_ansible_content").freeze
+  def clean_consolidated_plugin_directory


@jrafanie, @carbonin - I'm not sure what user is applying this new path, but at the bare minimum it would require read/write by the awx user in Embedded Ansible

carbonin · 2018-03-27T13:40:12Z

app/models/embedded_ansible_worker/object_management.rb

+  end
+
+  PLUGIN_PLAYBOOK_PROJECT_NAME = "Default ManageIQ Playbook Project".freeze
+  PLAYBOOK_PROJECT_ATTRIBUTES = {


Does a project need to be added to an organization? Or is it okay to not specify that here?

@syncrou ☝️

@carbonin - It does need to be added to an Organization. It would be ok to specify that here. There is a chance the 'Default' organization would be set if the organization id is not passed in.

jrafanie · 2018-03-27T15:26:13Z

Note, I added and then mostly reverted a change to remove the temporary git repo. Basically, we can't remove it until it's cloned into the ansible project and there isn't a synchronous way to create the project so we don't really know when it's done unless we poll ansible or something like that. It doesn't seem worth it, at least for now. We'll leave it around and remove it on role activation to ensure we start clean.

JPrause · 2018-04-03T13:40:32Z

@jrafanie is this still a WIP?

jrafanie · 2018-04-03T15:31:13Z

@JPrause yes, I need to finish writing tests and squash some commits

ManageIQ/manageiq-design#40 https://bugzilla.redhat.com/show_bug.cgi?id=1539762 All of this depends on: https://github.com/ManageIQ/ManageIQ/manageiq/pull/17096 ManageIQ/manageiq-content#254

We can't remove this repo until it's been cloned in the ansible project. It's not worth the complexity to poll ansible to see if it's complete.

jrafanie · 2018-04-04T15:44:41Z

Ok, this is finally ready @syncrou @gmcculloug

carbonin · 2018-04-04T18:13:36Z

app/models/embedded_ansible_worker/object_management.rb

@@ -53,4 +54,91 @@ def ensure_host(provider, connection)
      :variables => {'ansible_connection' => "local"}.to_yaml
    ).id
  end
+
+  CONSOLIDATED_PLUGIN_PLAYBOOKS_TEMPDIR = Pathname.new("/var/lib/awx_consolidated_source").freeze


Can this live either down with the method that uses it or up at the top of the class?

carbonin · 2018-04-04T18:14:12Z

app/models/embedded_ansible_worker/object_management.rb

+  def copy_plugin_ansible_content
+    FileUtils.mkdir_p(self.class.consolidated_plugin_directory)
+
+    # TODO: make this a public api via an attr_reader


Should this block this PR? Is this simple to do now?

Make testability changes to make it easier to test: * Move constants to class methods that are easier to stub * Move chown call to a method that's easy to stub

miq-bot · 2018-04-04T19:06:09Z

Checked commits jrafanie/manageiq@7adff04~...eb7f90c with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0
4 files checked, 0 offenses detected
Everything looks fine. 🍰

gtanzillo

👍 Looks good @jrafanie

JPrause · 2018-04-10T16:46:12Z

@miq-bot add_label blocker

Seed plugin ansible playbooks (cherry picked from commit c9e7f02) https://bugzilla.redhat.com/show_bug.cgi?id=1566658

simaishi · 2018-04-12T17:37:50Z

Gaprindashvili backport details:

$ git log -1
commit cd10f5f723666562a53117e452cf3032634efec4
Author: Greg McCullough <[email protected]>
Date:   Fri Apr 6 17:57:52 2018 -0400

    Merge pull request #17185 from jrafanie/seed_plugin_ansible_playbooks
    
    Seed plugin ansible playbooks
    (cherry picked from commit c9e7f02baccbc2c2ca3713b00b8787c84c5b0fe9)
    
    https://bugzilla.redhat.com/show_bug.cgi?id=1566658

jrafanie added the wip label Mar 21, 2018

syncrou reviewed Mar 21, 2018

View reviewed changes

jrafanie force-pushed the seed_plugin_ansible_playbooks branch from 9f48ca9 to af75367 Compare March 22, 2018 18:43

gmcculloug added gaprindashvili/yes enhancement labels Mar 22, 2018

jrafanie force-pushed the seed_plugin_ansible_playbooks branch 3 times, most recently from 3f92cea to f76697c Compare March 22, 2018 22:09

carbonin reviewed Mar 23, 2018

View reviewed changes

jrafanie commented Mar 23, 2018

View reviewed changes

jrafanie force-pushed the seed_plugin_ansible_playbooks branch 2 times, most recently from 53e2043 to 3754fe3 Compare March 23, 2018 16:48

syncrou reviewed Mar 26, 2018

View reviewed changes

jrafanie force-pushed the seed_plugin_ansible_playbooks branch from 3754fe3 to 076bcce Compare March 26, 2018 20:50

carbonin reviewed Mar 27, 2018

View reviewed changes

jrafanie force-pushed the seed_plugin_ansible_playbooks branch from f585c71 to 5c6c5ff Compare April 2, 2018 20:55

jrafanie force-pushed the seed_plugin_ansible_playbooks branch from 5c6c5ff to 3bc082b Compare April 3, 2018 20:20

Add seeding of gem playbook content

7adff04

ManageIQ/manageiq-design#40 https://bugzilla.redhat.com/show_bug.cgi?id=1539762 All of this depends on: https://github.com/ManageIQ/ManageIQ/manageiq/pull/17096 ManageIQ/manageiq-content#254

jrafanie force-pushed the seed_plugin_ansible_playbooks branch from 3bc082b to b5dec8d Compare April 3, 2018 21:11

jrafanie changed the title ~~[WIP] Seed plugin ansible playbooks~~ Seed plugin ansible playbooks Apr 3, 2018

miq-bot removed the wip label Apr 3, 2018

jrafanie force-pushed the seed_plugin_ansible_playbooks branch 2 times, most recently from 6c7facb to 4a80166 Compare April 4, 2018 14:18

jrafanie added 2 commits April 4, 2018 11:43

Ruggedize the git repo/initial commit work.

3b53d4b

seed projects as part of initial object creation

f561f95

jrafanie added 7 commits April 4, 2018 11:43

Don't remove the temp git repo until ansible has cloned it.

a3b4049

Give file ownership to the awx user on the git repo

191e8a5

Don't remove the temporary git repo

2796b3b

We can't remove this repo until it's been cloned in the ansible project. It's not worth the complexity to poll ansible to see if it's complete.

Use the message catalog for the default project name

aa19dd1

Put the project in the default organization

8d50ee0

Populate default_project in custom attributes

40eb21c

Remove no longer referenced constant

664c570

jrafanie force-pushed the seed_plugin_ansible_playbooks branch from 4a80166 to 907612a Compare April 4, 2018 15:43

carbonin reviewed Apr 4, 2018

View reviewed changes

jrafanie added 3 commits April 4, 2018 15:02

Add tests

974af5d

Make testability changes to make it easier to test: * Move constants to class methods that are easier to stub * Move chown call to a method that's easy to stub

Allow consumers to ask for registered_ansible_content

f23bdb3

Use the now public registered_ansible_content reader

eb7f90c

jrafanie force-pushed the seed_plugin_ansible_playbooks branch from 907612a to eb7f90c Compare April 4, 2018 19:03

jrafanie assigned gmcculloug Apr 6, 2018

gtanzillo approved these changes Apr 6, 2018

View reviewed changes

carbonin approved these changes Apr 6, 2018

View reviewed changes

gmcculloug merged commit c9e7f02 into ManageIQ:master Apr 6, 2018

gmcculloug added this to the Sprint 83 Ending Apr 9, 2018 milestone Apr 6, 2018

jrafanie deleted the seed_plugin_ansible_playbooks branch April 10, 2018 15:31

miq-bot added the blocker label Apr 10, 2018

simaishi pushed a commit that referenced this pull request Apr 12, 2018

Merge pull request #17185 from jrafanie/seed_plugin_ansible_playbooks

cd10f5f

Seed plugin ansible playbooks (cherry picked from commit c9e7f02) https://bugzilla.redhat.com/show_bug.cgi?id=1566658

simaishi added gaprindashvili/backported and removed gaprindashvili/yes labels Apr 12, 2018


		commit_git_plugin_content

		if project = existing_plugin_playbook_project(connection)

Seed plugin ansible playbooks #17185

Seed plugin ansible playbooks #17185

Conversation

jrafanie commented Mar 21, 2018 • edited Loading

Choose a reason for hiding this comment

jrafanie commented Mar 22, 2018

jrafanie commented Mar 22, 2018

carbonin left a 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

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

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

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

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 Mar 26, 2018 • 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

jrafanie commented Mar 27, 2018

JPrause commented Apr 3, 2018

jrafanie commented Apr 3, 2018

jrafanie commented Apr 4, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

miq-bot commented Apr 4, 2018

gtanzillo left a comment

Choose a reason for hiding this comment

JPrause commented Apr 10, 2018

simaishi commented Apr 12, 2018

jrafanie commented Mar 21, 2018 •

edited

Loading

jrafanie Mar 26, 2018 •

edited

Loading