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 examples to non-root nginx-hello #832

Merged
merged 4 commits into from
Feb 4, 2020

Conversation

mohamed-gougam
Copy link
Contributor

@mohamed-gougam mohamed-gougam commented Jan 28, 2020

Proposed changes

This updates all the .yaml files using docker image nginxdemos/hello to use the new non-root image nginxdemos/nginx-hello. Container ports and target ports have been updated to match the image as well. These updates have also been applied for the tests to use the new image.

Checklist

Before creating a PR, run through this checklist and mark each as complete.

  • I have read the CONTRIBUTING doc
  • I have added tests that prove my fix is effective or that my feature works
  • I have checked that all unit tests pass after adding my changes
  • I have updated necessary documentation
  • I have rebased my branch onto master
  • I will ensure my PR is targeting the master branch and pulling from my branch from my own fork

Copy link
Contributor

@pleshakov pleshakov left a comment

Choose a reason for hiding this comment

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

thanks @mohamed-gougam !

lgtm! including @tellet to review the test parts

@pleshakov pleshakov requested a review from tellet January 29, 2020 00:21
tests/data/common/backend1.yaml Outdated Show resolved Hide resolved
Copy link
Contributor

@tellet tellet left a comment

Choose a reason for hiding this comment

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

Sorry I didn't request it in my first review. Could you please revert the changes related to the test files only? I don't see any need in tests using the new image. Again sorry, I didn't bring that up earlier. Feel free to discuss it one again with the team.

Copy link
Contributor

@tellet tellet left a comment

Choose a reason for hiding this comment

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

Please revert changes for backend1-svc.yaml too. Apart from this everything is OK.

@tellet tellet self-requested a review February 4, 2020 07:19
Copy link
Contributor

@tellet tellet left a comment

Choose a reason for hiding this comment

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

lgtm

@pleshakov pleshakov merged commit 2d70062 into nginxinc:master Feb 4, 2020
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.

3 participants