Basic Puppet provisioner #18851

rodjek · 2018-09-13T11:22:37Z

Opening this up for general review of the Go code & style (I'm sure there's some code there that isn't idiomatic Go or not in the preferred style used at Hashicorp).

This PR adds a provisioner to provision instances with Puppet. It supports both open source Puppet and Puppet Enterprise. Docs will be added when the code is closer to being finalised.

This closes #21335.

mildwonkey

Hi @rodjek! Thanks for submitting this.

I've left a few comments and questions inline, but overall this is great to see.
I don't have experience with provisioners in terraform prior to this PR, so I'm going to get a second pair of eyes as well - don't take it personally :)

Thanks!

builtin/provisioners/puppet/bolt/bolt.go

builtin/provisioners/puppet/resource_provisioner.go

svanharmelen

Thank you very much for this PR @rodjek! Overall this looks great but I left a few comments that could maybe improve a few bits. Please have a look and let me know what you think about the suggestions.

Thanks!

svanharmelen · 2018-12-13T10:48:03Z

builtin/provisioners/puppet/bolt/bolt.go

+	}
+	cmdargs = append(cmdargs, command)
+
+	ctx, cancel := context.WithTimeout(context.Background(), 300*time.Second)


Could you share the reasoning for adding this particular (hardcoded) timeout?

Just a safety precaution in case bolt hangs for some reason. I've changed the timeout to be configurable with a default value of 5 minutes.

builtin/provisioners/puppet/nix_provisioner.go

builtin/provisioners/puppet/bolt/bolt.go

builtin/provisioners/puppet/nix_provisioner.go

builtin/provisioners/puppet/resource_provisioner.go

builtin/provisioners/puppet/windows_provisioner.go

hashicorp-cla · 2019-01-29T10:37:40Z

All committers have signed the CLA.

raphink · 2019-03-04T12:39:39Z

@svanharmelen what is the status of this PR? I'd be really great to have this feature in terraform!

svanharmelen

@rodjek I just got around to doing another review after you added the updates/fixes (sorry for taking so long!). I think it looks really good now, so thank you very much for all your work and the updates/fixes you added 👍

I do have a couple of small followup comments, but they should be very straightforward to address. After that I'm good to merge this, but I need to check in with the team to see if we want to add this to the current v0.12 release cycle, or wait until that is released and add this for v0.12.1.

@mildwonkey can you comment on when you think we can/should merge this (once the remaining comments are addressed)?

Thanks!

builtin/provisioners/puppet/resource_provisioner.go

builtin/provisioners/puppet/bolt/bolt.go

Co-Authored-By: rodjek <[email protected]>

rodjek · 2019-03-12T07:11:16Z

Thanks @svanharmelen, I think I've addressed all the outstanding items!

svanharmelen

One minor suggestion left, but the PR looks really nice now! Thanks for bearing with me and for updating the PR!

I'll leave merging the PR to @mildwonkey so she can coordinate it with the current v0.12 releases 👍

builtin/provisioners/puppet/resource_provisioner.go

Co-Authored-By: rodjek <[email protected]>

turbodog · 2019-03-14T16:37:32Z

@mildwonkey is there an ETA for the 0.12.1 release?

mildwonkey · 2019-03-14T16:57:55Z

No estimate yet @turbodog; we're still working on the 0.12.0 general release! I will update this as soon as I can.

randomcamel · 2019-03-14T17:37:01Z

@rodjek @turbodog We don't have a specific timeline on 0.12.1, but I'd like us to get it out ASAP after 0.12.0. In practice, this means letting 0.12.0 bake a little bit until we're confident any major issues have been dealt with. HCL2 alone means 0.12.0 has a fundamentally (very) large surface area, and we want a feature like the Puppet provisioner to ship on a version of 0.12.x that provides a stable platform on top of the 0.12.0 refactor.

turbodog · 2019-03-14T18:25:09Z

Thanks. Any ETA on 0.12.0?

Also, is there a possibility of backporting this into 0.11.x? As you said 0.12 has a large surface area and would be great to get this into people's hands in their current release line.

rodjek · 2019-04-09T03:09:04Z

@mildwonkey @svanharmelen Just pushed a minor change to the configuration schema.

mildwonkey · 2019-05-23T12:06:08Z

Hi @rodjek !

TERRAFORM 0.12 HAS RELEASED 🎉 🎉 🎉
We can finally move on with our lives 🤣

I'm going to dive back into this PR. You've already addressed tons of our feedback, so hopefully all that's left to do is add documentation. It would be nice to see more code comments, for the benefit of folks not as familiar with puppet (or terraform provisioners, for that matter), but that's not a blocker.

Out of sheer curiosity, do you have some sort of local acceptance testing set up? I'd love to get some hands-on experience with this provisioner, or even just watch a demo of it in action. Again, this is just me being nosy, no pressure 😄

mildwonkey · 2019-05-23T12:23:55Z

builtin/provisioners/puppet/resource_provisioner.go

+		return err
+	}
+
+	if err = p.runPuppetAgent(); err != nil {


I imagine documentation will cover this, but what's the expected workflow if autosign is false? Would a user just expect the puppet run fail until the cert is signed?

If autosign is false, the Puppet run will probably fail. If they've configured their master to use naive certname matching autosigning (i.e. just trusting and signing any request for *.mycompany.com) then that will succeed. For other cases, I'm assuming they'll use something like a custom remote-exec provisioner before the puppet provisioner to take whatever action their setup requires (adding an entry to their CMBD perhaps).

mildwonkey · 2019-06-10T19:30:43Z

🎉This will be included in the next release - thanks @rodjek!!

llomgui · 2019-06-13T08:56:58Z

Do we have any documentation about this new provisioner?

lfarnell · 2019-06-13T12:44:20Z

@llomgui No. This PR didn't have any documentation. @rodjek Is this something you can do? If not I can submit a PR.

cc @mildwonkey

turbodog · 2019-06-16T23:51:55Z

@lfarnell could you point us to an example or similar set of docs?

rkst · 2019-06-17T17:47:00Z

@turbodog The Chef provisioner is probably a good similar base for docs.

rodjek · 2019-06-19T07:38:16Z

PR opened to add some docs at #21792

Out of sheer curiosity, do you have some sort of local acceptance testing set up? I'd love to get some hands-on experience with this provisioner, or even just watch a demo of it in action.

@mildwonkey I've pushed up a repo with a basic setup that you can use on AWS at https://github.com/rodjek/terraform-puppet-example/ (just don't judge me based on the quick and dirty execs provisioning the puppet master instance! 😀)

ghost · 2019-07-25T01:59:24Z

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

mildwonkey reviewed Dec 11, 2018

View reviewed changes

rodjek force-pushed the puppet-provisioner branch 2 times, most recently from dc141cf to c09ecad Compare December 12, 2018 04:21

svanharmelen reviewed Dec 17, 2018

View reviewed changes

rodjek added 7 commits January 29, 2019 20:28

Basic Puppet provisioner

cc50f33

(fixup) fix snake_case use in Bolt

c203e77

(fixup) Remove unused ValidateFunc

1be9846

(fixup) Check bolt result status

3a83288

(lint) go fmt

2780fcb

Requested changes

606146e

Remove PE autodetection

e5ad531

rodjek force-pushed the puppet-provisioner branch from a7e0216 to e5ad531 Compare January 29, 2019 10:37

svanharmelen reviewed Mar 6, 2019

View reviewed changes

Sander van Harmelen and others added 4 commits March 12, 2019 14:02

Apply suggestions from @svanharmelen

4552211

Co-Authored-By: rodjek <[email protected]>

Tag all JSON fields in bolt output

71942f1

Defer comm.Disconnect() as suggested

52d12d7

Make bolt timeout configurable

de5a44f

svanharmelen approved these changes Mar 12, 2019

View reviewed changes

builtin/provisioners/puppet/resource_provisioner.go Outdated Show resolved Hide resolved

Update builtin/provisioners/puppet/resource_provisioner.go

ea7463f

Co-Authored-By: rodjek <[email protected]>

rodjek changed the title ~~[WIP] Basic Puppet provisioner~~ Basic Puppet provisioner Mar 12, 2019

randomcamel added this to the v0.12.1 milestone Mar 12, 2019

mildwonkey self-assigned this Mar 14, 2019

Make extension_requests and custom_attributes configurable

4445558

apparentlymart mentioned this pull request May 16, 2019

terraform provisioner for Puppet #21335

Closed

mildwonkey reviewed May 23, 2019

View reviewed changes

pselle modified the milestones: v0.12.1, TBD Jun 4, 2019

mildwonkey approved these changes Jun 10, 2019

View reviewed changes

mildwonkey merged commit 615110e into hashicorp:master Jun 10, 2019

rodjek mentioned this pull request Jun 19, 2019

Docs for the Puppet provisioner #21792

Merged

ghost locked and limited conversation to collaborators Jul 25, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Basic Puppet provisioner #18851

Basic Puppet provisioner #18851

rodjek commented Sep 13, 2018 •

edited by apparentlymart

Loading

mildwonkey left a comment

svanharmelen left a comment

svanharmelen Dec 13, 2018

rodjek Mar 12, 2019

hashicorp-cla commented Jan 29, 2019 •

edited

Loading

raphink commented Mar 4, 2019

svanharmelen left a comment

rodjek commented Mar 12, 2019

svanharmelen left a comment

turbodog commented Mar 14, 2019

mildwonkey commented Mar 14, 2019

randomcamel commented Mar 14, 2019

turbodog commented Mar 14, 2019

rodjek commented Apr 9, 2019

mildwonkey commented May 23, 2019

mildwonkey May 23, 2019

rodjek Jun 19, 2019

mildwonkey commented Jun 10, 2019

llomgui commented Jun 13, 2019

lfarnell commented Jun 13, 2019

turbodog commented Jun 16, 2019

rkst commented Jun 17, 2019

rodjek commented Jun 19, 2019

ghost commented Jul 25, 2019

Basic Puppet provisioner #18851

Basic Puppet provisioner #18851

Conversation

rodjek commented Sep 13, 2018 • edited by apparentlymart Loading

mildwonkey left a comment

Choose a reason for hiding this comment

svanharmelen left a comment

Choose a reason for hiding this comment

svanharmelen Dec 13, 2018

Choose a reason for hiding this comment

rodjek Mar 12, 2019

Choose a reason for hiding this comment

hashicorp-cla commented Jan 29, 2019 • edited Loading

raphink commented Mar 4, 2019

svanharmelen left a comment

Choose a reason for hiding this comment

rodjek commented Mar 12, 2019

svanharmelen left a comment

Choose a reason for hiding this comment

turbodog commented Mar 14, 2019

mildwonkey commented Mar 14, 2019

randomcamel commented Mar 14, 2019

turbodog commented Mar 14, 2019

rodjek commented Apr 9, 2019

mildwonkey commented May 23, 2019

mildwonkey May 23, 2019

Choose a reason for hiding this comment

rodjek Jun 19, 2019

Choose a reason for hiding this comment

mildwonkey commented Jun 10, 2019

llomgui commented Jun 13, 2019

lfarnell commented Jun 13, 2019

turbodog commented Jun 16, 2019

rkst commented Jun 17, 2019

rodjek commented Jun 19, 2019

ghost commented Jul 25, 2019

rodjek commented Sep 13, 2018 •

edited by apparentlymart

Loading

hashicorp-cla commented Jan 29, 2019 •

edited

Loading