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.

Sign up for GitHub

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

Jump to bottom

Add support for RHEL7/Systemd in Terraform example #1629

Merged
merged 1 commit into from
Mar 10, 2016
Merged

Add support for RHEL7/Systemd in Terraform example #1629

merged 1 commit into from
Mar 10, 2016

Conversation

moofish32
Copy link
Contributor

This change started out as a quick update to RHEL 7 support (aka systemd), in
the process I realized most of the other platforms could use an update. While
trying to cleanup there I discovered I was repeating of bunch of information
that might be better maintained in one place - as a result:

  • consolidated server.sh and install.sh
  • removed upstart-join.conf in a favor of join flag in the consul start
  • removed platform specific folders and increased complexity of install.sh to
    include handling the differences
  • updated and extracted consul version
  • added a specific condition for centos6 - which is not ideal

@slackpad
Copy link
Contributor

slackpad commented Feb 2, 2016

Hi @moofish32 thanks for updating these! If you've gotten to test them then we can merge them in case they are useful to others. Only thing I saw from a quick look is that GOMAXPROCS doesn't need to be set any more with Go 1.5+.

@moofish32
Copy link
Contributor Author

@slackpad - I need to go back through and plum the rhel7 variable and a few others. I will also kill the GOMAXPROCS

Thinking about this again though I'll probably call it centos7. It will take a couple of days to get the free time, in a bit of the busy season over here (as if there is a non-busy season).

@slackpad slackpad added the type/docs Documentation needs to be created/updated/clarified label Feb 6, 2016
@moofish32 moofish32 force-pushed the update_terraform_rhel7 branch from 1ba9018 to 9361a9b Compare February 6, 2016 16:31
@moofish32
Copy link
Contributor Author

@slackpad - so this change drifted a little. When I started I was going after just one and then I found a few others. In an effort to slim down I consolidated scripts. If you don't like this, let's just roll it back and I'll add only the rhel7/centos7 piece. I was a little annoyed I had to subscribe to use the centos AMI.

I'm looking into build issue with sudo. I may need to back track a bit.

@moofish32 moofish32 force-pushed the update_terraform_rhel7 branch from 9361a9b to 3133b15 Compare February 6, 2016 17:27
@moofish32
Copy link
Contributor Author

rebased test passing

@slackpad
Copy link
Contributor

Hi @moofish32 it looks like you need another rebase. If you were able to test the older scripts then we can update them all, but if not I'd prefer if we just updated the version 7 ones.

@moofish32 moofish32 force-pushed the update_terraform_rhel7 branch from 3133b15 to f1e1532 Compare February 13, 2016 14:30
@moofish32
Copy link
Contributor Author

@slackpad rebased to master. A couple quick items:

  • Tested - I ran the terraform script for each distro and verified the cluster was up with the right number of members. If there is a better way to test let me know.
  • I updated all the Consul versions to 0.6.3, the PR you merged modified many to 0.5.x, was that intentional?
  • The "big change" here is I refactored the bash installers into one script. IMHO I think it's easier to maintain.

@moofish32 moofish32 force-pushed the update_terraform_rhel7 branch from f1e1532 to 8be6a7c Compare February 13, 2016 15:05
@moofish32
Copy link
Contributor Author

@slackpad -- I am getting intermittent builds locally and on Travis. Is that new?

@slackpad
Copy link
Contributor

Hi @moofish32 moving up to latest Consul is definitely the right thing - sorry I regressed that with the older PR. We have a few tests that are timing dependent that are a little flaky and we are working on those. Your changes definitely don't interact with the unit tests :-)

I was looking through old issues and found this one that we might want to roll into this PR since you're in here - #1304. You definitely want all three (8300 for RPC, 8301 for LAN, 8302 for WAN) to be exposed for servers and (8301 for LAN) for clients, though if things are working perhaps these are getting exposed anyway? The comment about a reboot seems apt, too. Would it be possible for you to take a look at that?

@moofish32
Copy link
Contributor Author

@slackpad -- I was officially dominated by the linux distros today. I'll have a fix up in the next couple of days for all the issues you mentioned, but there is just enough difference between upstart in Ubuntu, Centos, and RHEL that I may have to move those back to separate files :(

I thought about rendering a template inside a remote exec to append to file, but need to kick the tires one more time.

@moofish32 moofish32 force-pushed the update_terraform_rhel7 branch 2 times, most recently from 101eaa7 to e77c23d Compare February 20, 2016 17:25
@moofish32
Copy link
Contributor Author

@slackpad -- All four OS variants are now tested and can be restarted. Obviously there is still an anchor dependency on the consul-0 host because that is the default join address. Please review and let me know if you find anything else.

@moofish32 moofish32 force-pushed the update_terraform_rhel7 branch from e77c23d to ca828d5 Compare February 26, 2016 17:39
@moofish32
Copy link
Contributor Author

@slackpad -- rebased to latest master

@moofish32 moofish32 force-pushed the update_terraform_rhel7 branch 3 times, most recently from 8b7d075 to 18acc72 Compare March 3, 2016 15:48
@moofish32 moofish32 force-pushed the update_terraform_rhel7 branch 2 times, most recently from 858928d to 7720fd1 Compare March 8, 2016 15:48
@slackpad
Copy link
Contributor

@moofish32 thanks for all the work on this one! Looks good except we should knock this file out :-)

https://github.com/moofish32/consul/blob/update_terraform_rhel7/terraform/aws/terraform.tfvars

@moofish32
Update quick start AWS Terraform
b4dfd0f 
This change started out as a quick update to RHEL 7 support (aka systemd), in
the process I realized most of the other platforms could use an update. While
trying to cleanup there I discovered I was repeating of bunch of information
that might be better maintained in one place - as a result:
 * consolidated server.sh and install.sh
 * removed upstart-join.conf in a favor of join flag in the consul start
 * removed platform specific folders and increased complexity of install.sh to
   include handling the differences
 * updated and extracted consul version
 * added a consistent ip_table.sh file to open ports on firewalls
 * updating consul service management configurations to enable proper restarting behavior for each platform
 * the configuration naming convention is <distro_origin>_file_name
 * added platform to the security group name so you can easily launch multpile platforms at once
 * fixes hashicorp#1304
@moofish32 moofish32 force-pushed the update_terraform_rhel7 branch from 7720fd1 to b4dfd0f Compare March 10, 2016 14:22
@moofish32
Copy link
Contributor Author

@slackpad -- great catch, bad miss on my part but it was at least only the names; removed file and rebased.

slackpad added a commit that referenced this pull request Mar 10, 2016
@slackpad
Merge pull request #1629 from moofish32/update_terraform_rhel7
0b581b3 
Add support for RHEL7/Systemd in Terraform example
@slackpad slackpad merged commit 0b581b3 into hashicorp:master Mar 10, 2016
@slackpad
Copy link
Contributor

@moofish32 thanks for all your work on this one!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
type/docs Documentation needs to be created/updated/clarified
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@moofish32 @slackpad