-
Notifications
You must be signed in to change notification settings - Fork 9.6k
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
Habitat provisioner updates #22705
Habitat provisioner updates #22705
Conversation
Not 100% sure why this failed, but I'm willing to help get this going if needed |
I'm not sure either, but I think git.apache.org may be down (which fresh builds have a dependency on): https://status.apache.org |
@pselle, I reviewed @apparentlymart provisioner changes from e0d7293#diff-b19db9165cff9397acb294490cf66308, and I was wondering if I should close this PR? It sounds like the recommendation is to not use provisioners moving forward, so I should probably not put these changes in (as that would go against the new policy)? |
@kmott I think that's a fair read -- adding more features to provisioners sends a mixed message when we're telling people "please don't use these", and that closing this PR is a reasonable action in that context. |
@pselle I guess my concern with that approach is there's a lot of Habitat (and I'm sure other provisioners) that needs to be setup when an instance is created (ring data, package installation, etc.), but you can't/shouldn't do that during the build/package phase with something like Packer (since it will be unique to each instance/deployment). If that first-class support for the provisioner isn't in-place in Terraform, then it's left to a bunch of remote_exec's and file() calls, which (as we all know) is cumbersome and error prone (they can be abstracted away to modules to reduce some of that complexity/duplicity, but it's still a concern, at least in my mind). |
@kmott I've gotten some clarification -- while the stance on provisioners reads strong, provisioners do exist currently in Terraform and basic maintenance to keep them working is the plan until/if an explicit plan on phasing out provisioners comes to fruition. To say it another way, the stance is not meant as "never use provisioners" but rather "consider these other options instead and prefer them whenever possible." As to your point about requirements of Habitat's architecture, there are possibly ways to achieve the goals of connecting to a ring with Now as to this PR specifically then -- I'm happy to move this forward (once de-conflicted) with the thought in mind of: while we can discourage users from using provisioners, they are still in Terraform, and your changes include bugfixes that would benefit such users (including those who filed the tickets you linked to, and surely more). I hope this response helps some? |
Yeah, thank you @pselle, I will get on this as soon as I can. :-) |
…cense`, adjust how license acceptance is done, update hab provisioner doc.
@pselle, I think my last commit is the final bit of cleanup needed. Hopefully I did the merge and conflict resolution correctly (git still confuses me sometimes, ;-)). I did test on a local vSphere deployment using hab 0.79.1 and 0.85.0 with several permanent peers, several machines with other services and binds, and it all seemed to work together just fine. Let me know if I need to adjust anything else. Thanks again for all of your help in brokering this ticket! :-) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One question/thing that might need a fix, but otherwise lgtm!
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. |
This is a continuation of: #21776
This PR should include most all feedback from the original PR, and gets tests running on master again.
Note that the Chef license changes are still a string to be more explicit about how the license acceptance works on the system you are provisioning Chef Habitat to.
Closes #20684
Closes #19394
Related to #18492
Closes #17799