-
-
Notifications
You must be signed in to change notification settings - Fork 141
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
add support for etcd backups with s3 storage #471
base: main
Are you sure you want to change the base?
Conversation
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.
Wow, you made me VERY happy with this PR! It was on my list, so I appreciate you took the time to implement it.
I requested some changes if you don't mind, but these are mostly minor things and quick changes as you have done a great job really.
Thanks a lot!
also make clear the s3 options are only available for the etcd option
I realize it was outside the scope of this PR, but after the request to refactor the structor of the cluster_config.yaml options I figured I would add some of the additional back up options. The only backup option I could not figure out was schedule_cron. I could not get it to work correctly for the life of me. Maybe it's the way the string was getting parsed with all of the quotes. That can be saved for a future PR. Note to self, I was looking at creating a /etc/rancher/k3s/config.yaml file with the snapshot-cron value, instead of passing it as an arg to the k3s binary. |
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.
A few more comments, it's looking good!!! 👍
Can you share more details on the issue you had with that setting? It should be a regular string if it's in cron format, isn't that the case? |
I was only able to get it to work using a config file config files when changed do not automatically restart the k3s service, it has to be done manually
I tried every variation I could think of for a cli flag. None of them worked. I even tried manually setting it in master_install_script.sh in case it was the YAML parser being weird. If you are able to get it to work that would be awesome as it would be cleaner. Based on this comment k3s-io/k3s#5983 (comment), I was able to get it to work using a config file. So if Whenever a cli flag changes the k3s service will automatically restart when rerunning the master_install_script.sh. However, if a config file changes, the service will not restart. I created an issue in the k3s repo k3s-io/k3s#11180. But in the meantime we would have to manually restart the k3s service, just in case there are changes in a config file, even if there are none. |
since the etcd class isn’t optional in the Datastore Config is will always be set technically, which is fine if they are using an external datastore since the options won’t matter
I see, good idea to open an issue there. Did you also try normal escaping with |
Quality Gate passedIssues Measures |
Yes, I tried escaping with \" and also \\\" just in case |
I see. Thanks for the clarification |
K3S has built in support for uploading snapshots of the embedded etcd database to S3. This PR adds the settings to enable that to the configuration file.
If the s3 settings are not included it will default to not being enabled, to ensure backward compatibility.
I also added a check to make sure it is only enabled when using the etcd datastore mode.
It may also work with other S3 compatible providers. I cannot give a list, however I tested it with Backblaze B2 and it worked.
An example when uploading to AWS S3 in the us-east-1 region: