Skip to content

Update mc-router chart for new auto scale options #270

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

Merged
merged 3 commits into from
May 9, 2025
Merged

Update mc-router chart for new auto scale options #270

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
0fc6a76
Update mc-router chart for new auto scale options
Sammcb May 3, 2025
0d4c823
Fixed behavior
Sammcb May 4, 2025
b52280d
Allow using either secret or configmap
Sammcb May 8, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion charts/mc-router/Chart.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
apiVersion: v1
name: mc-router
version: 1.3.0
version: 1.4.0
# not used
appVersion: 1.0.0
home: https://github.com/itzg/mc-router
Expand Down
11 changes: 11 additions & 0 deletions charts/mc-router/templates/autoscale-allow-deny-configmap.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
{{- if eq .Values.minecraftRouter.autoScale.configObject "ConfigMap" | and .Values.minecraftRouter.autoScale.allowDeny }}
apiVersion: v1
kind: ConfigMap
metadata:
name: {{ include "mc-router.fullname" . }}-autoscale-allow-deny
namespace: {{ .Release.Namespace }}
labels:
{{- include "mc-router.labels" . | nindent 4 }}
data:
auto-scale-allow-deny-list.json: {{ .Values.minecraftRouter.autoScale.allowDeny | toJson | squote }}
{{- end }}
11 changes: 11 additions & 0 deletions charts/mc-router/templates/autoscale-allow-deny-secret.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
{{- if eq .Values.minecraftRouter.autoScale.configObject "Secret" | and .Values.minecraftRouter.autoScale.allowDeny }}
apiVersion: v1
kind: Secret
metadata:
name: {{ include "mc-router.fullname" . }}-autoscale-allow-deny
namespace: {{ .Release.Namespace }}
labels:
{{- include "mc-router.labels" . | nindent 4 }}
data:
auto-scale-allow-deny-list.json: {{ .Values.minecraftRouter.autoScale.allowDeny | toJson | b64enc }}
{{- end }}
2 changes: 1 addition & 1 deletion charts/mc-router/templates/clusterrole.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ rules:
- apiGroups: [""]
resources: ["services"]
verbs: ["watch","list"]
{{- if .Values.minecraftRouter.autoScaleUp.enabled }}
{{- if .Values.minecraftRouter.autoScale.up.enabled }}
Copy link
Owner

Choose a reason for hiding this comment

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

Unfortunately this would be a breaking change for existing chart users. You could do this as an or-expression to allow for both old and new value identifiers.

- apiGroups: ["apps"]
resources: ["statefulsets", "statefulsets/scale"]
verbs: ["watch","list","get","update"]
Expand Down
42 changes: 38 additions & 4 deletions charts/mc-router/templates/deployment.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,18 @@ spec:
{{- .Values.deploymentStrategy | toYaml | nindent 4}}
template:
metadata:
{{- with .Values.podAnnotations }}
{{- if or .Values.minecraftRouter.autoScale.allowDeny .Values.podAnnotations }}
annotations:
{{- if .Values.minecraftRouter.autoScale.allowDeny }}
{{- if eq .Values.minecraftRouter.autoScale.configObject "Secret" }}
checksum/autoscale-allow-deny-config: {{ include (print .Template.BasePath "/autoscale-allow-deny-secret.yaml") . | sha256sum }}
{{- else if eq .Values.minecraftRouter.autoScale.configObject "ConfigMap" }}
checksum/autoscale-allow-deny-config: {{ include (print .Template.BasePath "/autoscale-allow-deny-configmap.yaml") . | sha256sum }}
{{- end }}
{{- end }}
{{- with .Values.podAnnotations }}
{{- toYaml . | nindent 8 }}
{{- end }}
{{- end }}
labels:
{{- include "mc-router.labels" . | nindent 8 }}
Expand All @@ -54,7 +63,12 @@ spec:
{{- include "mc-router.envMap" (list "PORT" $minecraftPort) }}

{{- with .Values.minecraftRouter }}
{{- include "mc-router.envMap" (list "AUTO_SCALE_UP" .autoScaleUp.enabled) }}
{{- include "mc-router.envMap" (list "AUTO_SCALE_UP" .autoScale.up.enabled) }}
Copy link
Owner

Choose a reason for hiding this comment

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

Same backward compatibility comment

{{- include "mc-router.envMap" (list "AUTO_SCALE_DOWN" .autoScale.down.enabled) }}
{{- include "mc-router.envMap" (list "AUTO_SCALE_DOWN_AFTER" .autoScale.down.after) }}
{{- if .autoScale.allowDeny }}
{{- include "mc-router.envMap" (list "AUTO_SCALE_ALLOW_DENY" "etc/mc-router/auto-scale-allow-deny-list.json") }}
{{- end }}
{{- include "mc-router.envMap" (list "CONNECTION_RATE_LIMIT" .connectionRateLimit) }}
{{- include "mc-router.envMap" (list "CPU_PROFILE" .cpuProfilePath) }}
{{- include "mc-router.envMap" (list "DEBUG" .debug.enabled) }}
Expand Down Expand Up @@ -136,13 +150,20 @@ spec:
port: {{ $apiPort }}
resources:
{{- toYaml .Values.resources | nindent 12 }}
{{- with .Values.extraVolumes }}
{{- if or .Values.minecraftRouter.autoScale.allowDeny .Values.extraVolumes }}
volumeMounts:
{{- if .Values.minecraftRouter.autoScale.allowDeny }}
- name: autoscale-allow-deny
mountPath: /etc/mc-router
readOnly: true
{{- end }}
{{- with .Values.extraVolumes }}
{{- range . }}
{{- if .volumeMounts }}
{{- toYaml .volumeMounts | nindent 12 }}
{{- end }}
{{- end }}
{{- end }}
{{- end }}
{{- with .Values.nodeSelector }}
nodeSelector:
Expand All @@ -157,13 +178,26 @@ spec:
{{- toYaml . | nindent 8 }}
{{- end }}

{{- with .Values.extraVolumes }}
{{- if or .Values.minecraftRouter.autoScale.allowDeny .Values.extraVolumes }}
volumes:
{{- if .Values.minecraftRouter.autoScale.allowDeny }}
{{- if eq .Values.minecraftRouter.autoScale.configObject "Secret" }}
- name: autoscale-allow-deny
secret:
secretName: {{ include "mc-router.fullname" . }}-autoscale-allow-deny
{{- else if eq .Values.minecraftRouter.autoScale.configObject "ConfigMap" }}
- name: autoscale-allow-deny
configMap:
name: {{ include "mc-router.fullname" . }}-autoscale-allow-deny
{{- end }}
{{- end }}
{{- with .Values.extraVolumes }}
{{- range . }}
{{- if .volumes }}
{{- toYaml .volumes | nindent 8 }}
{{- end }}
{{- end }}
{{- end }}
{{- end }}
{{- range $key, $value := .Values.extraPodSpec }}
{{ $key }}: {{ tpl $value $ }}
Expand Down
52 changes: 43 additions & 9 deletions charts/mc-router/values.schema.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
{
"$schema": "http://json-schema.org/schema",
"$schema": "https://json-schema.org/draft/2020-12/schema",
"required": [
"minecraftRouter",
"services"
Expand Down Expand Up @@ -59,17 +59,51 @@

"minecraftRouter": {
"type": "object",
"additionalProperties": false,
"properties": {
"autoScaleUp": {
Copy link
Owner

Choose a reason for hiding this comment

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

Regarding my backward compatibility commments, I'm not sure if the old value/object needs to be also included here. I'd have to look if JSON schema has a deprecated flag.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

(This applies to all other backwards compatible comments). I'm happy to make changes to allow for backwards compatibility, but wondering how you would feel about simply bumping the chart major version and leaving this as a breaking change? I completely understand if you would prefer to make it backwards compatible, just wanted to check first 😄

Copy link
Owner

Choose a reason for hiding this comment

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

Good point, that is supposed to be point of major versions 😀 Even a minor version bump is probably somewhat legit for just an interface change.

How about this:

  • keep it as a minor version bump
  • don't worry about backward compatible handling
  • could place somewhere a condition and fail if the old value is set. (I'm not sure the best convention of where to declare such template guards)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think that sounds good, I'll take a look at the JSON Schema because I think the additionalProperties key being false should guard against that.

"autoScale": {
"type": "object",
"title": "\"Wake up\" any stopped Minecraft servers.",
"description": "This requires Minecraft servers to be kind: StatefulSet",
"properties": {
"enabled": {
"anyOf": [
{ "type": "string", "enum": ["default"] },
{ "type": "boolean" }
]
"up": {
"type": "object",
"properties": {
"enabled": {
"title": "\"Wake up\" any stopped Minecraft servers.",
"description": "This requires Minecraft servers to be kind: StatefulSet",
"anyOf": [
{ "type": "string", "enum": ["default"] },
{ "type": "boolean" }
]
}
}
},
"down": {
"type": "object",
"properties": {
"enabled": {
"title": "Shut down any running Minecraft servers after there are no more connections.",
"description": "This requires Minecraft servers to be kind: StatefulSet",
"anyOf": [
{ "type": "string", "enum": ["default"] },
{ "type": "boolean" }
]
},
"after": {
"type": "string",
"title": "Shut down waiting period after there are no more connections.",
"description": "It is recommended that this value is high enough so momentary disconnections do not result in a server shutdown"
}
}
},
"configObject": {
"type": "string",
"enum": ["Secret", "ConfigMap"],
"title": "Type of Kubernetes object to store autoscale allow/deny list config in."
},
"allowDeny": {
"title": "Specify a server allow/deny list to restrict which players may trigger the scalers.",
"$comment": "Update tag below and in values.yaml comment when this file changes",
"$ref": "https://raw.githubusercontent.com/itzg/mc-router/refs/tags/1.29.0/docs/allow-deny-list.schema.json"
}
}
},
Expand Down
36 changes: 31 additions & 5 deletions charts/mc-router/values.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -143,10 +143,36 @@ extraDeploy: []
# }

minecraftRouter:
# "Wake up" any stopped Minecraft servers.
# This requires Minecraft servers to be kind: StatefulSet
autoScaleUp:
enabled: false
autoScale:
up:
# "Wake up" any stopped Minecraft servers.
# This requires Minecraft servers to be kind: StatefulSet
enabled: false
down:
# Shut down any running Minecraft servers after there are no more connections.
# This requires Minecraft servers to be kind: StatefulSet
enabled: false
# Shut down waiting period after there are no more connections.
# It is recommended that this value is high enough so momentary disconnections do not result in a server shutdown
after: ""
# Type of Kubernetes object to store autoscale allow/deny list config in.
# Valid options: Secret,ConfigMap
configObject: Secret
# Specify a server allow/deny list to restrict which players may trigger the scalers.
# For more info on the schema, check out the file in `mc-router`
# https://github.com/itzg/mc-router/blob/1.29.0/docs/allow-deny-list.schema.json
allowDeny: {}
# global:
# denylist:
# - uuid: some-player-uuid
# name: somePlayerName
# servers:
# my.server.domain:
# allowlist:
# - uuid: some-player-uuid
# my.other-server.domain:
# denylist:
# - uuid: some-player-uuid

# Max number of connections to allow per second
connectionRateLimit: 1
Expand All @@ -170,7 +196,7 @@ minecraftRouter:
# port: 25565

metrics:
# Backend to use for metrics exposure/publishing: discard,expvar,influxdb
# Backend to use for metrics exposure/publishing: discard,expvar,influxdb,prometheus
backend: discard

# InfluxDB settings. Required if backend is set to influxdb
Expand Down