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

fix(example): render nginx port to config #19

Merged
merged 10 commits into from
Oct 17, 2024
Merged

Conversation

thevilledev
Copy link
Contributor

@thevilledev thevilledev commented Oct 17, 2024

Contains three individual fixes:

  1. Since feat(image): make Nginx TCP port configurable hello-container#3 the hello-container's sample nginx configuration file "index.conf" has been changed to a template. It is not a configuration file that works out of the box as something is expected to render the listening port into the configuration file. Due to this change the example code for a rolling update through the Ansible inventory plugin does not work.

This PR replaces the variable with a default port of 80, through a regexp match, and does not introduce any template rendering.

  1. While at it I also fixed the README in the repo example. At the moment terraform output does not return a raw string, leading to other commands in the tutorial to fail.
$ curl $(terraform -chdir=resources output lb_url)
curl: (3) URL rejected: Bad hostname

This works after returning a raw string representation of the LB URL.

  1. Upon successful ansible runs, the unarchive step does not overwrite existing files. Thus, changing the animal parameter from cow to tiger doesn't affect the files on disk. With this PR, the nginx content directory is cleaned up before unarchiving.

@thevilledev thevilledev requested a review from a team as a code owner October 17, 2024 09:26
@thevilledev thevilledev merged commit 8cb0a44 into main Oct 17, 2024
8 checks passed
@thevilledev thevilledev deleted the fix/example-nginx-config branch October 17, 2024 12:11
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