Skip to content

Adds install of procps to nginx Dockerfile#59

Merged
hbradio merged 3 commits intodevelopmentfrom
bugfix/Nginx_image_start_script_fails
Apr 30, 2017
Merged

Adds install of procps to nginx Dockerfile#59
hbradio merged 3 commits intodevelopmentfrom
bugfix/Nginx_image_start_script_fails

Conversation

@hbradio
Copy link
Copy Markdown
Collaborator

@hbradio hbradio commented Apr 29, 2017

Not sure what changed about the official nginx Docker image, but suddenly ps was unavailable.

@hbradio hbradio added this to the Release 0.4 milestone Apr 29, 2017
@hbradio hbradio requested a review from cabarnes April 29, 2017 21:33
@coveralls
Copy link
Copy Markdown

coveralls commented Apr 29, 2017

Coverage Status

Coverage remained the same at 100.0% when pulling c1ccd30 on bugfix/Nginx_image_start_script_fails into f49362d on development.

cabarnes
cabarnes reviewed Apr 29, 2017
View reviewed changes
Copy link
Copy Markdown
Member

@cabarnes cabarnes left a comment

Choose a reason for hiding this comment

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

One of the things you could do instead of having to install it is to lock down the version of the nginx image so that things like this don't happen unexpectedly.

@hbradio
Copy link
Copy Markdown
Collaborator Author

hbradio commented Apr 30, 2017

I was just thinking about that. Do you think locking in the nginx image version is the better choice here?

@cabarnes
Copy link
Copy Markdown
Member

cabarnes commented Apr 30, 2017

I think it depends on how stable you think things should be right now. If you still feel like heavy development is going on, then maybe we should just fix this and get to a newer working version of nginx. But, I don't think newer versions of nginx are going to make much of a difference, so I would probably just lock it in to not have to mess with it going forward.

@hbradio
Copy link
Copy Markdown
Collaborator Author

hbradio commented Apr 30, 2017

Agreed. Locking in nginx version now.

@coveralls
Copy link
Copy Markdown

coveralls commented Apr 30, 2017

Coverage Status

Coverage remained the same at 100.0% when pulling df11e49 on bugfix/Nginx_image_start_script_fails into f49362d on development.

cabarnes
cabarnes approved these changes Apr 30, 2017
View reviewed changes
Copy link
Copy Markdown
Member

@cabarnes cabarnes left a comment

Choose a reason for hiding this comment

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

LGTM. Might want to change the description of the PR.

@hbradio hbradio merged commit 897f2db into development Apr 30, 2017
@cabarnes cabarnes deleted the bugfix/Nginx_image_start_script_fails branch May 23, 2017 14:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@cabarnes cabarnes cabarnes approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

Release 0.4

Development

Successfully merging this pull request may close these issues.

3 participants

@hbradio @coveralls @cabarnes