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

[geth] add upload/download datadir snapshot from S3 #266

Merged
merged 5 commits into from
Oct 27, 2023

Conversation

VladStarr
Copy link
Contributor

No description provided.

@VladStarr VladStarr requested a review from voron October 26, 2023 16:12
Copy link
Contributor

@voron voron left a comment

Choose a reason for hiding this comment

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

Good staff!
Some minor questions, interesting one is about SA, others are non-blocking.

dysnix/geth/templates/scripts/_s3-cron.tpl Outdated Show resolved Hide resolved
kind: Secret
metadata:
name: {{ include "geth.fullname" . }}-s3-secret
data:
Copy link
Contributor

Choose a reason for hiding this comment

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

WDYT on stringData instead of data ? It may save a couple bytes by removing toString|b64enc

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just left it in data cuz changes to secrets will be visible in helm diff

Copy link
Contributor

@voron voron Oct 27, 2023

Choose a reason for hiding this comment

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

Hm, helm diff doesn't display secrets' values, as long as --show-secrets option is missing

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Using data instead of stringData I can capture this in helm diff output.

geth, geth-2-s3-secret, Secret (v1) has changed:
  apiVersion: v1
  kind: Secret
  metadata:
    name: geth-2-s3-secret
  data:
-   AWS_ACCESS_KEY_ID: '-------- # (61 bytes)'
+   AWS_ACCESS_KEY_ID: '++++++++ # (62 bytes)'
    AWS_SECRET_ACCESS_KEY: 'REDACTED # (40 bytes)'
    S3_ENDPOINT_URL: 'REDACTED # (30 bytes)'

I just need to see whether it is changed or not. Don't need secret values to be present in output.

When using stringData helm diff does not provide info whether secret has changed or not.

Copy link
Contributor

@voron voron Oct 27, 2023

Choose a reason for hiding this comment

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

I cannot reproduce this using helm or helmfile and dysnix/raw chart with following values

templates:
  - |
    apiVersion: v1
    stringData:
      foo: newbar
    kind: Secret
    metadata:
      name: {{ .Release.Name }}
    type: Opaque

I change foo's value and see

-   foo: '-------- # (3 bytes)'
+   foo: '++++++++ # (6 bytes)'

Copy link
Contributor

Choose a reason for hiding this comment

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

Seems, related issue has been fixed ~ a year ago.

dysnix/geth/templates/statefulset.yaml Outdated Show resolved Hide resolved
dysnix/geth/values.yaml Outdated Show resolved Hide resolved
dysnix/geth/values.yaml Show resolved Hide resolved
dysnix/geth/values.yaml Outdated Show resolved Hide resolved
dysnix/geth/templates/scripts/_s3-env.tpl Show resolved Hide resolved
Copy link
Contributor

@voron voron left a comment

Choose a reason for hiding this comment

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

LGTM!

@VladStarr VladStarr merged commit 2609031 into main Oct 27, 2023
2 checks passed
@VladStarr VladStarr deleted the geth-add-s3-snapshot branch October 27, 2023 13:52
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.

2 participants