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

Partial updates on Ansible, Puppet and Chef #373

Merged
merged 41 commits into from
Aug 20, 2018
Merged

Conversation

rambleraptor
Copy link
Contributor

@rambleraptor rambleraptor commented Jul 27, 2018

This has been too long coming.

How does Terraform do integration tests on updates? I can do a manual test to verify that everything doesn't crash + burn, but I'd like something more automated in the future.


[all]

Partial updates on A/C/P

[terraform]

[puppet]

Partial updates

[puppet-compute]

[puppet-container]

[puppet-dns]

[puppet-logging]

[puppet-pubsub]

[puppet-resourcemanager]

[puppet-sql]

[puppet-storage]

[chef]

Partial updates

[chef-compute]

[chef-container]

[chef-dns]

[chef-logging]

[chef-sql]

[chef-storage]

[ansible]

Partial updates

@modular-magician
Copy link
Collaborator

modular-magician commented Jul 27, 2018

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.

@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 1497538) have been included in your existing downstream PRs.

@SirGitsalot
Copy link
Member

Is there any easy way to see the generated code for a PR?

@rambleraptor
Copy link
Contributor Author

rambleraptor commented Jul 27, 2018

After the magician runs, it'll give a list of PRs with the generated code. If you look up a couple comments, you'll see the Magician message with a link to (link redacted)

@rambleraptor rambleraptor changed the title Partial updates on Ansible Partial updates on Ansible + Puppet Jul 27, 2018
@rambleraptor
Copy link
Contributor Author

I added in Puppet support for partial updates too.

I started on Chef, but it seems like it'll be a lot trickier because we don't have an easy way to do property diffs.

@danawillow
Copy link
Contributor

Here's an example of a Terraform test that tests update: https://github.com/terraform-providers/terraform-provider-google/blob/master/google/resource_compute_disk_test.go#L247. The Terraform libraries give us the ability to have multiple TestSteps in a given test, so in this case, it runs the basic config (https://github.com/terraform-providers/terraform-provider-google/blob/master/google/resource_compute_disk_test.go#L601) and then runs terraform as if you had changed the config to the updated one (https://github.com/terraform-providers/terraform-provider-google/blob/master/google/resource_compute_disk_test.go#L629). We don't have any way in Terraform to test that it was indeed an update and not a destroy/create, but this at least lets us check that the resource has the properties we expect at the end of the test.

@rambleraptor rambleraptor changed the title Partial updates on Ansible + Puppet Partial updates on Ansible, Puppet and Chef Jul 31, 2018
@@ -0,0 +1,23 @@
<% update_props = properties_by_custom_update(object.all_user_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.

I'd like to start putting the shared Puppet+Chef code inside a puppetchef folder to clean up the templates/ directory. Any objections?

Copy link
Contributor

Choose a reason for hiding this comment

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

SGTM.

api/type.rb Outdated
# Represents a fingerprint. A fingerprint is an output-only
# field used for optimistic locking during updates.
class Fingerprint < String
class Fingerprint < FetchedExternal
Copy link
Contributor

Choose a reason for hiding this comment

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

Hm, what does that do to the Terraform output code?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This shouldn't do anything.

Copy link
Contributor

Choose a reason for hiding this comment

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

Great!

@rambleraptor rambleraptor force-pushed the partial-update branch 4 times, most recently from 0053480 to d470196 Compare August 10, 2018 20:09
@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 ee93eca) have been included in your existing downstream PRs.

@modular-magician
Copy link
Collaborator

modular-magician commented Aug 10, 2018

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#83
depends: GoogleCloudPlatform/chef-google-compute#45
depends: modular-magician/ansible#63

@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 5045044) have been included in your existing downstream PRs.

Tracked submodules are build/puppet/_bundle build/puppet/auth build/puppet/bigquery build/puppet/compute build/puppet/sql build/puppet/storage build/puppet/spanner build/puppet/container build/puppet/dns build/puppet/pubsub build/puppet/resourcemanager build/chef/_bundle build/chef/auth build/chef/compute build/chef/sql build/chef/storage build/chef/spanner build/chef/container build/chef/dns build/chef/iam build/terraform build/ansible.
@modular-magician modular-magician merged commit 14bc77e into master Aug 20, 2018
@nat-henderson
Copy link
Contributor

Woop woop! This is huge. :)

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.

6 participants