Automatically reboot instance if hung #3

SweetOps · 2017-09-01T13:50:58Z

What

Automatically reboot instance if hung

Why

Requested in task https://app.asana.com/0/415062006611341/414132833064953

const-bon · 2017-09-01T14:58:43Z

README.md

@@ -8,7 +8,7 @@ Note: add `${var.ssh_key_pair}` private key to the `ssh agent`.

 Include this repository as a module in your existing terraform code:

-```
+```terraform
 module "admin_tier" {
  source                      = "git::https://github.com/cloudposse/tf_instance.git?ref=tags/0.1.0"


Replace tags/0.1.0 with master.

Add name, namespace and stage variables.

const-bon · 2017-09-01T14:59:24Z

README.md

-| `subnets`                    | []             | List of VPC Subnet IDs creating instance launched in                    | Yes |
-| `associate_public_ip_address`| `true`         | Associate a public ip address with the creating instance. Boolean value | No  |
+|  Name                        |  Default                                     |  Description                                                                     | Required |
+|:----------------------------:|:--------------------------------------------:|:--------------------------------------------------------------------------------:|:--------:|


Fix table indentation.

Nevermind, it's ok.

const-bon · 2017-09-01T15:16:25Z

variables.tf

@@ -61,3 +61,46 @@ variable "ssh_user" {
 variable "welcome_message" {
  default = ""
 }
+
+variable "comparison_operator" {
+  description = "The arithmetic operation to use when comparing the specified Statistic and Threshold. Possible values are: GreaterThanOrEqualToThreshold, GreaterThanThreshold, LessThanThreshold, LessThanOrEqualToThreshold."


Use these descriptions in README.md.

if add it to readme the line will be too long for table and markdown'll become unreadable

const-bon · 2017-09-01T15:17:03Z

main.tf

+
+resource "aws_cloudwatch_metric_alarm" "reboot" {
+  alarm_name          = "${module.label.id}-status-check-failed"
+  comparison_operator = "${var.comparison_operator}"


Shouldn't we hardcode these variables?
@osterman what do you think?

I probably would have just hard coded it, but since @SweetOps has already parameterized it, I'm okay with it so long as we rename reboot to default or something more general.

osterman · 2017-09-01T17:04:43Z

main.tf

+
+resource "null_resource" "check_alarm" {
+  triggers = {
+    action = "arn:aws:swf:${var.aws_region}:${var.aws_account_id}:${var.default_alarm_action}"


You can use a data source to retrieve aws_region and aws_account_id. No need for additional inputs.

osterman · 2017-09-01T17:08:18Z

README.md

+| `applying_period`            | `60`                                         | Period in seconds over which the specified statistic is applied                  | Yes      |
+| `statistic_level`            | `Maximum`                                    | Statistic to apply to the alarm's associated metric                              | Yes      |
+| `metric_threshold`           | `1`                                          | Value against which the specified statistic is compared                          | Yes      |
+| `default_alarm_action`       |`action/actions/AWS_EC2.InstanceId.Reboot/1.0`|  List of actions to execute when this alarm transitions into an ALARM state      | Yes      |


https://github.com/cloudposse/tf_instance/pull/3/files#diff-c9ac8098c5ea9d3e6a9a596ff0c512a4R100 is not type list

osterman · 2017-09-01T17:10:58Z

main.tf

+}
+
+resource "aws_cloudwatch_metric_alarm" "reboot" {
+  alarm_name          = "${module.label.id}-status-check-failed"


this alarm_name should be default since everything is parameterized and it could mean anything.

SweetOps · 2017-09-05T09:53:28Z

osterman · 2017-09-05T17:27:31Z

README.md

+## Outputs
+
+| Name                | Decription                                                         |
+|:-------------------:|:------------------------------------------------------------------:|


Do not center justify name and description.

osterman · 2017-09-05T17:28:24Z

main.tf

+
+# Restart dead or hung instance
+
+data "aws_region" "current" {


these should be called default

osterman · 2017-09-05T17:28:31Z

main.tf

+  current = true
+}
+
+data "aws_caller_identity" "current" {}


call it default

osterman · 2017-09-05T17:29:20Z

main.tf

+
+data "aws_caller_identity" "current" {}
+
+resource "null_resource" "check_alarm" {


just call it default

osterman · 2017-09-05T17:29:39Z

main.tf

+
+resource "null_resource" "check_alarm" {
+  triggers = {
+    action = "arn:aws:swf:${data.aws_region.current.name}:${data.aws_caller_identity.current.account_id}:${var.default_alarm_action}"


rename action to check_alarm_action

const-bon · 2017-09-06T15:59:11Z

variables.tf

@@ -15,7 +15,7 @@ variable "associate_public_ip_address" {
 }

 variable "ansible_arguments" {
-  type = "list"
+  type    = "list"
  default = []
 }



Remove default values for name, stage and namespace variables.

const-bon · 2017-09-06T16:16:32Z

Tested.

added alarm

4a10450

SweetOps self-assigned this Sep 1, 2017

SweetOps requested review from osterman and const-bon September 1, 2017 13:50

updated README

0300217

const-bon suggested changes Sep 1, 2017

View reviewed changes

reworked aws_cloudwatch_metric_alarm

68668cd

osterman reviewed Sep 1, 2017

View reviewed changes

small fixes

0936caf

osterman reviewed Sep 5, 2017

View reviewed changes

main.tf Outdated

# Restart dead or hung instance

data "aws_region" "current" {

Copy link

Member

osterman Sep 5, 2017

Choose a reason for hiding this comment

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

these should be called default

osterman reviewed Sep 5, 2017

View reviewed changes

main.tf Outdated

current = true

}

data "aws_caller_identity" "current" {}

Copy link

Member

osterman Sep 5, 2017

Choose a reason for hiding this comment

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

call it default

osterman reviewed Sep 5, 2017

View reviewed changes

main.tf Outdated

data "aws_caller_identity" "current" {}

resource "null_resource" "check_alarm" {

Copy link

Member

osterman Sep 5, 2017

Choose a reason for hiding this comment

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

just call it default

osterman reviewed Sep 5, 2017

View reviewed changes

SweetOps added 2 commits September 5, 2017 21:58

renamed resources and reworked readme

b395d82

alignment white space in table for outputs

2d295ef

osterman previously approved these changes Sep 5, 2017

View reviewed changes

const-bon reviewed Sep 6, 2017

View reviewed changes

removed default value for variables name,namespace,stage

4f067e7

SweetOps dismissed osterman’s stale review via 4f067e7 September 6, 2017 16:23

const-bon approved these changes Sep 6, 2017

View reviewed changes

const-bon merged commit fd5c0b4 into master Sep 6, 2017

const-bon deleted the alarm branch September 6, 2017 16:31

SweetOps added a commit that referenced this pull request Nov 24, 2017

typo #3

29178b3


		# Restart dead or hung instance

		data "aws_region" "current" {


		data "aws_caller_identity" "current" {}

		resource "null_resource" "check_alarm" {

Uh oh!

Automatically reboot instance if hung #3

Automatically reboot instance if hung #3

Uh oh!

Conversation

SweetOps commented Sep 1, 2017

What

Why

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SweetOps Sep 1, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SweetOps commented Sep 5, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

const-bon commented Sep 6, 2017

Uh oh!

Uh oh!

SweetOps Sep 1, 2017 •

edited

Loading