Add signal-reboot

[ckotzbauer]

Based on #813
close #416
close #722

This PR adds a --reboot-method flag with "command" (default) and "signal" option. The "command" option uses the --reboot-command on the host with nsenter as before. The new "signal" mode uses a SIGRTMIN+5 signal by default against PID 1 (systemd) to reboot the node. The signal can be changed via --reboot-signal flag.

With this, the kured pod runs without privileged mode.

This PR is published as docker-image (amd64 and arm64): ghcr.io/ckotzbauer/kured:1.14.0-alpha.2
Usage (based on the latest helm-chart) - Helm values.yaml:

image:
  repository: ghcr.io/ckotzbauer/kured
  tag: 1.14.0-alpha.2
updateStrategy: RollingUpdate
configuration:
  period: "0h0m30s"
  rebootDelay: 0h1m0s
  rebootSentinel: /sentinel/reboot-required
extraArgs:
  reboot-method: signal
containerSecurityContext:
  readOnlyRootFilesystem: true
  privileged: false
  allowPrivilegeEscalation: false
  capabilities:
    drop: ["*"]
    add: ["CAP_KILL"]
volumes:
  - name: sentinel
    hostPath:
      path: /var/run
      type: Directory
volumeMounts:
  - name: sentinel
    mountPath: /sentinel
    readOnly: true

[rptaylor]

In my test this works, using the same values.yml file:

$ helm -n kured install -f ~/kured/values.yml   kured kubereboot/kured
$ helm -n kured list
NAME 	NAMESPACE	REVISION	UPDATED                                	STATUS  	CHART      	APP VERSION
kured	kured    	1       	2023-08-08 21:50:33.438477295 +0000 UTC	deployed	kured-5.1.0	1.13.2     
$ kubectl -n kured exec -it kured-vs4xb -- /bin/sh
/ # kill -s SIGRTMIN+5   1

Then journald shows:

Aug 08 14:52:08 kermes-dev-k8s-node-a03 systemd[1]: Received SIGRTMIN+6 from PID 22396 (sh).

and the system immediately does a clean reboot. However note that "SIGRTMIN+5" was sent but it was interpreted by host as SIGRTMIN+6. I believe that is related to the different output of kill -l in the pod vs on the host, which is a potential compatibility issue that we want to avoid by not using the pod's kill executable in the first place; the go library will send the signal instead. When I instead did kill -s 39 1 it worked as expected. I'm not sure how universal the interpretation 39=SIGRTMIN+5 is or whether it could be a good idea for the precise signal to be configurable so that users can ensure the signal is definitively the right one for their particular OS.

Anyway this was running on a v1.23 k8s cluster with the old PSP admission controller disabled. The nodes are Almalinux 8.8. I'm afraid I could not reproduce the issue; it works fine for me. Looking at the capabilities of the kured process it also appears fine:

/ # ps -ef | grep kured | grep -v grep
 6107 root      0:00 /usr/bin/kured --ds-name=kured --ds-namespace=kured --metrics-port=8080 --period=0h0m30s --reboot-sentinel=/sentinel/reboot-required --reboot-command=/bin/systemctl reboot --reboot-delay=0h1m0s --log-format=text --reboot-method=signal
/ # grep ^Cap /proc/6107/status
CapInh:	0000000000000000
CapPrm:	00000000a80425fb
CapEff:	00000000a80425fb
CapBnd:	00000000a80425fb
CapAmb:	0000000000000000

Also when I touched the sentinel file on the host, kured worked as expected and the node rebooted:

time="2023-08-08T22:27:19Z" level=info msg="Reboot required"
.....   #evicting pods etc
time="2023-08-08T22:27:58Z" level=info msg="Delaying reboot for 1m0s"
time="2023-08-08T22:28:58Z" level=info msg="Emit reboot-signal for node: cluster-dev-k8s-node-a03"
time="2023-08-08T22:28:58Z" level=info msg="Waiting for reboot"

I confirmed with kubectl get pod -o yaml that the kured pods have

  hostPID: true


    securityContext:
      allowPrivilegeEscalation: false
      capabilities:
        add:
        - CAP_KILL
        drop:
        - '*'
      privileged: false
      readOnlyRootFilesystem: true
   

[ckotzbauer]

Thanks for your tests and the positive feedback!

Yes, the specific signal value may indeed be a problem. We would need to add a good guidance for the user in our docs, to determine which signal value is correct for their host OS. Most of the users won't know as this is expert knowlege 😆

[ckotzbauer]

I made it work on my cluster with priviliged: false, by disabling AppArmor for the container with the Pod-Annotation

container.apparmor.security.beta.kubernetes.io/kured: unconfined

[rptaylor]

Ah of course, I'm not familiar with AppArmor at all but that is the SELinux equivalent on some OSes.

[github-actions]

This PR was automatically considered stale due to lack of activity. Please refresh it and/or join our slack channels to highlight it, before it automatically closes (in 7 days).

[rptaylor]

Is anyone available to review this?

[jackfrancis]

@rptaylor thanks for your patience, added some review comments for @ckotzbauer, I think this generally looks like a great evolution, thanks this work!

[ckotzbauer]

Sorry for the very long delay, this is ready for another review @jackfrancis @rptaylor

[rptaylor]

@ckotzbauer very exciting! I can test it on a dev cluster, is there a new image available in ghcr.io/ckotzbauer/ ?

[ckotzbauer]

Thanks for the hint: ghcr.io/ckotzbauer/kured:1.15.0-alpha.0 (amd64 and arm64) @rptaylor

[rptaylor]

Okay refreshing my memory on what this is even about 4 months later .... ;p

On a dev cluster I temporarily disabled PSP admission control for testing purposes.

$ cat values.yml 
image:
  repository: ghcr.io/ckotzbauer/kured
  tag: 1.15.0-alpha.0
configuration:
  period: "0h0m30s"
  rebootDelay: 0h1m0s
  rebootSentinel: /sentinel/reboot-required
# update faster
maxUnavailable: 10
extraArgs:
  reboot-method: signal
containerSecurityContext:
  readOnlyRootFilesystem: true
  privileged: false
  allowPrivilegeEscalation: false
  capabilities:
    drop: ["*"]
    add: ["CAP_KILL"]
volumes:
  - name: sentinel
    hostPath:
      path: /var/run
      type: Directory
volumeMounts:
  - name: sentinel
    mountPath: /sentinel
    readOnly: true

$ helm -n kured install -f values.yml   kured kubereboot/kured
$ helm -n kured list
NAME 	NAMESPACE	REVISION	UPDATED                                	STATUS  	CHART      	APP VERSION
kured	kured    	1       	2023-12-15 22:20:49.185347412 +0000 UTC	deployed	kured-5.2.0	1.14.0     

Then on a node I touched /var/run/reboot-required.

pod logs:

$ kubectl -n kured logs -f kured-79vrp
time="2023-12-15T22:37:53Z" level=info msg="Binding node-id command flag to environment variable: KURED_NODE_ID"
time="2023-12-15T22:37:53Z" level=info msg="Kubernetes Reboot Daemon: 544a287"
time="2023-12-15T22:37:53Z" level=info msg="Node ID: cluster-dev-k8s-node-a02"
time="2023-12-15T22:37:53Z" level=info msg="Lock Annotation: kured/kured:weave.works/kured-node-lock"
time="2023-12-15T22:37:53Z" level=info msg="Lock TTL not set, lock will remain until being released"
time="2023-12-15T22:37:53Z" level=info msg="Lock release delay not set, lock will be released immediately after rebooting"
time="2023-12-15T22:37:53Z" level=info msg="PreferNoSchedule taint: "
time="2023-12-15T22:37:53Z" level=info msg="Blocking Pod Selectors: []"
time="2023-12-15T22:37:53Z" level=info msg="Reboot schedule: SunMonTueWedThuFriSat between 00:00 and 23:59 UTC"
time="2023-12-15T22:37:53Z" level=info msg="Reboot check command: [test -f /sentinel/reboot-required] every 30s"
time="2023-12-15T22:37:53Z" level=info msg="Concurrency: 1"
time="2023-12-15T22:37:53Z" level=info msg="Reboot method: signal"
time="2023-12-15T22:37:53Z" level=info msg="Reboot signal: 39"
time="2023-12-15T22:39:41Z" level=info msg="Reboot required"
time="2023-12-15T22:39:41Z" level=info msg="Acquired reboot lock"
time="2023-12-15T22:39:41Z" level=info msg="Draining node cluster-dev-k8s-node-a02"
evicting pod  ....
time="2023-12-15T22:40:35Z" level=info msg="Delaying reboot for 1m0s"
time="2023-12-15T22:41:35Z" level=info msg="Emit reboot-signal for node: cluster-dev-k8s-node-a02"
time="2023-12-15T22:41:35Z" level=info msg="Waiting for reboot"

Eventually .... node rebooted:

Dec 15 14:41:35 cluster-dev-k8s-node-a02 systemd[1]: Received SIGRTMIN+5 from PID 2802298 (kured).

Does that cover everything needed? Is it good to merge now?

[rptaylor]

Actually wait ... the node powered off instead of rebooting, that's not good...

[rptaylor]

I tried again and the node rebooted normally. Sometimes our cloud infrastructure is a bit flakey... Anyway as long as systemd on the node reports "Received SIGRTMIN+5" it should be good.

[rptaylor]

Is this meant to be sigRtminPlus5 ?

[ckotzbauer]

Yes, I fix the typo.

[rptaylor]

Looks like the typo is still there? But otherwise this should be finally good to go!

[jackfrancis]

Sorry, I didn't catch that. Let's fix the typo. If we're going to use camel case I'd vote for:

• sigRTMin

Maybe:

const (
...
    sigRTMin = 34
    defaultSignal = sigRTMin + 5
...
)

And we should probably change the MethodSignal comment so that it doesn't imply that SIGRTMIN+5 is static (it's just a default, but the user can use whatever number within the min-max range that their OS allows). Maybe:

// MethodSignal is used as "--reboot-method" value when rebooting according to a signal in the allowed range between SIGRTMIN and SIGRTMAX as defined on the OS platform running kured

[ckotzbauer]

Thanks for testing @rptaylor, this should cover all needed changes, so this should be ready for merge.

[jackfrancis]

lgtm

great work, thank you!