puppet-nginx
puppet-nginx copied to clipboard
voxpupuli
remove port from upstream member when service is defined
The service parameter of the upstream member enables port discovery
via DNS SRV records. When it is used, a server port must not be
specified or nginx will fail with the following error:
nginx: [emerg] service upstream may not have port
Pull Request (PR) description
The PR removes the port from the server directive when the serviceparameter is set and updates the corresponding unit test.
This Pull Request (PR) fixes the following issues
Fixes #1282
Hi @hbog, thanks for the change. Are you able to provide a tiny acceptance test for this? So we know for sure that the generated config works. You can find some examples in https://github.com/voxpupuli/puppet-nginx/tree/master/spec/acceptance. Let us know if you need some help. We're reachable in the #voxpupuli IRC channel on freenode and on https://slack.puppet.com
@bastelfreak I think acceptance tests are hard here because SRV records are hard without a real DNS server. I guess it'd be enough to verify that the server actually starts up though.
The service parameter for the server directive that I use is only available in the commercial offering NGINX-Plus.
Is there a license to add NGINX-plus to the acceptance tests ?
I think I could include dnsmasq in the test box to provide responses to the DNS SRV queries.
I would already be happy when we know that the generated config works and the daemon starts. IMO there is no need to test the actual SRV records. As mentioned in the other PR, I didn't know that this requires nginx-plus, so I assume simple acceptance testing won't be possible. Can you please update the docs with this new parameter? I think we can merge this without the acceptance tests.
Dear @hbog, thanks for the PR!
This is pccibot, your friendly Vox Pupuli GitHub Bot. I noticed that your pull request contains merge conflict. Can you please rebase?
You can find my sourcecode at voxpupuli/vox-pupuli-tasks
![vox-pupuli-tasks[bot] avatar](https://avatars.githubusercontent.com/in/37588?v=4 "Published by a pub.dev verified publisher"){kind=small}
Dear @hbog, thanks for the PR!
This is pccibot, your friendly Vox Pupuli GitHub Bot. I noticed that your pull request contains merge conflict. Can you please rebase?
You can find my sourcecode at voxpupuli/vox-pupuli-tasks
Dear @hbog, thanks for the PR!
This is pccibot, your friendly Vox Pupuli GitHub Bot. I noticed that your pull request contains merge conflict. Can you please rebase?
You can find my sourcecode at voxpupuli/vox-pupuli-tasks
Dear @hbog, thanks for the PR!
This is pccibot, your friendly Vox Pupuli GitHub Bot. I noticed that your pull request contains merge conflict. Can you please rebase?
You can find my sourcecode at voxpupuli/vox-pupuli-tasks