diff --git a/OPERATOR.md b/OPERATOR.md index 912a1bb..46b5626 100644 --- a/OPERATOR.md +++ b/OPERATOR.md @@ -48,6 +48,21 @@ If you set those variables you can use - `task docker-hub-authorize` to upload to the current kubernetes runtime a secret to give the possibility to download the operator enterprise image +## Redis graceful shutdown + +The Redis command uses `exec` so the server is process 1 and receives the +container stop signal. With RDB save points configured, this permits Redis to +complete its final snapshot during an ordinary pod replacement. The termination +grace period must still be sufficient for the dataset being saved. + +Existing installations must apply the updated Redis StatefulSet command through +their component deployment procedure; updating only the operator image does not +rewrite an existing StatefulSet. During the first rollout from the old shell +command, arrange an authenticated `SHUTDOWN SAVE` for the old Redis process before +it can be forcibly terminated. Subsequent ordinary replacements use the corrected +signal path. This does not change RDB/AOF policy or preserve unsaved RDB writes +after a hard crash or `SIGKILL`. ACL identity persistence is a separate concern. + ## Customization notes OpenServerless operator is normally deployed using a `whisk.yaml` configuration file which is applied via the installer. diff --git a/deploy/redis/redis-set.yaml b/deploy/redis/redis-set.yaml index a28d9a3..48d2bd8 100644 --- a/deploy/redis/redis-set.yaml +++ b/deploy/redis/redis-set.yaml @@ -38,7 +38,7 @@ spec: containers: - name: redis image: docker.io/bitnamilegacy/redis:8.2.1 - command: ["/bin/sh","-c","redis-server /redis-master/redis.conf"] + command: ["/bin/sh","-c","exec redis-server /redis-master/redis.conf"] env: - name: MASTER value: "true" diff --git a/openserverless/templates/redis-set.yaml b/openserverless/templates/redis-set.yaml index 8bee1d8..0d9a3d4 100644 --- a/openserverless/templates/redis-set.yaml +++ b/openserverless/templates/redis-set.yaml @@ -43,7 +43,7 @@ spec: containers: - name: redis image: docker.io/bitnamilegacy/redis:8.2.1 - command: ["/bin/sh","-c","redis-server /redis-master/redis.conf"] + command: ["/bin/sh","-c","exec redis-server /redis-master/redis.conf"] env: - name: MASTER value: "true" diff --git a/tests/test_redis_shutdown.py b/tests/test_redis_shutdown.py new file mode 100644 index 0000000..1c73218 --- /dev/null +++ b/tests/test_redis_shutdown.py @@ -0,0 +1,90 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +# + +import os +from pathlib import Path +import signal +import subprocess +import tempfile +import time +import unittest + +from jinja2 import Template +import yaml + +ROOT = Path(__file__).resolve().parents[1] +MANIFESTS = ('deploy/redis/redis-set.yaml', 'openserverless/templates/redis-set.yaml') + + +@unittest.skipUnless(os.name == 'posix', 'Container shutdown requires POSIX signals') +class RedisShutdownTest(unittest.TestCase): + def test_server_receives_sigterm_through_each_configured_entrypoint(self): + for name in MANIFESTS: + with self.subTest(manifest=name), tempfile.TemporaryDirectory() as directory: + work = Path(directory) + binary = work / 'redis-server' + ready, stopped = work / 'ready', work / 'stopped' + binary.write_text("""#!/usr/bin/env python3 +import os +from pathlib import Path +import signal +import sys + +def stop(signum, frame): + Path(os.environ['OPS_TEST_STOPPED']).write_text('SIGTERM') + raise SystemExit(0) + +assert sys.argv[1:] == ['/redis-master/redis.conf'] +signal.signal(signal.SIGTERM, stop) +Path(os.environ['OPS_TEST_READY']).write_text('ready') +while True: + signal.pause() +""") + binary.chmod(0o755) + document = yaml.safe_load(Template((ROOT / name).read_text()).render( + applypodsecurity=False, persistence=False)) + container = next(c for c in document['spec']['template']['spec']['containers'] + if c['name'] == 'redis') + env = dict(os.environ, PATH=str(work) + os.pathsep + os.environ.get('PATH', ''), + OPS_TEST_READY=str(ready), OPS_TEST_STOPPED=str(stopped)) + process = subprocess.Popen(container['command'], env=env, start_new_session=True, + stdout=subprocess.DEVNULL, stderr=subprocess.PIPE) + try: + deadline = time.monotonic() + 10 + while not ready.exists() and time.monotonic() < deadline: + self.assertIsNone(process.poll(), 'Configured Redis entrypoint exited before readiness') + time.sleep(0.01) + self.assertTrue(ready.exists(), 'Configured Redis entrypoint did not start the server') + process.send_signal(signal.SIGTERM) + process.wait(timeout=5) + self.assertTrue(stopped.exists(), 'Redis server did not receive the container stop signal') + self.assertEqual(stopped.read_text(), 'SIGTERM') + self.assertEqual(process.returncode, 0) + finally: + # The old shell command leaves its child alive after TERM. + # Reap only the isolated process group created by this test. + try: + os.killpg(process.pid, signal.SIGKILL) + except ProcessLookupError: + pass + process.wait(timeout=5) + process.stderr.close() + + +if __name__ == '__main__': + unittest.main()