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

Add mc-backup Sidecar #93

Merged
merged 30 commits into from Jan 12, 2022
Merged

Add mc-backup Sidecar #93

merged 30 commits into from Jan 12, 2022

Conversation

merkinbob
Copy link
Contributor

@merkinbob merkinbob commented Jan 9, 2022

Add mc-backup sidecar for minecraft-server.

Ready for testing:

  • tar
  • rclone
  • restic

Please be careful with Restic, take note of the HOSTNAME conflict

Tested:

  • tar method
  • rclone method
  • restic method

image

I believe restic and rclone are setup right, but i have no way of testing these

Solves #91

@merkinbob merkinbob marked this pull request as ready for review January 9, 2022 22:42
@merkinbob
Copy link
Contributor Author

is there any interest in rclone or restic for the chart?

@itzg
Copy link
Owner

itzg commented Jan 10, 2022

is there any interest in rclone or restic for the chart?

People use those methods, but can be included in a follow up PR.

@itzg
Copy link
Owner

itzg commented Jan 10, 2022

Please take a look at the build failure.

@merkinbob
Copy link
Contributor Author

is there any interest in rclone or restic for the chart?

People use those methods, but can be included in a follow up PR.

in theory restic is done, but i don't know how to test it. rclone i need to add a config map for.

@merkinbob
Copy link
Contributor Author

Please take a look at the build failure.

will do

@merkinbob
Copy link
Contributor Author

@itzg I think this now adds restic, and rclone capabilities, i didn't test it tho. tar still works, linting should pass.

@merkinbob
Copy link
Contributor Author

merkinbob commented Jan 11, 2022

3rd time the charm?

@cburza
Copy link

cburza commented Jan 11, 2022

@merkinbob I will attempt to test rclone.

@cburza
Copy link

cburza commented Jan 11, 2022

@merkinbob I am getting the following error on container startup:

time="2022-01-11T18:06:19Z" level=fatal msg="Failed to run sub-command" error="fork/exec /usr/bin/backup: operation not permitted"

I have RCON enabled in the server. I am setting no other values for mcbackup other than enabled: true.

Do you perhaps know what could be causing it?

@merkinbob
Copy link
Contributor Author

merkinbob commented Jan 11, 2022

@merkinbob I am getting the following error on container startup:

time="2022-01-11T18:06:19Z" level=fatal msg="Failed to run sub-command" error="fork/exec /usr/bin/backup: operation not permitted"

I have RCON enabled in the server. I am setting no other values for mcbackup other than enabled: true.

Do you perhaps know what could be causing it?

my guess is it's not happy with empty dir for the backup volume.

Below is all i have set in my values.yaml file for my argocd deployment.

mcbackup:
  enabled: true     

  persistence:
    storageClass: "csi-cephfs-sc"
    backupDir:
      enabled: true
      Size: 500Gi

As soon as I set backupDir: enabled: false I got the same crashloop

I think the issue is in here somewhere

https://github.com/itzg/entrypoint-demoter

I could try to set the UID and GID ENV to match the security context.

After some research, it appears the emptydir is owned by root:root. This is an old issue in k8s, and not resolved.

kubernetes/kubernetes#2630

Copy link
Owner

@itzg itzg left a comment

Choose a reason for hiding this comment

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

Final review looks great! Ready for a merge?

@merkinbob
Copy link
Contributor Author

Final review looks great! Ready for a merge?

one concern I have about restic is the HOSTNAME note on your mc-backup docs. Other than that, I think this is a good start. I hope someone finds this useful.

@itzg
Copy link
Owner

itzg commented Jan 12, 2022

Just discovered there's a --hostname. I wrote up itzg/docker-mc-backup#55 , but I think the chart changes will be good even with that.

@itzg itzg merged commit ce8e857 into itzg:master Jan 12, 2022
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.

None yet

3 participants