Skip to content
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

update to alpine3.5 and nginx 1.10.2 #3

Merged
merged 1 commit into from
Jan 31, 2017
Merged

Conversation

stefancocora
Copy link
Contributor

  • the nginx version 1.10.2 in alpine3.5 comes with the
    http_realip_module module compiled in. This module is used to
    whitelist/blacklist on proxyprotocol set ips from upstream
    loadbalancers.

@vaijab
Copy link
Contributor

vaijab commented Jan 26, 2017

nginx version 1.10.2

Any breaking changes?

@stefancocora
Copy link
Contributor Author

No breaking changes as far as I've seen, although it is really hard to ascertain without a list of current supported features.
Do you have this list of currently supported features for this nginx package ?

Semver wise I'll bump this to v1.0.0 just because it a newer version of nginx and the alpine maintainers have enabled a lot more nginx modules at compile time compared to alpine3.4's nginx
http://git.alpinelinux.org/cgit/aports/tree/main/nginx/APKBUILD?h=3.5-stable

@vaijab
Copy link
Contributor

vaijab commented Jan 26, 2017

I guess an upgrade makes sense, but I'd like more testing to be done before any release. Can you build an image of this branch and use it for testing? You mentioned you have a real use-case that needs certain features from the upgrade.

@stefancocora
Copy link
Contributor Author

Can you build an image of this branch and use it for testing?

This is easy , I can build a test image.

You mentioned you have a real use-case that needs certain features from the upgrade.

This isn't as trivial as you'd think, the feature I need is in the reverse proxy part of nginx in the real_ip header module.
To test this part I'd need to bring up a testing stack that looks like

  • HTTP caller code ==> this_docker_container_in_reverse_proxy ==> backend webserver
  • capture the result at the "backend webserver" and parse the headers

While this can be done it is costly and time consuming, think of the project wait times too please.

Happy to go ahead with this testing routine if you consider it absolutely necessary. It is your call !

@vaijab
Copy link
Contributor

vaijab commented Jan 27, 2017

Happy to go ahead with this testing routine if you consider it absolutely necessary

I was thinking in terms of testing in general. If you can build a test image and use it in real world, that'd be quite good to verify that the upgrade does not break many things.

@stefancocora
Copy link
Contributor Author

Now it is clear what you meant earlier with testing.
I'll build an image from the branch changes and test it as a replacement for docker-proxy-nginx.
I'll put my findings in this ticket.

@vaijab
Copy link
Contributor

vaijab commented Jan 27, 2017

I'll put my findings in this ticket.

Perfecto! There are no tests here, so I am just a little worried, that's all :-) It's the nginx version upgrade, but I am more worried that alpine now compiles in every single module that there is.

@stefancocora
Copy link
Contributor Author

I've tested the nginx image 845ec912426f121580b17c07a409e8b860209e6b with nginx v1.10.2 with real apps on the dev cluster.
These nginx modules ( and probably others that I can't remember ) have been exercised in this test and work fine:

@vaijab
Copy link
Contributor

vaijab commented Jan 30, 2017

@stefancocora awesome. Please clean up drone.yaml and this is good to go then! Thanks!

@stefancocora stefancocora force-pushed the update_alpine3.5 branch 2 times, most recently from 21a71a5 to d8dfd3f Compare January 30, 2017 16:10
@stefancocora
Copy link
Contributor Author

.drone.yml has been cleaned up

@vaijab
Copy link
Contributor

vaijab commented Jan 30, 2017

.drone.yml has been cleaned up

Not sure you have. You deleted push master event.

@stefancocora
Copy link
Contributor Author

Something went wrong with my rebase, I'll re-do it.

- the nginx version 1.10.2 in alpine3.5 comes with the
http_realip_module module compiled in. This module is used to
whitelist/blacklist on proxyprotocol set ips from upstream
loadbalancers.
@stefancocora
Copy link
Contributor Author

done, my local branch was detached from the upstream branch that I pushed earlier.

Copy link
Contributor

@vaijab vaijab left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great, thanks. Don't release it yet. CI will build a new image off master anyway if you must use this.

@stefancocora
Copy link
Contributor Author

Ok I'll merge it so that CI bulds an image that I can use straight away.
Any reason why you don't want to release it straight away ?

@stefancocora stefancocora merged commit 61c3277 into master Jan 31, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants