Enable tiered storage in AWS via IAM policy #86

vuldin · 2022-10-24T17:36:13Z

This PR adds tiered storage feature in AWS deployments. Once merged, new PRs will be created for adding the same feature to other cloud providers. Tiered storage is disabled by default (tiered_storage_enabled in vars.tf). When tiered storage is enabled, then by default it is enabled across all topics in this PR.

Steps to run

git clone https://github.com/redpanda-data/deployment-automation.git
cd deployment-automation
git checkout enable-si
cd aws
terraform init -upgrade
terraform apply -var "enable_monitoring=false" -var "tiered_storage_enabled=true"
cd ..
ansible-playbook --private-key `cat ~/.ssh/id_rsa.pub | awk '{print $2}'` -i hosts.ini -v ansible/playbooks/provision-node.yml -e '{ "redpanda": { "cluster": { "cloud_storage_segment_max_upload_interval_sec": 30 }}}'

Now connect via ssh to a broker, create a topic, produce some data, and check S3 bucket for remote segments. If you need to retrieve the IP address of one of the nodes, run terraform output.

ssh ubuntu@<redpanda-ec2-ip>
rpk topic create alog
BATCH=$(date) ; printf "$BATCH %s\n" {1..1000} | rpk topic produce alog

Run the last command 20+ times. View contents of the bucket in AWS console to see remote segments.

ansible/playbooks/start-redpanda.yml

vuldin · 2022-10-28T03:11:54Z

~~Question: Should shadow indexing (an enterprise feature) be enabled by default in this project? Right now it is.~~ We decided via slack to have it disabled by default.

ansible/playbooks/start-redpanda.yml

README.md

aws/cluster.tf

aws/vars.tf

rkruze · 2022-10-31T23:45:23Z

Let's rename this to tiered storage instead of SI. As this is what the docs are focused around.

ansible/playbooks/start-redpanda.yml

aws/vars.tf

vuldin · 2022-11-01T14:39:05Z

aws/cluster.tf

-  deployment_id = "redpanda-${local.uuid}-${local.timestamp}"
+  uuid                       = random_uuid.cluster.result
+  timestamp                  = time_static.timestamp.unix
+  deployment_id              = length(var.deployment_prefix) > 0 ? var.deployment_prefix : "redpanda-${substr(local.uuid, 0, 8)}-${local.timestamp}"


Terraform complained about names being too long, so I shortened both the UUID and the timestamp.

how safe is it to just grab the first 8 characters off the uuid? I haven't look at how go-uuid works to see if it's one of the time-based uuids.

also, why do we use a timestamp here instead of just the larger uuid?

aws/provider.tf

vuldin · 2022-11-02T13:22:02Z

I'll squash the PR feedback commits once the PR is approved (let me know if you think this is unneeded).

Allow for configuration of arbitrary node and cluster configuration items. N.B. Further work to be done on idempotence and integration of TLS. #74 and #86 will need some rework

vuldin · 2022-12-08T18:40:04Z

ansible/playbooks/roles/redpanda_broker/templates/configs/tiered_storage.j2

+  cloud_storage_enable_remote_read: true
+  cloud_storage_enable_remote_write: true


This makes all topics tiered-storage-enabled by default if the tiered_storage_enabled terraform variable is enabled.

vuldin · 2022-12-08T18:41:45Z

ansible/playbooks/roles/redpanda_broker/vars/main.yml

+    condition: "{{ tls | default(False) | bool }}"
+  - template: configs/tiered_storage.j2
+    condition: "{{ tiered_storage_bucket_name is defined | default(False) | bool }}"


The tiered_storage_bucket_name ansible variable is pulled from hosts.ini, and is only defined if the tiered_storage_enabled terraform variable is true.

vuldin · 2022-12-08T18:43:02Z

aws/cluster.tf

+          "s3:*",
+          "s3-object-lambda:*",


This needs to be more limited to list only those needed permissions.

Discussed this with the broader team. From that conversation:

Having an policy that tightly limits redpanda to its own bucket is clearly the right thing to do, it's less obvious to me that restricting the verbs is helpful: it creates maintenance burden to keep those up to date as/when we change redpanda, and doesn't meaningfully change security if redpanda is the owner of the bucket.

So limiting this policy to the specific bucket (as shown in this file below) is the best approach, and avoids a maintenance burden as tiered storage capabilities are expanded.

vuldin · 2022-12-08T19:18:09Z

ansible/playbooks/roles/redpanda_broker/templates/configs/defaults.j2

@@ -10,7 +10,7 @@ node:
    - address: {{ hostvars[inventory_hostname].advertised_ip }}
      port: {{ redpanda_kafka_port }}
    advertised_rpc_api:
-    - address: {{ hostvars[inventory_hostname].advertised_ip }}
+      address: {{ hostvars[inventory_hostname].advertised_ip }}


This resulted in an error due to the fact that advertised_rpc_api (unlike other listeners) isn't a list.

tmgstevens · 2022-12-19T15:48:27Z

ansible/playbooks/roles/redpanda_broker/templates/configs/tiered_storage.j2

+  cloud_storage_enable_remote_write: true
+  cloud_storage_region: {{ aws_region if aws_region is defined }}
+  cloud_storage_secret_key: THISVALUENOTUSED
+  cloud_storage_credentials_source: aws_instance_metadata


Need to pass this in as a param from terraform otherwise we're assuming AWS only

hcoyote · 2023-01-13T00:04:20Z

Temporarily cherry picked the ansible2.14+ node_exporter fixes into my local repo in order to get this to run. fwiw, it seems to run ok with the ansible 7.1.0 bottle in homebrew (or, at least, it's behaving the same way so far as other clusters I've run this week from a cluster provisioning standpoint).

hcoyote

added some questions for clarity, I don't see anything specific that should hold this back as is.

hcoyote · 2023-01-13T17:17:26Z

ansible/playbooks/roles/redpanda_broker/templates/configs/tiered_storage.j2

@@ -0,0 +1,10 @@
+cluster:
+  cloud_storage_access_key: THISVALUENOTUSED


can you help me understand why this is here if it's not used? Are we expecting users to override this with their own storage key in the TF config?

No, this is just a placeholder. Unfortunately Redpanda requires this to be something even when we are using aws_instance_metadata (IAM permissions applied to the EC2 instance).

hcoyote · 2023-01-13T17:57:04Z

aws/cluster.tf

-  deployment_id = "redpanda-${local.uuid}-${local.timestamp}"
+  uuid                       = random_uuid.cluster.result
+  timestamp                  = time_static.timestamp.unix
+  deployment_id              = length(var.deployment_prefix) > 0 ? var.deployment_prefix : "redpanda-${substr(local.uuid, 0, 8)}-${local.timestamp}"


also, why do we use a timestamp here instead of just the larger uuid?

hcoyote · 2023-01-13T18:00:14Z

aws/s3.tf

+resource "aws_s3_bucket_versioning" "tiered_storage" {
+  count  = var.tiered_storage_enabled ? 1 : 0
+  bucket = aws_s3_bucket.tiered_storage[count.index].id
+  versioning_configuration {
+    status = "Disabled"
+  }
+}


Can we get a comment added on here on why we disable versioning (and maybe what happens, good or bad, if the user overrides this)?

tmgstevens · 2023-01-18T17:59:45Z

Thanks for the review @hcoyote

vuldin added enhancement New feature or request v2-branch Will be addressed in v2 branch labels Oct 24, 2022

vuldin requested a review from r-vasquez October 24, 2022 17:36

vuldin commented Oct 24, 2022

View reviewed changes

ansible/playbooks/start-redpanda.yml Outdated Show resolved Hide resolved

vuldin linked an issue Oct 24, 2022 that may be closed by this pull request

Enable SI on AWS #87

Closed

vuldin mentioned this pull request Oct 24, 2022

s3 setup for archival storage #24

Closed

vuldin force-pushed the enable-si branch 2 times, most recently from 43cceed to 4847065 Compare October 28, 2022 03:00

vuldin marked this pull request as ready for review October 28, 2022 03:00

vuldin force-pushed the enable-si branch from 4847065 to c3f9da4 Compare October 28, 2022 03:35

vuldin commented Oct 28, 2022

View reviewed changes

ansible/playbooks/start-redpanda.yml Outdated Show resolved Hide resolved

vuldin requested a review from tmgstevens October 28, 2022 03:42

vuldin force-pushed the enable-si branch 2 times, most recently from e1ed30f to 021416b Compare October 31, 2022 15:14

vuldin commented Oct 31, 2022

View reviewed changes

README.md Outdated Show resolved Hide resolved

rkruze reviewed Oct 31, 2022

View reviewed changes

aws/cluster.tf Show resolved Hide resolved

rkruze reviewed Oct 31, 2022

View reviewed changes

aws/vars.tf Show resolved Hide resolved

r-vasquez reviewed Nov 1, 2022

View reviewed changes

ansible/playbooks/start-redpanda.yml Outdated Show resolved Hide resolved

ansible/playbooks/start-redpanda.yml Outdated Show resolved Hide resolved

tmgstevens reviewed Nov 1, 2022

View reviewed changes

aws/vars.tf Outdated Show resolved Hide resolved

vuldin added a commit that referenced this pull request Nov 1, 2022

PR #86 feedback 1

c4ada28

vuldin commented Nov 1, 2022

View reviewed changes

vuldin added a commit that referenced this pull request Nov 1, 2022

PR #86 feedback 1

4c1953e

vuldin force-pushed the enable-si branch from c4ada28 to 4c1953e Compare November 1, 2022 14:41

vuldin changed the title ~~Enable shadow indexing in AWS~~ Enable tiered storage in AWS via IAM policy Nov 1, 2022

vuldin removed the v2-branch Will be addressed in v2 branch label Nov 1, 2022

vuldin added a commit that referenced this pull request Nov 1, 2022

PR #86 feedback 2

23ce1d7

tmgstevens reviewed Nov 2, 2022

View reviewed changes

aws/provider.tf Show resolved Hide resolved

vuldin added a commit that referenced this pull request Nov 2, 2022

PR #86 feedback 3

0f1ef4d

tmgstevens added a commit that referenced this pull request Nov 9, 2022

Extensible configuration (#90)

be48d76

Allow for configuration of arbitrary node and cluster configuration items. N.B. Further work to be done on idempotence and integration of TLS. #74 and #86 will need some rework

vuldin force-pushed the enable-si branch from 0f1ef4d to 2263a05 Compare December 8, 2022 18:32

vuldin commented Dec 8, 2022

View reviewed changes

enable tiered storage in AWS via IAM policy

d42acae

vuldin force-pushed the enable-si branch from 2263a05 to d42acae Compare December 8, 2022 18:50

vuldin commented Dec 8, 2022

View reviewed changes

tmgstevens reviewed Dec 19, 2022

View reviewed changes

WesWWagner linked an issue Jan 10, 2023 that may be closed by this pull request

s3 setup for archival storage #24

Closed

WesWWagner self-requested a review January 10, 2023 17:04

hcoyote self-requested a review January 12, 2023 23:59

hcoyote approved these changes Jan 13, 2023

View reviewed changes

Merge branch 'main' into enable-si

a94fa72

tmgstevens merged commit 231ee44 into main Jan 18, 2023

gene-redpanda deleted the enable-si branch March 9, 2023 16:33

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Enable tiered storage in AWS via IAM policy #86

Enable tiered storage in AWS via IAM policy #86

vuldin commented Oct 24, 2022 •

edited

vuldin commented Oct 28, 2022 •

edited

rkruze commented Oct 31, 2022

vuldin Nov 1, 2022

hcoyote Jan 13, 2023

hcoyote Jan 13, 2023

vuldin commented Nov 2, 2022

vuldin Dec 8, 2022

vuldin Dec 8, 2022

vuldin Dec 8, 2022

vuldin Dec 8, 2022

vuldin Dec 8, 2022

tmgstevens Dec 19, 2022

hcoyote commented Jan 13, 2023

hcoyote left a comment

hcoyote Jan 13, 2023

vuldin Jan 17, 2023

hcoyote Jan 13, 2023

hcoyote Jan 13, 2023

tmgstevens commented Jan 18, 2023

		cloud_storage_enable_remote_read: true
		cloud_storage_enable_remote_write: true

		@@ -0,0 +1,10 @@
		cluster:
		cloud_storage_access_key: THISVALUENOTUSED

Enable tiered storage in AWS via IAM policy #86

Enable tiered storage in AWS via IAM policy #86

Conversation

vuldin commented Oct 24, 2022 • edited

Steps to run

vuldin commented Oct 28, 2022 • edited

rkruze commented Oct 31, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vuldin commented Nov 2, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hcoyote commented Jan 13, 2023

hcoyote left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tmgstevens commented Jan 18, 2023

vuldin commented Oct 24, 2022 •

edited

vuldin commented Oct 28, 2022 •

edited