Skip to content
This repository has been archived by the owner on Jan 31, 2019. It is now read-only.

extend ES support; roles, etc #132

Merged
merged 3 commits into from May 22, 2015

Conversation

phrawzty
Copy link
Contributor

There are ASG and LC resource blocks for each of the three types of ES role (master, interface, and data, respectively). I had to pass this bit of information to userdata somehow, so I exploited the socorro_role positional argument to pass additional data (i.e. 'elasticsearch FACTER_elasticsearch_role=master'). This is gross but it works for now. 馃槖

As a side-effect, I appear to have proposed a model for splitting out values based on environment, as seen in variables.tf. It's functional and readable, so that's nice, but I'm not married to it.

Finally, I had to change the Terraform labels to socorroelasticsearch2 so that the resources wouldn't collide with the existing socorroelasticsearch resources; before we merge, the latter resources will need to be removed entirely and the former will be renamed as appropriate.

I tested this against AMI ID ami-2fe2df1f (built from this branch) and the nodes came up, ES started, discovery occurred, and the cluster came up 馃崗 as expected.

It's also worth noting that this PR introduces another IAM role specific to Elasticsearch (for enabling auto-discovery). As we are not currently managing IAM via Terraform, the details of that role are not present in this PR - a conversation for another time, perhaps. 馃槈

This PR also satisfies bug 1167134 by installing OpenJDK 1.8 in the Base AMI.

r? @rhelmer @jdotpz

@phrawzty
Copy link
Contributor Author

Travis is failing on 2a61e1e because I haven't modified the test to supply the appropriate FAKE data. I'll push a fix for this soon.

package {
'elasticsearch-plugin-cloud-aws':
ensure => latest,
require => Package['elasticsearch']
Copy link
Contributor

Choose a reason for hiding this comment

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

Would it make more sense to put this in the role specific to ES? No other nodes will need it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We put everything in the base package - roles are for service configuration only.

... in fact I'm pretty sure that you're the one who told me that. 馃槃

Copy link
Contributor

Choose a reason for hiding this comment

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

At the time I was talking about on buildbox, but this can't really hurt anything for base, so that works for me. :)

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah we don't need Java or ES everywhere either but we are just baking everything into the AMI and only starting services/dropping in files at deploy time. Sorta immutable infra :)

@phrawzty
Copy link
Contributor Author

Concerning the Instance types and counts, those are preliminary and we'll need to do some testing and tuning. Same goes for the Elasticsearch config itself - the current config is functional but not optimal.

associate_public_ip_address = true
security_groups = [
"${aws_security_group.elb-socorroelasticsearch-sg.id}",
"${aws_security_group.ec2-socorroelasticsearch-sg.id}"
"${aws_security_group.elb-socorroelasticsearch2-sg.id}",
Copy link
Contributor

Choose a reason for hiding this comment

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

This is going to add EC2 servers to an ELB security group. Instead, you can add ingress rules to the ec2-sg allowing ports from the elb-sg.

An example of that is : https://github.com/mozilla/socorro-infra/blob/master/terraform/buildbox/main.tf#L60
(Same comment on the other launch configs)

@jdotpz
Copy link
Contributor

jdotpz commented May 22, 2015

Enthusiastic r+

phrawzty added a commit that referenced this pull request May 22, 2015
@phrawzty phrawzty merged commit a08dd37 into mozilla:master May 22, 2015
@phrawzty phrawzty deleted the bug1118471__es_is_bae branch May 22, 2015 17:26
@jdotpz
Copy link
Contributor

jdotpz commented May 22, 2015

dance

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants