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.

Sign up for GitHub

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

Jump to bottom

[bitnami/opensearch] Enable automated setup of snapshot policies #29796

Merged
merged 24 commits into from
Nov 18, 2024
Merged

[bitnami/opensearch] Enable automated setup of snapshot policies #29796

merged 24 commits into from
Nov 18, 2024

Conversation

lindhe
Copy link
Contributor

@lindhe lindhe commented Oct 7, 2024 •
edited
Loading

Description of the change

This change lets users opt-in to automatically initialize snapshot repositories and snapshot policies in OpenSearch using a post-install chart hook.

Benefits

Before this change, configuring snapshot policies was a fairly complex task that requires interacting with the OpenSearch API manually. This change automates that process and allows for easy configuration via the Helm chart's values. I believe this will enable more people to have an OpenSearch instance that is properly configured so consistent backups can be taken.

Possible drawbacks

Considering this is an opt-in feature, I don't see any significant draw-backs.

Applicable issues

Additional information

I've tried to split the PR into logically separate commits, so if we want the PVC creation to be a separate PR that's easy to do. Please inspect my commits for more info!

Checklist

  • Chart version bumped in Chart.yaml according to semver. This is not necessary when the changes only affect README.md files.
  • Variables are documented in the values.yaml and added to the README.md using readme-generator-for-helm
  • Title of the pull request follows this pattern [bitnami/<name_of_the_chart>] Descriptive title
  • All commits signed off and in agreement of Developer Certificate of Origin (DCO)

lindhe added 4 commits October 7, 2024 11:32
@lindhe
[bitnami/opensearch] Conditionally create PVC for snapshots
e935d4f 
I've borrowowed most of the config from the dashboards PVC.

Signed-off-by: Andreas Lindhé <7773090+lindhe@users.noreply.github.com>
@lindhe
[bitnami/opensearch] Create volumes for snapshots
0bade28 
Signed-off-by: Andreas Lindhé <7773090+lindhe@users.noreply.github.com>
@lindhe
[bitnami/opensearch] Create snapshots post-install hook
2e41cf6 
This change creates a new job to be run as an optional post-install
hook for creating initial snapshot policies in OpenSearch, along with
relvant ConfigMaps and NetworkPolicies.

I have tried to follow most of Bitnami's conventions for the templates,
but it's hard to tell if everything is 100% aligned with best practices.

Signed-off-by: Andreas Lindhé <7773090+lindhe@users.noreply.github.com>
@lindhe
[bitnami/opensearch] Bump chart version
c29ade4 
Signed-off-by: Andreas Lindhé <7773090+lindhe@users.noreply.github.com>
@github-actions github-actions bot added opensearch triage Triage is needed labels Oct 7, 2024
@github-actions github-actions bot requested a review from javsalgar October 7, 2024 09:47
lindhe
lindhe commented Oct 7, 2024
View reviewed changes
bitnami/opensearch/values.yaml Outdated Show resolved Hide resolved
lindhe
lindhe commented Oct 7, 2024
View reviewed changes
bitnami/opensearch/values.yaml Outdated Show resolved Hide resolved
@carrodher
Merge branch 'main' into lindhe/os-snapshot-init
a137a1f 
Signed-off-by: Carlos Rodríguez Hernández <carlosrh@vmware.com>
@carrodher carrodher added verify Execute verification workflow for these changes in-progress labels Oct 7, 2024
@github-actions github-actions bot removed the triage Triage is needed label Oct 7, 2024
@github-actions github-actions bot removed the request for review from javsalgar October 7, 2024 10:16
@github-actions github-actions bot requested a review from jotamartos October 7, 2024 10:16
@lindhe
Merge branch 'main' into lindhe/os-snapshot-init
1b744f4 
Signed-off-by: Andreas Lindhé <7773090+lindhe@users.noreply.github.com>
@jotamartos
Copy link
Contributor

Sorry for the delay in getting back to you. This is just a quick message to let you know that we are going to review it and will provide feedback as soon as possible.

@lindhe
Copy link
Contributor Author

lindhe commented Oct 21, 2024

Thanks, I really appreciate the ping to let me know you will get to it soon! 👍

jotamartos
jotamartos reviewed Oct 28, 2024
View reviewed changes
Copy link
Contributor

@jotamartos jotamartos left a comment

Choose a reason for hiding this comment

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

I just added some comments. Please take a look at them and let me know so we finish reviewing this PR. Thanks for your contribution

bitnami/opensearch/templates/_helpers.tpl Show resolved Hide resolved
bitnami/opensearch/values.yaml Outdated Show resolved Hide resolved
bitnami/opensearch/values.yaml Outdated Show resolved Hide resolved
bitnami/opensearch/values.yaml Outdated Show resolved Hide resolved
@lindhe @jotamartos
[bitnami/opensearch] Remove reundant comment in values
efedb9d 
Co-authored-by: Juan José Martos <jotamartos@gmail.com>
Signed-off-by: Andreas Lindhé <7773090+lindhe@users.noreply.github.com>
@jotamartos
Copy link
Contributor

jotamartos commented Oct 28, 2024 •
edited
Loading

Please take a look at the error while generating the README

error=Please ensure all *.registry params include the [default: REGISTRY_NAME] modifier in the chart bitnami/opensearch/values.yaml file

@lindhe
Copy link
Contributor Author

lindhe commented Oct 28, 2024

Please take a look at the error while generating the README

error=Please ensure all *.registry params include the [default: REGISTRY_NAME] modifier in the chart bitnami/opensearch/values.yaml file

I'm aware of the error. It relates to this discussion: #29796 (comment)

If we want to go for keeping the API body as YAML format, do you have any idea how we should go about configuring the @param argument for the documentation?

jotamartos
jotamartos reviewed Oct 29, 2024
View reviewed changes
bitnami/opensearch/values.yaml Outdated Show resolved Hide resolved
bitnami/opensearch/values.yaml Outdated Show resolved Hide resolved
lindhe and others added 2 commits October 29, 2024 13:05
@lindhe
[bitnami/opensearch] Remove another reundant comment
a65f57c 
Signed-off-by: Andreas Lindhé <7773090+lindhe@users.noreply.github.com>
@lindhe @jotamartos
Add defaults for documented image values
a99b834 
Co-authored-by: Juan José Martos <jotamartos@gmail.com>
Signed-off-by: Andreas Lindhé <7773090+lindhe@users.noreply.github.com>
@bitnami-bot
Update CHANGELOG.md
085e9ef 
Signed-off-by: Bitnami Containers <bitnami-bot@vmware.com>
@bitnami-bot
Update README.md with readme-generator-for-helm
c63321a 
Signed-off-by: Bitnami Containers <bitnami-bot@vmware.com>
@jotamartos
Copy link
Contributor

Just updated the thread above. Please ping me with a new comment when you update the PR. Thanks

@lindhe
Copy link
Contributor Author

lindhe commented Oct 31, 2024

Great. I'll just run one last test by installing it in my local cluster to try and verify nothing is obviously broken. I'll ping you once I'm done with that.

lindhe and others added 4 commits October 31, 2024 10:19
@lindhe
[bitnami/opensearch] Fix typo in nindent
125aef8 
Signed-off-by: Andreas Lindhé <7773090+lindhe@users.noreply.github.com>
@bitnami-bot
Update CHANGELOG.md
c364a0c 
Signed-off-by: Bitnami Containers <bitnami-bot@vmware.com>
@lindhe
[bitnami/opensearch] Add --show-error to curl
6e560e0 
This drastically improves debugging things when `curl` returns errors.

Signed-off-by: Andreas Lindhé <7773090+lindhe@users.noreply.github.com>
@lindhe
[bitnami/opensearch] Allow UDP/53 in NetworkPolicy
a19c8fb 
Signed-off-by: Andreas Lindhé <7773090+lindhe@users.noreply.github.com>
@lindhe
Copy link
Contributor Author

lindhe commented Oct 31, 2024

I found a typo in the Network Policy that I fixed. When debugging this, I also added a flag to curl to make debugging things a lot easier when curl crashes.

I've tested this now and it seems to work! Assuming you approve my recent changes, I think we are ready to merge! 🚀

@lindhe
Copy link
Contributor Author

lindhe commented Nov 7, 2024

Hey @jotamartos , if you have time to check on my latest changes that would be great. 🙂

lindhe and others added 2 commits November 7, 2024 15:20
@lindhe
Merge branch 'main' into lindhe/os-snapshot-init
d5054b7 
Signed-off-by: Andreas Lindhé <7773090+lindhe@users.noreply.github.com>
@bitnami-bot
Update CHANGELOG.md
0a50c9e 
Signed-off-by: Bitnami Containers <bitnami-bot@vmware.com>
@lindhe
Copy link
Contributor Author

lindhe commented Nov 7, 2024 •
edited
Loading

After merging main I'm getting a CI error I don't believe I got before:

image

Is this a new check? I have no idea where to find the images warning list or why it's wrong.

EDIT: Think I found it! Pushing an attempt to fix it.

lindhe added 2 commits November 7, 2024 15:36
@lindhe
Add snapshots.image to warnings list
b22febf 
Signed-off-by: Andreas Lindhé <7773090+lindhe@users.noreply.github.com>
@lindhe
Merge branch 'main' into lindhe/os-snapshot-init
5b1e08d 
Signed-off-by: Andreas Lindhé <7773090+lindhe@users.noreply.github.com>
@lindhe
Copy link
Contributor Author

lindhe commented Nov 13, 2024

@jotamartos I've resolved the conflicts now. If you could have a look at the latest changes and review before the next conflict emerge, that would be awesome! 👍

@bitnami-bot
Update CHANGELOG.md
3917b13 
Signed-off-by: Bitnami Containers <bitnami-bot@vmware.com>
jotamartos
jotamartos reviewed Nov 15, 2024
View reviewed changes
Copy link
Contributor

@jotamartos jotamartos left a comment

Choose a reason for hiding this comment

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

Two minor comments. Please have a look

bitnami/opensearch/values.yaml Show resolved Hide resolved
bitnami/opensearch/values.yaml Outdated Show resolved Hide resolved
lindhe and others added 4 commits November 15, 2024 16:16
@lindhe
Move snapshots.enabled to the top
1c80e37 
Signed-off-by: Andreas Lindhé <7773090+lindhe@users.noreply.github.com>
@lindhe
Move snapshots.persistence.enabled to the top
3c85e0c 
Signed-off-by: Andreas Lindhé <7773090+lindhe@users.noreply.github.com>
@bitnami-bot
Update CHANGELOG.md
7749789 
Signed-off-by: Bitnami Containers <bitnami-bot@vmware.com>
@bitnami-bot
Update README.md with readme-generator-for-helm
d519da7 
Signed-off-by: Bitnami Containers <bitnami-bot@vmware.com>
jotamartos
jotamartos approved these changes Nov 18, 2024
View reviewed changes
Copy link
Contributor

@jotamartos jotamartos left a comment

Choose a reason for hiding this comment

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

Changes LGTM! Thank you for your contribution

@jotamartos jotamartos merged commit 39322b9 into bitnami:main Nov 18, 2024
12 checks passed
@lindhe lindhe deleted the lindhe/os-snapshot-init branch November 18, 2024 14:01
@lindhe
Copy link
Contributor Author

lindhe commented Nov 18, 2024

Oh happy day! 🎉 Thank you for the collaboration!

sajad-sadra pushed a commit to sajad-sadra/bitnami-charts that referenced this pull request Nov 20, 2024
@lindhe @carrodher
@jotamartos @bitnami-bot @sajad-sadra
[bitnami/opensearch] Enable automated setup of snapshot policies (bit…
9603dac 
…nami#29796)

* [bitnami/opensearch] Conditionally create PVC for snapshots

I've borrowowed most of the config from the dashboards PVC.

Signed-off-by: Andreas Lindhé <7773090+lindhe@users.noreply.github.com>

* [bitnami/opensearch] Create volumes for snapshots

Signed-off-by: Andreas Lindhé <7773090+lindhe@users.noreply.github.com>

* [bitnami/opensearch] Create snapshots post-install hook

This change creates a new job to be run as an optional post-install
hook for creating initial snapshot policies in OpenSearch, along with
relvant ConfigMaps and NetworkPolicies.

I have tried to follow most of Bitnami's conventions for the templates,
but it's hard to tell if everything is 100% aligned with best practices.

Signed-off-by: Andreas Lindhé <7773090+lindhe@users.noreply.github.com>

* [bitnami/opensearch] Bump chart version

Signed-off-by: Andreas Lindhé <7773090+lindhe@users.noreply.github.com>

* [bitnami/opensearch] Remove reundant comment in values

Co-authored-by: Juan José Martos <jotamartos@gmail.com>
Signed-off-by: Andreas Lindhé <7773090+lindhe@users.noreply.github.com>

* [bitnami/opensearch] Remove another reundant comment

Signed-off-by: Andreas Lindhé <7773090+lindhe@users.noreply.github.com>

* Add defaults for documented image values

Co-authored-by: Juan José Martos <jotamartos@gmail.com>
Signed-off-by: Andreas Lindhé <7773090+lindhe@users.noreply.github.com>

* Update CHANGELOG.md

Signed-off-by: Bitnami Containers <bitnami-bot@vmware.com>

* Update README.md with readme-generator-for-helm

Signed-off-by: Bitnami Containers <bitnami-bot@vmware.com>

* [bitnami/opensearch] Fix typo in nindent

Signed-off-by: Andreas Lindhé <7773090+lindhe@users.noreply.github.com>

* Update CHANGELOG.md

Signed-off-by: Bitnami Containers <bitnami-bot@vmware.com>

* [bitnami/opensearch] Add `--show-error` to curl

This drastically improves debugging things when `curl` returns errors.

Signed-off-by: Andreas Lindhé <7773090+lindhe@users.noreply.github.com>

* [bitnami/opensearch] Allow UDP/53 in NetworkPolicy

Signed-off-by: Andreas Lindhé <7773090+lindhe@users.noreply.github.com>

* Update CHANGELOG.md

Signed-off-by: Bitnami Containers <bitnami-bot@vmware.com>

* Add snapshots.image to warnings list

Signed-off-by: Andreas Lindhé <7773090+lindhe@users.noreply.github.com>

* Update CHANGELOG.md

Signed-off-by: Bitnami Containers <bitnami-bot@vmware.com>

* Move `snapshots.enabled` to the top

Signed-off-by: Andreas Lindhé <7773090+lindhe@users.noreply.github.com>

* Move `snapshots.persistence.enabled` to the top

Signed-off-by: Andreas Lindhé <7773090+lindhe@users.noreply.github.com>

* Update CHANGELOG.md

Signed-off-by: Bitnami Containers <bitnami-bot@vmware.com>

* Update README.md with readme-generator-for-helm

Signed-off-by: Bitnami Containers <bitnami-bot@vmware.com>

---------

Signed-off-by: Andreas Lindhé <7773090+lindhe@users.noreply.github.com>
Signed-off-by: Carlos Rodríguez Hernández <carlosrh@vmware.com>
Signed-off-by: Bitnami Containers <bitnami-bot@vmware.com>
Co-authored-by: Carlos Rodríguez Hernández <carlosrh@vmware.com>
Co-authored-by: Juan José Martos <jotamartos@gmail.com>
Co-authored-by: Bitnami Containers <bitnami-bot@vmware.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jotamartos jotamartos jotamartos approved these changes

Assignees

@jotamartos jotamartos

Labels
opensearch solved verify Execute verification workflow for these changes
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[bitnami/opensearch] Enable automated snapshots for OpenSearch
5 participants
@lindhe @jotamartos @javsalgar @carrodher @bitnami-bot