Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ansible Catchup #145

Merged
merged 16 commits into from
May 4, 2018
Merged

Conversation

rambleraptor
Copy link
Contributor

The final catchup PR! This merges in Ansible commits, brings in Ansible resource override support, and fixes some misc bugs on Puppet/Chef DNS.

Corresponding Ansible PR: https://github.com/modular-magician/ansible/pull/1/commits

Take a look at the last two commits in particular, since that's new code. This is a large PR, so let me know how to best split this up.


[all]

Ansible Catchup as of 20180501

[terraform]

[puppet]

[puppet-dns]

Moving quota to NestedObject

[puppet-sql]

[puppet-compute]

[chef]

This involves changes to pass module around so that input:true properties
can pick up on it.

Change-Id: I4913accdb988e5eb051626e08b7115feaee2aebd
Change-Id: I3c4b3a4e1083f38803744d890bf3d86963041c50
This also adds support for NameValues

Change-Id: Iba8513f382a7a18da983cb987829fcbd640f94c7
Change-Id: Id301f6521c61306f7534f9fb580821a4e4a0184c
Change-Id: I96b968df0a5cb0086bd729df075499f5e1b55510
Change-Id: Id0da7cf24540e692c627119ea5be4890ec19cbb1
Change-Id: I64a3ff47163c1c151ebe4826bd0ca0f902571979
Change-Id: I79fe83666821521eae652d18d05fa6839559c47b
Change-Id: If1de77011d1c1055d915541413a16e12d7e96795
Change-Id: Ice3076b7c47b66e1c5529900e4cab317cb5c1e00
Change-Id: I20806546d8263819093af14964ee4eeeb2fed59b
Change-Id: Ifc26d50a5f91a2946c05db5176a3d7bd8cb79241
Change-Id: Ibacca5e151fc50817edb103fd3b85a0cc982f819
Change-Id: Iee42d18e0c562f4c880978a77b566f8f07e7c287
@nat-henderson
Copy link
Contributor

This PR crashes terraform generation (so the magician will never show up to comment). Any idea what might be going on there?

@rambleraptor
Copy link
Contributor Author

Good call. Not sure how I missed that last time around.

Made a change that I'm fairly positive fixes it. Made a poor assumption about how overrides are working.

@modular-magician
Copy link
Collaborator

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: GoogleCloudPlatform/puppet-google-compute#25

@@ -272,11 +269,11 @@ def resource_to_request(module):
<%
encoder_name = if object.encoder?
object.transport.encoder
else config['encoder']
config['encoder']
else object.encoder
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is there a condition for a else statement here?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ping

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed. (object.encoder was the name for a custom encoder, changed to make more explicit)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(But actually fixed now)

@@ -66,7 +66,8 @@
url = git@github.com:GoogleCloudPlatform/chef-google-logging
[submodule "build/ansible"]
path = build/ansible
url = git@github.com:ansible/ansible.git
url = git@github.com:modular-magician/ansible
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fix tabs/spaces.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed.

description: |
Maximum allowed size for total rrdata in one ChangesCreateRequest
in bytes.
- !ruby/object:Api::Type::NestedObject
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You need to add an entry to puppet.yaml and chef.yaml to warn about this change.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What did you mean by this?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated.

(Chef DNS did not previously have a changelog).

default_value_property :exclude, false
default_value_property :editable, true
default_value_property :imports, []
default_value_property :provider_helpers, []

check_property :access_api_results, :boolean
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Check all properties.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed.

@modular-magician
Copy link
Collaborator

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

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

Firewall: !ruby/object:Provider::Ansible::ResourceOverride
exclude: true
BackendService: !ruby/object:Provider::Ansible::ResourceOverride
aliases:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be a property override.

properties:
timeoutSec:
aliases: ['timeout_seconds']

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good call! Fixed. Much more property_override driven.

@rambleraptor rambleraptor force-pushed the catchup branch 4 times, most recently from c73a415 to 5efed2f Compare May 3, 2018 20:29
Copy link
Contributor

@rosbo rosbo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 for the overrides usage.

Change-Id: I5ad6ecf1344621cb40d4123dfc80cc5861c28c8d
@modular-magician
Copy link
Collaborator

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

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

Change-Id: Ida223aca0a6ddecc7918b6a3d82c117953fe9af8
@rambleraptor rambleraptor merged commit 219d0cb into GoogleCloudPlatform:master May 4, 2018
@rambleraptor rambleraptor deleted the catchup branch May 4, 2018 07:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants