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

ksynth on ec2 instance #61

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

jdziurzynski
Copy link

KNTK-376

Terraform configuration for ksynth agent deployment on AWS ec2 instance.


| Name | Version |
|--------------|----------|
| terraform | >= 1.0.0 |
Copy link
Contributor

@mateuszmidor mateuszmidor Jun 30, 2022

Choose a reason for hiding this comment

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

better peg the major versions to avoid incompatible changes when new major version is released


filter {
name = "name"
values = ["ubuntu/images/hvm-ssd/ubuntu-focal-20.04-amd64-server-*"]
Copy link
Contributor

Choose a reason for hiding this comment

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

Note that AMIs get deprecated and unavailable with time, this may deserve at least a comment

curl -s https://packagecloud.io/install/repositories/kentik/ksynth/script.deb.sh | sudo bash
apt-get install ksynth
echo "KENTIK_COMPANY=${var.plan_id}" >> /etc/default/ksynth
systemctl start ksynth
Copy link
Contributor

Choose a reason for hiding this comment

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

ksynth start-stop-start sequence may deserve a comment

@@ -0,0 +1,12 @@
terraform {
required_version = ">= 1.0"
Copy link
Contributor

Choose a reason for hiding this comment

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

Please peg major versions

}

provider "aws" {
region = "us-east-2"
Copy link
Contributor

Choose a reason for hiding this comment

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

Probably should be a variable

Copy link
Contributor

@mateuszmidor mateuszmidor left a comment

Choose a reason for hiding this comment

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

Looks good

Copy link
Contributor

@mmac-m3a mmac-m3a left a comment

Choose a reason for hiding this comment

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

This won't work as-is. Has anybody tested the TF config?

#!/bin/bash
curl -s https://packagecloud.io/install/repositories/kentik/ksynth/script.deb.sh | sudo bash
apt-get install ksynth
echo "KENTIK_COMPANY=${var.plan_id}" >> /etc/default/ksynth
Copy link
Contributor

Choose a reason for hiding this comment

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

The plan_id variable is misnamed. The KENTIK_COMPANY variable needs to contain company id, which is a different identifier than plan_id. It is also crucial to have an option to provide "kentik environment" parameter (indicating which Kentik instance the agent should join).


user_data = <<EOF
#!/bin/bash
curl -s https://packagecloud.io/install/repositories/kentik/ksynth/script.deb.sh | sudo bash
Copy link
Contributor

Choose a reason for hiding this comment

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

Error handling?

curl -s https://packagecloud.io/install/repositories/kentik/ksynth/script.deb.sh | sudo bash
apt-get install ksynth
echo "KENTIK_COMPANY=${var.plan_id}" >> /etc/default/ksynth
systemctl start ksynth
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 line is definitely not needed.

| Name | Description | Type | Default | Required |
|------------------------|--------------------------------------------------|--------------|---------|----------|
| vpc_security_group_ids | List of security groups IDs | list(string) | | true |
| plan_id | Kentik plan ID | string | | true |
Copy link
Contributor

Choose a reason for hiding this comment

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

plan_id is never needed to run ksynth. This should be company_id.

| vpc_security_group_ids | List of security groups IDs | list(string) | | true |
| plan_id | Kentik plan ID | string | | true |
| region | Specifies AWS provider region | string | | false |
| key_name | Key name of the Key Pair to use for the instance | string | | false |
Copy link
Contributor

Choose a reason for hiding this comment

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

What "key pair" are referring to?

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.

3 participants