Revert "Added ngnix::resources::upstream::member" #336
Conversation
Upstream members can no longer be exported and collected. The change in voxpupuli#331 was fundamentally broken. I have therefore reverted it as it shouldn't of been merged. Essentially you can't use ensure with this change - meaning you can no longer REMOVE an nginx config from the system - which is part of the tests and also sane module practice. The idea was nice - but the implementation broke things. This reverts back to a good state, without modifying any tests where tests pass again with the recent commits. This reverts commit ebf3e4e.
|
Thinking out loud - not sure the change was needed in the first place as using stdlib you can build up the list of members and pass that in anyway. Everyone uses stdlib. |
|
I should also clarify that although it doesn't work in the version of Puppet in the Gemfile (which would currently be 3.6.2 as the file specified >= 3.0.1) - it might in some newer version but still the tests should be updated and, if necessary, min. puppet version upped when stable. |
|
New version of Librarian Puppet is borked - so we'll use the one before it then. |
1.1.0 is broken - 1.0.3 works fine.
Revert "Added ngnix::resources::upstream::member"
|
@leepa Can you help me understand how you would use stdlib to build the list of members and pass it in as an array? I have used the stdlib for simple things like validating if a parameter passed is a bool, but never used it for something like this. |
|
@zxjinn I was merely thinking out loud rather than having a full solution. stdlib doesn't just have a resource type, but a number of functions that extend the parser in interesting ways. One of these is around list management. This change in #331 however wasn't the right way to go as, well, it didn't even pass Puppet syntax checking. If I get some time I'll see if there's a way to make it work - but I'm not sure how right now without breaking the module itself. Sorry I can't be of more help there. |
|
@leepa No problem at all, I thought you had a solution worked out that you were already using. If I stumble into an easy and smooth way to do this, I'll comment here. I presume that my solution is going to be so hacky that I won't want to share it with the world. I love the idea of 331, but no pressure to get the whole module re-written to support it |
|
Unless I'm missing something, (and aside from linting issues), all #331 needed to work was a version update of concat (1.1.0 has the ensure parameter, puppet-nginx is currently pinned at 1.0.0), and the resource_upstream_spec.rb needed to be updated to reflect the fact that concat was being used (see: #344 and #345). If there are more issues with the #331, let's address and fix them. :-) |
|
@rabbitt 👍 sounds like a plan! |
|
@rainopik both tools don't cover everything. Please always run rake spec as that actually tries to execute the code. |
Upstream members can no longer be exported and collected.
The change in #331 was fundamentally broken. I have therefore reverted
it as it shouldn't of been merged.
Essentially you can't use ensure with this change - meaning you can no
longer REMOVE an nginx config from the system - which is part of the
tests and also sane module practice.
The idea was nice - but the implementation broke things. This reverts
back to a good state, without modifying any tests where tests pass again
with the recent commits.
This reverts commit ebf3e4e.