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 Daily Reboot Cronjob (K8s) #206

Merged
merged 22 commits into from
Jan 31, 2024

Conversation

NotaSF
Copy link
Contributor

@NotaSF NotaSF commented Jan 29, 2024

Requires the k8s deployment to be in the palworld namespace.

Currently, it is set to run at 9:30am UTC daily.

I'm newer to configuring such things, and will work to improve/cleanup a little bit. :)

Context

  • This launches a cronjob to run a shell script (defined in cm.yaml), which sends a broadcast notifying players that the server is saving. After 30 seconds, it does a formal backup using the established 'exec pod/podnamehere backup' command in the repo. After 30 seconds later, it sends a broadcast notifying players that the server will reboot in 60 seconds; 30 seconds; then each second for the last 10 seconds. It the runs the "Shutdown 1" command via rcon-cli, waits 30 additional seconds, then redeploys the application.

Choices

  • I prefer the K8s cluster schedule and perform a cronjob so I can keep track of it and also rollback if needed.
  • This made the most sense to me, as it does not intrude or alter the main image itself.
  • I wanted to learn more about how CronJobs work in Kubernetes.

Test instructions

  1. Copy the folder and its contents
  2. Edit the cronjob.yaml schedule to '* * * * *' so it schedules it immediately.
  3. Log into server
  4. Apply the yaml files, specifically applying cronjob.yaml last.
  5. Wait for the broadcasts, also monitor the logs of the job created using kubectl -n palworld logs job/jobnamehere
  6. Server should disconnect once timer reaches 0; redeployment should be under way.
  7. Server should come back up once redeployment is finished.

Checklist before requesting a review

  • I have performed a self-review of my code
  • I've added documentation about this change to the README.
  • I've not introduced breaking changes.

Requires the k8s deployment to be in the palworld namespace.

This launches a cronjob to run a shell script (defined in cm.yaml), which sends a broadcast notifying players that the server is saving. After 30 seconds, it does a formal backup using the established 'exec pod/podnamehere backup' command in the repo. After 30 seconds later, it sends a broadcast notifying players that the server will reboot in 60 seconds; 30 seconds; then each second for the last 10 seconds. It the runs the "Shutdown 1" command via rcon-cli, waits 30 additional seconds, then redeploys the application.

Currently, it is set to run at 9:30am UTC daily.

I'm newer to configuring such things, and will work to improve/cleanup a little bit. :)
Removed commented out section
@NotaSF NotaSF changed the title Add Daily Reboot Cronjob Add Daily Reboot Cronjob (K8s) Jan 29, 2024
@thijsvanloef thijsvanloef added help wanted Extra attention is needed kubernetes Issue/PR related to Kubernetes labels Jan 29, 2024
Copy link
Contributor

@Twinki14 Twinki14 left a comment

Choose a reason for hiding this comment

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

I would say,

  • Add this to the helm chart, and make it opt-in, it's far better suited for the helm chart
  • If we still want this in the repository (outside the helm chart), let's store it only as an example, perhaps in a directory k8s/examples/daily-restart, with an included readme to explain it and note any requirements/changes someone would need to make to use it

Really, since we have a helm chart now, we don't need to have the raw k8s manifests like we do now anyway. Helm is extremely simple to setup in a k8s environment, and is basically the defacto standard in any k8s environment

Copy link
Contributor

@MSpreckels MSpreckels left a comment

Choose a reason for hiding this comment

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

I've reviewed it but as @Twinki14 said, this would fit in the helm chart too. Can you add that?

If you add a kustomization.yaml you could easily patch every namespace as well as the SA reference in the role binding :)

k8s/cronjob-reboot/cm.yaml Outdated Show resolved Hide resolved
k8s/cronjob-reboot/cm.yaml Outdated Show resolved Hide resolved
k8s/cronjob-reboot/cronjob.yaml Outdated Show resolved Hide resolved
command:
- /bin/sh
- -c
- /restart-script/restart-deployment.sh
Copy link
Contributor

Choose a reason for hiding this comment

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

do you even need a CM with the shell script? what about inline?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That is a good point, I'm not sure actually. Pardon my ignorance, I'm a little new to this - when you say inline, do you mean taking the contents of the shell script and making it a one-liner instead of using a CM and mounting it as a volume?

Copy link
Contributor

Choose a reason for hiding this comment

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

Hi sorry for the confusion. What i mean is instead of having a configmap mounted you could also put that script in that command as an arg like

command:
  - /bin/sh
  - -c
args:
  - |
    cont=$(kubectl -n palworld get pods -o=jsonpath='{.items[?(@.metadata.labels.app=="palworld-server")].metadata.name}')
    kubectl exec -n palworld -i pod/$cont rcon-cli "Broadcast Saving_Server_Data..."
    kubectl exec -n palworld -i pod/$cont rcon-cli save
    sleep 30
    kubectl exec -n palworld -i pod/$cont rcon-cli "Broadcast Backing_up_Server_Data..."
    kubectl exec -n palworld -i pod/$cont backup
 ...

k8s/cronjob-reboot/role.yaml Outdated Show resolved Hide resolved
k8s/cronjob-reboot/rolebinding.yaml Outdated Show resolved Hide resolved
k8s/cronjob-reboot/serviceaccount.yaml Outdated Show resolved Hide resolved
Indentation; namespace adjustment
Remove namespace
Remove namespace
Remove namespace
Remove namespace
Add additional parameters for cronjob
@NotaSF
Copy link
Contributor Author

NotaSF commented Jan 29, 2024

I would say,

  • Add this to the helm chart, and make it opt-in, it's far better suited for the helm chart
  • If we still want this in the repository (outside the helm chart), let's store it only as an example, perhaps in a directory k8s/examples/daily-restart, with an included readme to explain it and note any requirements/changes someone would need to make to use it

Really, since we have a helm chart now, we don't need to have the raw k8s manifests like we do now anyway. Helm is extremely simple to setup in a k8s environment, and is basically the defacto standard in any k8s environment

I have made some changes and added helm chart values. Tested on my dev cluster... let me know if some additional changes would be needed.

@Twinki14
Copy link
Contributor

I have made some changes and added helm chart values. Tested on my dev cluster... let me know if some additional changes would be needed.

Nice 👍 My only final nit-pick is the config map script, I think that can definitely be cleaned up with some type of for-loop

Forgot escape characters.
@thijsvanloef
Copy link
Owner

@NotaSF Is this ready to merge?

@NotaSF
Copy link
Contributor Author

NotaSF commented Jan 30, 2024

Did some last minute cleanup, moving it to an examples folder instead.

It should be ready for merge, but I must admit I do not know how to update the chart metadata with the newly-added chart values other than manually adding them.

EDIT - Found it, my bad. VALUES_SUMMARY has been updated; entire PR is ready to merge. @thijsvanloef

Generated updated VALUES_SUMMARY with helm-docs
Fix markdown linting error
@NotaSF
Copy link
Contributor Author

NotaSF commented Jan 30, 2024

Found the source of the markdown lint failure - should be good now.

@thijsvanloef thijsvanloef merged commit 0e37862 into thijsvanloef:main Jan 31, 2024
4 checks passed
@MSpreckels MSpreckels mentioned this pull request Feb 2, 2024
3 tasks
MusclePr pushed a commit to MusclePr/palworld-server-docker that referenced this pull request Jun 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed kubernetes Issue/PR related to Kubernetes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants