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 nginx dependencies #2588

Merged
merged 1 commit into from
May 31, 2018
Merged

Conversation

aledbf
Copy link
Member

@aledbf aledbf commented May 31, 2018

What this PR does / why we need it:

Add required module for #2586

Which issue this PR fixes: fixes #2547

@k8s-ci-robot k8s-ci-robot added approved Indicates a PR has been approved by an approver from all required OWNERS files. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels May 31, 2018
Copy link
Contributor

@toolboc toolboc left a comment

Choose a reason for hiding this comment

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

Install of lua-cjson should be done in install_lua_resty_waf.sh

@aledbf
Copy link
Member Author

aledbf commented May 31, 2018

@toolboc we need that module installed in the base image independently of the lua waf module
(is going to be used for the prometheus refactoring I am working on)

@codecov-io
Copy link

codecov-io commented May 31, 2018

Codecov Report

Merging #2588 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #2588   +/-   ##
=======================================
  Coverage   40.54%   40.54%           
=======================================
  Files          75       75           
  Lines        5105     5105           
=======================================
  Hits         2070     2070           
  Misses       2754     2754           
  Partials      281      281

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ded8c15...b0643e4. Read the comment docs.

luarocks install lrexlib-pcre 2.7.2-1
fi

luarocks install lua-cjson
Copy link
Member

@ElvinEfendi ElvinEfendi May 31, 2018

Choose a reason for hiding this comment

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

Copy link
Member Author

Choose a reason for hiding this comment

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

It's ok if I remove that and we use luarocks? For some reason that package is not being installed in arm

Copy link
Member

Choose a reason for hiding this comment

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

oh interesting, yeah we can use luarocks instead, but could you fix version?

Copy link
Member Author

Choose a reason for hiding this comment

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

done

fi

luarocks install lua-cjson
luarocks install luasocket
Copy link
Member

Choose a reason for hiding this comment

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

this should not be necessary for that PR, we already have ngx.socket we can use

Copy link
Member Author

Choose a reason for hiding this comment

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

removed

@ElvinEfendi
Copy link
Member

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 31, 2018
@k8s-ci-robot
Copy link
Contributor

[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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot merged commit aeab200 into kubernetes:master May 31, 2018
@aledbf aledbf deleted the update-nginx branch June 11, 2018 00:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Ingress controller crash on boot - arm arch
5 participants