-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
include lua-upstream-nginx-module in Nginx build #2163
Conversation
/assign @aledbf |
images/nginx/build.sh
Outdated
libperl-dev \ | ||
cmake \ | ||
util-linux \ | ||
lua5.1 liblua5.1-0 liblua5.1-dev \ |
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.
As we are using Luajit now these are not needed. I moved to the rest of the packages to Dockerfile so that it is cached.
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.
Please don't remove this because luajit is not available in ppc64le and s390x
images/nginx/build.sh
Outdated
lua-cjson \ | ||
|| exit 1 | ||
|
||
ln -s /usr/lib/x86_64-linux-gnu/liblua5.1.so /usr/lib/liblua.so |
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.
I deleted this as well because if the same reason as above
Codecov Report
@@ Coverage Diff @@
## master #2163 +/- ##
==========================================
+ Coverage 36.44% 36.45% +<.01%
==========================================
Files 69 69
Lines 4862 4861 -1
==========================================
Hits 1772 1772
+ Misses 2820 2819 -1
Partials 270 270
Continue to review full report at Codecov.
|
@ElvinEfendi please don't change the package installation from the build.sh to the Dockerfile. We always want the latest packages and moving the files only increases the size of the image. |
@aledbf I reverted the extra changes and The PR is now only about adding lua-ngx-upstream-module. |
@ElvinEfendi I think you also removed the |
add6041
to
898aac5
Compare
@aledbf should be good now :) |
/approve |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: aledbf, ElvinEfendi The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
@ElvinEfendi thanks! |
What this PR does / why we need it:
This is a follow up for #2155. We need lua-ngx-upstream-module to expose related Nginx C API to Lua land. This lets us implement any custom load balancing algorithm using Lua.
Which issue this PR fixes:
n/a
Special notes for your reviewer:
Here is https://github.com/Shopify/ingress/pull/16/files#diff-b00d77a6df9c8c05a483044b08e6bc50R2 how I use this API.