Ansible facts modules #247

rambleraptor · 2018-06-05T20:39:41Z

This may end up being a long-running PR.

This PR is meant to get support for Ansible facts modules started. So far, the gcp_compute_instance_facts module works.

There is missing documentation and I doubt linters will work on this.

Let me know your thoughts on the best way to slowly merge this in.

[all]

[terraform]

[puppet]

[puppet-compute]

[puppet-sql]

[puppet-storage]

[chef]

[chef-compute]

[chef-sql]

[chef-storage]

[ansible]

modular-magician · 2018-06-05T20:57:07Z

I am a robot that works on MagicModules PRs!

I built this PR into one or more PRs on other repositories, and when those are closed, this PR will also be merged and closed.

rambleraptor · 2018-06-21T00:06:33Z

This is currently generating the gcp_compute_instance_facts module and uses the gcp_compute_instance integration test for a full end-to-end test of both modules.

rambleraptor · 2018-07-10T23:11:32Z

Facts modules are in a good place now! I'm successfully building out 1 facts module (gcp_compute_instance_facts) that successfully passes Ansible's linters. That facts module is used as the verifying data source for gcp_compute_instance's integration test successfully.

What comes next (but not necessarily in this PR)?

Fixing up the Example class so that I can build facts examples from regular examples.
More facts modules! If I can build facts examples from regular examples, there should be no configuration that needs to happen.
I'm pretty convinced I could combine this into the Ansible provider, so that'll probably happen very soon.

danawillow · 2018-07-10T23:14:11Z

Exciting! Are you still looking for a code review for this or do you have more stuff you're hoping to get done in this PR before I take a look?

rambleraptor · 2018-07-10T23:14:57Z

Code review would be great!

modular-magician · 2018-07-10T23:33:27Z

I am a robot that works on MagicModules PRs!
I checked the downstream repositories (see README.md for which ones I can write to), and none of them seem to have any changes.

Once this PR is approved, you can feel free to merge it without taking any further steps.

danawillow

This looks reasonable to me, but I'd like to see it get generated- looks like the magician didn't think there was anything to generate.

danawillow · 2018-07-11T16:35:34Z

products/compute/ansible_facts.yaml

+  compile:
+tests: !ruby/object:Api::Resource::HashArray
+# This would be for custom network responses. Tests work by running some block
+# of autogenerated Chef/Puppet code and then verifying the network calls.


This comment can probably be changed, since this file is clearly for ansible only

danawillow · 2018-07-11T16:37:56Z

provider/ansible/example.rb

+    # Holds all information necessary to run a facts module and verify the
+    # creation / deletion of a resource.
+    #
+    # command         - The gcloud command being run


I'm not sure this documentation lines up with this class

danawillow · 2018-07-11T16:42:17Z

products/compute/ansible_facts.yaml

+  author: Google Inc. (@googlecloudplatform)
+overrides: !ruby/object:Provider::ResourceOverrides
+  Instance: !ruby/object:Provider::Ansible::ResourceOverride
+    generate_facts: true


Hmm, every resource that's listed in this file is going to have generate_facts: true, right? Is there a way we can avoid having to explicitly say that?

I just wanted to do that temporarily. My intention is to set that to true by default (and probably merge this provider into the regular Ansible provider).

This seems very much like enabled: true, right?

danawillow · 2018-07-11T16:46:59Z

provider/ansible/example.rb

+                       Google::StringUtils.underscore(object.name),
+                       'facts'].join('_')
+        [
+          "- name: verify that #{obj_name} was #{verb}",


There are a lot of formatting things in here- would it make sense for this part to be an erb template? Or is there enough regular ruby going on that this is better?

For the moment, I think it's alright to keep this as Ruby. I'm going to do a small refactor of this code in a separate PR. It should clean it up and make generating facts examples from regular examples possible.

As part of #343, I ended up using ERB templates. Thanks for the idea!

#343 should also enable me to build the facts examples from the regular examples, which would be super awesome and allow facts modules to be generated with no additional configuration (yay!)

modular-magician · 2018-07-11T17:26:38Z

I am a robot that works on MagicModules PRs!
I checked the downstream repositories (see README.md for which ones I can write to), and none of them seem to have any changes.

Once this PR is approved, you can feel free to merge it without taking any further steps.

nat-henderson · 2018-07-11T20:58:16Z

Ooh, hey, you've got a Magician bug / Concourse bug - you pushed a commit which didn't modify the SHA of the output magic module repo, while the other one was still running. Nice! It means that Concourse never marked any of your commits as having been successfully run. I can fix it!

nat-henderson · 2018-07-11T21:07:09Z

Fix is in #342. Take a look when you get a chance!

modular-magician · 2018-07-11T22:23:50Z

I am a robot that works on MagicModules PRs!

I built this PR into one or more PRs on other repositories, and when those are closed, this PR will also be merged and closed.
depends: modular-magician/ansible#53

nat-henderson

Facts are a lot like terraform datasources, right?

nat-henderson · 2018-07-14T00:58:48Z

products/compute/ansible_facts.yaml

+  author: Google Inc. (@googlecloudplatform)
+overrides: !ruby/object:Provider::ResourceOverrides
+  Instance: !ruby/object:Provider::Ansible::ResourceOverride
+    generate_facts: true


This seems very much like enabled: true, right?

rambleraptor · 2018-07-15T06:16:07Z

Facts are exactly like data sources (according to @danawillow) Backstory: I was going to combine this into the actual Ansible provider, but I wasn’t sure if it would become obvious that I would need a separate provider. My plan is to have a separate provider for the moment and if there’s no real need for egregious custom code (and it doesn’t clutter Ansible provider too much), I’ll combine this provider back into Ansible proper For the moment, I’m using a separate keyword because I can set it to false by default. Did that answer your question? Diffs arent the best on mobile email so I’m making some educated guesses on what you’re asking.

…

On Fri, Jul 13, 2018 at 6:00 PM Nathan McKinley ***@***.***> wrote: ***@***.**** approved this pull request. Facts are a lot like terraform datasources, right? ------------------------------ In products/compute/ansible_facts.yaml <#247 (comment)> : > + +--- !ruby/object:Provider::Ansible::Facts::Config +manifest: !ruby/object:Provider::Ansible::Manifest + metadata_version: '1.1' + status: + - preview + supported_by: 'community' + requirements: + - python >= 2.6 + - requests >= 2.18.4 + - google-auth >= 1.3.0 + version_added: '2.7' + author: Google Inc. ***@***.***) +overrides: !ruby/object:Provider::ResourceOverrides + Instance: !ruby/object:Provider::Ansible::ResourceOverride + generate_facts: true This seems very much like enabled: true, right? — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#247 (review)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABQ65gn14e5BOcIkOVl5PDJhcEQL78IBks5uGUKZgaJpZM4UbhMM> .

nat-henderson · 2018-07-16T21:42:38Z

Yeah... I see what you're saying, but I'm feeling suspicious of adding a new provider which will merge code into the same downstream repo. If we don't have a good reason right now to split it out, I vote we don't split it out - it'll never be easier to combine than it is today.

rambleraptor · 2018-07-16T21:45:12Z

I like that logic. I'll make that change now.

…

On Mon, Jul 16, 2018 at 2:42 PM Nathan McKinley ***@***.***> wrote: Yeah... I see what you're saying, but I'm feeling suspicious of adding a new provider which will merge code into the same downstream repo. If we don't have a good reason right now to split it out, I vote we don't split it out - it'll never be easier to combine than it is today. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#247 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABQ65t0RA78rqo57mov2cMObnwPPwMgEks5uHQjPgaJpZM4UbhMM> .

…ed out with facts modules

… provider

modular-magician · 2018-07-18T21:20:42Z

I am (still) a robot that works on MagicModules PRs!

I just wanted to let you know that your changes (as of commit aa22694) have been included in your existing downstream PRs.

modular-magician · 2018-07-18T21:21:05Z

I am (still) a robot that works on MagicModules PRs!

I just wanted to let you know that your changes (as of commit a1863bb) have been included in your existing downstream PRs.

modular-magician · 2018-07-18T21:28:23Z

I am (still) a robot that works on MagicModules PRs!

I just wanted to let you know that your changes (as of commit 8610537) have been included in your existing downstream PRs.

modular-magician · 2018-07-18T21:44:09Z

I am (still) a robot that works on MagicModules PRs!

I just wanted to let you know that your changes (as of commit df2393e) have been included in your existing downstream PRs.

Tracked submodules are build/puppet/compute build/puppet/sql build/puppet/storage build/puppet/container build/puppet/dns build/puppet/pubsub build/puppet/resourcemanager build/chef/compute build/chef/sql build/chef/storage build/chef/container build/chef/dns build/terraform build/ansible.

rambleraptor requested a review from danawillow June 5, 2018 20:39

modular-magician added the downstream-generated label Jun 5, 2018

rambleraptor force-pushed the ansible-facts-modules branch from fa507a8 to 5ba3207 Compare June 21, 2018 00:05

rambleraptor force-pushed the ansible-facts-modules branch from 5ba3207 to 9c34e2a Compare June 21, 2018 00:07

rambleraptor force-pushed the ansible-facts-modules branch from 9c34e2a to f52dae5 Compare July 10, 2018 23:05

rambleraptor changed the title ~~WIP: Ansible facts modules~~ Ansible facts modules Jul 10, 2018

rambleraptor requested a review from nat-henderson July 10, 2018 23:14

modular-magician added the downstream-generated label Jul 10, 2018

danawillow reviewed Jul 11, 2018

View reviewed changes

rambleraptor removed the downstream-generated label Jul 11, 2018

rambleraptor force-pushed the ansible-facts-modules branch from b294953 to a67efdd Compare July 11, 2018 17:01

modular-magician added the downstream-generated label Jul 11, 2018

rambleraptor force-pushed the ansible-facts-modules branch from cdc59fe to 4dd2c1c Compare July 11, 2018 17:44

rambleraptor force-pushed the ansible-facts-modules branch from 4dd2c1c to 16a26ef Compare July 11, 2018 21:54

modular-magician added the downstream-generated label Jul 11, 2018

nat-henderson approved these changes Jul 14, 2018

View reviewed changes

googlebot added the cla: yes label Jul 16, 2018

rambleraptor added 18 commits July 18, 2018 20:41

Baseline facts module created

a466c1d

Exclude all facts modules but instance for the moment

c25763d

moving facts into their own provider

e01e05a

Rakefile update for Ansible facts

c62679a

Instance fact module working

744714b

Rubocop fixes

1e0bd0d

Ansible Facts: CI support

43aa032

Adding verifier task to integration tests, so that gcloud can be subb…

a2d1363

…ed out with facts modules

list returns nil when empty because of course it does

53a7def

Rubocop fixes

05de810

Ignore facts generation by default

a5ca310

Fixing ansible linter issues

c082f24

Added EXAMPLES and RETURNS to facts modules

b9c4152

Rubocop fixes

d30ae80

PR changes

9aaf6c2

Adding generic datasource support and combining Ansible into a single…

559cbde

… provider

rubocop fixes

1ca3e70

Removing old facts provider

ba31600

rambleraptor force-pushed the ansible-facts-modules branch from a1863bb to ba31600 Compare July 18, 2018 20:41

modular-magician added the downstream-generated label Jul 18, 2018

Magician should use different remote on non-GCP pushes

69b5bc3

rambleraptor force-pushed the ansible-facts-modules branch from 974695f to 69b5bc3 Compare July 18, 2018 21:21

Merge 69b5bc3 into a6d8316

df2393e

modular-magician added the downstream-generated label Jul 18, 2018

modular-magician merged commit abc39f9 into GoogleCloudPlatform:master Jul 18, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Ansible facts modules #247

Ansible facts modules #247

rambleraptor commented Jun 5, 2018

modular-magician commented Jun 5, 2018 •

edited by rambleraptor

Loading

rambleraptor commented Jun 21, 2018

rambleraptor commented Jul 10, 2018 •

edited

Loading

danawillow commented Jul 10, 2018

rambleraptor commented Jul 10, 2018 via email •

edited

Loading

modular-magician commented Jul 10, 2018

danawillow left a comment

danawillow Jul 11, 2018

danawillow Jul 11, 2018

danawillow Jul 11, 2018

rambleraptor Jul 11, 2018

nat-henderson Jul 14, 2018

danawillow Jul 11, 2018

rambleraptor Jul 11, 2018

rambleraptor Jul 11, 2018 •

edited

Loading

modular-magician commented Jul 11, 2018

nat-henderson commented Jul 11, 2018

nat-henderson commented Jul 11, 2018

modular-magician commented Jul 11, 2018

nat-henderson left a comment

nat-henderson Jul 14, 2018

rambleraptor commented Jul 15, 2018 via email

nat-henderson commented Jul 16, 2018

rambleraptor commented Jul 16, 2018 via email

modular-magician commented Jul 18, 2018

modular-magician commented Jul 18, 2018

modular-magician commented Jul 18, 2018

modular-magician commented Jul 18, 2018

Ansible facts modules #247

Ansible facts modules #247

Conversation

rambleraptor commented Jun 5, 2018

[all]

[terraform]

[puppet]

[puppet-compute]

[puppet-sql]

[puppet-storage]

[chef]

[chef-compute]

[chef-sql]

[chef-storage]

[ansible]

modular-magician commented Jun 5, 2018 • edited by rambleraptor Loading

rambleraptor commented Jun 21, 2018

rambleraptor commented Jul 10, 2018 • edited Loading

danawillow commented Jul 10, 2018

rambleraptor commented Jul 10, 2018 via email • edited Loading

modular-magician commented Jul 10, 2018

danawillow 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

rambleraptor Jul 11, 2018 • edited Loading

Choose a reason for hiding this comment

modular-magician commented Jul 11, 2018

nat-henderson commented Jul 11, 2018

nat-henderson commented Jul 11, 2018

modular-magician commented Jul 11, 2018

nat-henderson left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rambleraptor commented Jul 15, 2018 via email

nat-henderson commented Jul 16, 2018

rambleraptor commented Jul 16, 2018 via email

modular-magician commented Jul 18, 2018

modular-magician commented Jul 18, 2018

modular-magician commented Jul 18, 2018

modular-magician commented Jul 18, 2018

modular-magician commented Jun 5, 2018 •

edited by rambleraptor

Loading

rambleraptor commented Jul 10, 2018 •

edited

Loading

rambleraptor commented Jul 10, 2018 via email •

edited

Loading

rambleraptor Jul 11, 2018 •

edited

Loading