add $members_tag parameter to nginx::resource::upstream

[brunoleon]

Add $members_tag parameter that allow to collect upstream member based on a specific tag.

The needs for this appeared while deploying a multi DC application where you do not want Proxy to register backends from other DC.
The ability to tag comes in as an easy and flexible solution.

[bastelfreak]

Hi @brunoleon, can you please rebase?

[brunoleon]

Hello @bastelfreak
Actually while testing my rebase, I encountered an issue which is more related to a previous change done in 03b4ac5

I does not seem a good idea to declare a class to collect resources within the define because it causes duplicate class declaration as soon as you have multiple upstream.

I can rebase and adress this issue by reverting back to previous behavior which was not using a class ::nginx::resource::upstream::collect.

What do you think of it ?

[wyardley]

Issue #897 also seems related. @brunoleon: if you can revert the previous behavior, that seems like a good idea. Please try and check to see if there's a way to suppress the warning that #832 was trying to resolve.

[brunoleon]

I rebased the code and reverted back to previous behavior.

As long as $members parameter is passed their should be no warning as no resource collection is done.

[wyardley]

@brunoleon: The one test failure was due to an unrelated issue; can you rebase against upstream one more time, and then push again?

[wyardley]

ps - It still needs tests.
Let us know if you need help figuring out what to test or how.

[brunoleon]

I had a look to testing code, but I do not see how to test for exported resources indeed.
Any hints ?

[wyardley]

@brunoleon: I'm not sure, the quick google searches that came up seemed to suggest that it isn't possible or isn't easy. I also don't know much about what the module is doing here, but maybe the members can be mocked in, and just the behavior tested? I'm asking @bastelfreak if we need additional tests here. I think the one failed test in Travis should be fine if we re-run it.

You could check in #testing channel on https://puppetcommunity.slack.com maybe?

[wyardley]

@hunner @petems: You have any suggestions in terms of what / how to test here?

[wyardley]

I had some discussions about testing possibilities for this. Specifically, @binford2k and @rnelson0 had some ideas... it sounds like you wouldn't test collection itself, but mock in the resources in the tests, and test that the module does the right thing.

I'd really like to see tests eventually, but I think for now, adding the tests is probably beyond my capabilities, and this problem is causing issues for other folks, so I think merging this now is probably the sane thing to do. If someone has the time / ability to add tests, I'd love to see those in a separate PR.