-
Notifications
You must be signed in to change notification settings - Fork 465
Conversation
d6021a8
to
bf79aa9
Compare
Thanks for the PR! Im going to run the test suite |
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.
This is fantastic, thank you! Let's hope the tests that @Etiene just kicked off pass :)
- Use [[ ]] and == - Add documentation about systemd/journalctl for logs and new systemd-stdout/systemd-stderr options
I've made the suggested changes, and also removed the creation of the |
Tests are failing to fetch logs ( |
It might be because I removed |
Ok, thanks! This error was already happening before the last commit though. It's probably linked to the stdout and stderr path not being updated on tests on |
Oh I see, thanks! Since the logs are using |
I've updated the tests so that it writes out the Vault logs to a file. I ran the |
@Etiene Could you take over getting this PR passing tests and merged from here? |
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.
Thanks! all tests are passing now. Great PR! 👍
Closes #73 by switching to Amazon Linux 2 and changing to
systemd
.The
vault.service
configuration was based off of the configuration listed at the Vault Deployment Guide.Also, I've updated
README.md
to indicate that its been updated to Amazon Linux 2, but there is a link to the latest public AMIs. That list will need to be updated with new Amazon Linux 2 AMIs if this PR is accepted.I've also updated one of the examples to also install
pip
as it's still required by theterraform-aws-consul
modules to installsupervisord
. I'm looking into updating those modules to usesystemd
as well, so will remove the reference topip
once I have an PR in for those modules.update: I submitted hashicorp/terraform-aws-consul#130, but it seems like
pip
andboto3
is needed for the auth signing script example. Thus, I've updated the PR to install them on Amazon Linux 2 as it seems like they're not already included.