-
Notifications
You must be signed in to change notification settings - Fork 10
Add s3 reliability test #44
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
base: master
Are you sure you want to change the base?
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,6 @@ | ||
| --- | ||
| - name: Install Vector configuration | ||
| template: | ||
| copy: | ||
| src: "{{ configuration_file }}" | ||
| dest: /etc/vector/vector.toml | ||
| mode: 0644 | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,21 @@ | ||
| --- | ||
| - name: Create verifiable-logger directory | ||
| file: | ||
| path: /var/lib/verifiable-logger | ||
| state: directory | ||
|
|
||
| - name: Copy verifiable-logger binary | ||
| get_url: | ||
| url: "https://verifiable-logger-builds.s3.us-east-2.amazonaws.com/verifiable-logger" | ||
| dest: /usr/local/bin/verifiable-logger | ||
| mode: 755 | ||
|
|
||
| - name: Copy generate-logs unit file | ||
| template: | ||
| src: generate-logs.service | ||
| dest: /etc/systemd/system/generate-logs.service | ||
|
|
||
| - name: Copy verify-logs unit file | ||
| template: | ||
| src: verify-logs.service | ||
| dest: /etc/systemd/system/verify-logs.service |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,2 @@ | ||
| --- | ||
| - include: "{{ action }}.yml" |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,6 @@ | ||
| --- | ||
| - name: Start generating logs | ||
| systemd: | ||
| name: generate-logs | ||
| state: started | ||
| daemon-reload: yes |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,6 @@ | ||
| --- | ||
| - name: Start verifying logs | ||
| systemd: | ||
| name: verify-logs | ||
| state: started | ||
| daemon-reload: yes |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,6 @@ | ||
| [Unit] | ||
| Description=generate-logs | ||
|
|
||
| [Service] | ||
| ExecStart=/usr/local/bin/verifiable-logger generate --rate 10 --output output.log | ||
| WorkingDirectory=/var/lib/verifiable-logger/ |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,6 @@ | ||
| [Unit] | ||
| Description=verify-logs | ||
|
|
||
| [Service] | ||
| ExecStart=/usr/local/bin/verifiable-logger verify file-to-s3-reliability-test-data us-east-1 --prefix "host=%H" --tail | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is pretty hardcoded right now, but should obviously be templated in once we figure out the right way to get the variables in here.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would highly recommend not adding support for environment variables in Systemd. I also ran into weird issues when I passed more than one flag. It was a mess. But if you look at the |
||
| WorkingDirectory=/var/lib/verifiable-logger/ | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,25 @@ | ||
| # File to S3 Reliability Test | ||
|
|
||
| This is a long-running test of Vector tailing a file and sending the data to S3. | ||
| It uses the [`verifiable-logger`][0] project both to generate the log data and | ||
| verify that it all reaches S3. | ||
|
|
||
| ## Try it | ||
|
|
||
| You can run this test via: | ||
|
|
||
| ``` | ||
| test -t file_to_s3_reliability | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. One thing missing here: how do we shut down the test and destroy all the resources?
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Currently, we kill the VMs via CloudWatch alert. We need a better way, maybe could use lambda for it.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, we should add a
What's wrong with CloudWatch? I think it works quite well for this.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. CloudWatch works! So this is where we use lambda: we can assign the invocation of I think this solution can replace our CloudWatch VMs removal because we can clean up everything, including the VMs!
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This should solve it for now: #52 |
||
| ``` | ||
|
|
||
| ## Resources | ||
|
|
||
| * [Setup][setup] | ||
| * [Development][development] | ||
| * [How it works][how_it_works] | ||
| * [Vector docs][docs] | ||
| * [Vector repo][repo] | ||
| * [Vector website][website] | ||
|
|
||
|
|
||
| [0]: https://github.com/timberio/verifiable-logger | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,21 @@ | ||
| --- | ||
| - hosts: '{{ test_namespace }}' | ||
| tasks: | ||
| - meta: refresh_inventory | ||
| - name: Wait 600 seconds for target connection to become reachable/usable | ||
| wait_for_connection: | ||
|
|
||
| - debug: | ||
| var: playbook_dir | ||
|
|
||
| - hosts: '{{ test_namespace }}:&tag_TestRole_subject' | ||
| become: true | ||
| roles: | ||
| - role: vector | ||
| action: install | ||
|
|
||
| - role: vector | ||
| action: configure | ||
|
|
||
| - role: verifiable-logger | ||
| action: install |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,16 @@ | ||
| data_dir = "/var/lib/vector" | ||
|
|
||
| [sources.file] | ||
| type = "file" | ||
| include = ["/var/lib/verifiable-logger/output.log"] | ||
| start_at_beginning = true | ||
|
|
||
| [sinks.s3] | ||
| inputs = ["file"] | ||
| type = "aws_s3" | ||
| region = "us-east-1" | ||
| bucket = "file-to-s3-reliability-test-data" | ||
|
Comment on lines
+11
to
+12
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Again, these should be templated in somehow.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm ok with this stuff being hard coded since it's all contained within this test. We could use variables (this is what the
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The problem is S3 bucket names have global scope, so we either have to use a different one each test run, or make this part of the state global. I think making it global is worse than templating the names. |
||
| key_prefix = "host={{ host }}/date=%F/" | ||
| encoding = "text" | ||
| compression = "none" | ||
| batch.timeout_secs = 30 | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,12 @@ | ||
| --- | ||
| - hosts: '{{ test_namespace }}:&tag_TestRole_subject' | ||
| become: true | ||
| roles: | ||
| - role: verifiable-logger | ||
| action: start-generating | ||
|
|
||
| - role: verifiable-logger | ||
| action: start-verifying | ||
|
|
||
| - role: vector | ||
| action: start |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,2 @@ | ||
| --- | ||
| foo: "bar" | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It didn't let me not have this file. Presumably, we could use this for things like the bucket name but a few things weren't clear:
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
We do the templating at part of |
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,6 @@ | ||
| # WARNING! | ||
| # | ||
| # Do not modify the parameters of this file since historical test results are | ||
| # based on these parameters. Please create a new configuration and specify taht | ||
| # configuration instead. | ||
| subject_instance_type = "c5.large" |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,70 @@ | ||
| provider "aws" { | ||
| region = "us-east-1" | ||
| version = "~> 2.53" | ||
| } | ||
|
|
||
| terraform { | ||
| required_version = ">= 0.12" | ||
| backend "s3" {} | ||
| } | ||
|
|
||
| module "topology" { | ||
| source = "../../../terraform/aws_uni_topology" | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Had to change this from the case I copied to get things working. |
||
|
|
||
| providers = { | ||
| aws = aws | ||
| } | ||
|
|
||
| pub_key = var.pub_key | ||
| subject_instance_type = var.subject_instance_type | ||
| test_configuration = var.test_configuration | ||
| test_name = var.test_name | ||
| user_id = var.user_id | ||
| results_s3_bucket_name = var.results_s3_bucket_name | ||
| } | ||
|
|
||
| resource "aws_s3_bucket" "logs-bucket" { | ||
| # data is namespaced by host within the bucket | ||
| bucket = "file-to-s3-reliability-test-data" | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Again, should be templated.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, I would template this so it's namespaced like our instance names. Ex: |
||
|
|
||
| lifecycle_rule { | ||
| enabled = true | ||
|
|
||
| expiration { | ||
| days = 14 | ||
| } | ||
| } | ||
| } | ||
|
|
||
| data "aws_iam_policy_document" "logs-bucket-policy" { | ||
| statement { | ||
| sid = "AllowTestHarnessListBucket" | ||
|
|
||
| actions = [ | ||
| "s3:ListBucket", | ||
| ] | ||
|
|
||
| resources = [ | ||
| aws_s3_bucket.logs-bucket.arn, | ||
| ] | ||
| } | ||
|
|
||
| statement { | ||
| sid = "AllowTestHarnessEverythingElse" | ||
|
|
||
| actions = [ | ||
| "s3:GetObject", | ||
| "s3:PutObject", | ||
| "s3:DeleteObject", | ||
| ] | ||
|
|
||
| resources = [ | ||
| "${aws_s3_bucket.logs-bucket.arn}/*", | ||
| ] | ||
| } | ||
| } | ||
|
|
||
| resource "aws_iam_role_policy" "default" { | ||
| role = module.topology.instance_profile_name | ||
| policy = data.aws_iam_policy_document.logs-bucket-policy.json | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,28 @@ | ||
| variable "pub_key" { | ||
| type = string | ||
| } | ||
|
|
||
| variable "subject_instance_type" { | ||
| type = string | ||
| } | ||
|
|
||
| variable "test_configuration" { | ||
| type = string | ||
| } | ||
|
|
||
| variable "test_name" { | ||
| type = string | ||
| } | ||
|
|
||
| variable "user_id" { | ||
| type = string | ||
| } | ||
|
|
||
| variable "test_harness_aws_account_ids" { | ||
| type = list(string) | ||
| } | ||
|
lukesteensen marked this conversation as resolved.
Outdated
|
||
|
|
||
| // don't actually need this, but need to accept it | ||
| variable "results_s3_bucket_name" { | ||
| type = string | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,3 @@ | ||
| terraform { | ||
| required_version = ">= 0.12" | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -11,7 +11,7 @@ locals { | |
| } | ||
|
|
||
| module "vpc" { | ||
| source = "../../../terraform/aws_vpc" | ||
| source = "../aws_vpc" | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not sure if I was doing something wrong, but nothing worked at all without changing these paths.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is odd. Test harness seems to work on
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Checked, doesn't work on my end either. Created #45. |
||
|
|
||
| providers = { | ||
| aws = aws | ||
|
|
@@ -24,7 +24,7 @@ module "vpc" { | |
| } | ||
|
|
||
| module "aws_instance_profile" { | ||
| source = "../../../terraform/aws_instance_profile" | ||
| source = "../aws_instance_profile" | ||
|
|
||
| providers = { | ||
| aws = aws | ||
|
|
@@ -36,7 +36,7 @@ module "aws_instance_profile" { | |
| } | ||
|
|
||
| module "aws_instance_subject" { | ||
| source = "../../../terraform/aws_instance" | ||
| source = "../aws_instance" | ||
|
|
||
| providers = { | ||
| aws = aws | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,3 @@ | ||
| output "instance_profile_name" { | ||
| value = module.aws_instance_profile.name | ||
| } |
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 to get around an issue where
templatetries to fill in the vector config's templates. We can probably get around it with escaping, but this seemed safer.It also opens the question of, if we actually wanted to template some things in, how would we do it?
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.
Good point! We can use different
variable_end_stringandvariable_start_stringwhen resolving vector templates.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.
Agree, @lukesteensen we use templates to insert addresses, ports, etc, so we'll need to revert this. I'll try to submit a PR today that changes this to use different
variable_end_stringandvariable_start_stringvalues.