From e3749af838842efcd37a32fc0e9c8a921bea420f Mon Sep 17 00:00:00 2001 From: Matej Focko Date: Wed, 24 Apr 2024 15:07:16 +0200 Subject: [PATCH 1/5] fix(redis): move away from redis to redict MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Multiple options available: - KeyDB (multithreaded open-source fork maintained by Snapchat) - Valkey (fork keeping the former BSD license) - Redict (fork switching to LGPL license) For now switch to the Redict as our needs seem to be satisfied: - they have a wide variety of containers (scratch, alpine, debian) - as it has been tested (on prod), it works smooth as a drop-in replacement - it is also present in the Fedora and EPEL repos As for the switch itself: - introduce new Kubernetes definition for Redict deployment - introduce new variables ‹with_redis› and ‹with_redict› to allow customizable deployment, in case we decide to revert, or switch - introduce ‹redis_hostname› that is deduced from the ‹with_*› variables; also »FAIL« for neither of them being set Fixes #561 Signed-off-by: Matej Focko --- README.md | 4 +- openshift/flower.yml.j2 | 2 +- openshift/packit-service-beat.yml.j2 | 2 +- openshift/packit-service-fedmsg.yml.j2 | 2 +- openshift/packit-service.yml.j2 | 2 +- openshift/packit-worker.yml.j2 | 2 +- openshift/redict.yml.j2 | 79 ++++++++++++++++++++++++ openshift/redis-commander.yml.j2 | 2 +- playbooks/deploy.yml | 13 ++++ playbooks/tasks/set-deployment-facts.yml | 5 +- playbooks/tasks/set-facts.yml | 19 ++++++ 11 files changed, 123 insertions(+), 9 deletions(-) create mode 100644 openshift/redict.yml.j2 diff --git a/README.md b/README.md index 59ba6a1..813585a 100644 --- a/README.md +++ b/README.md @@ -57,10 +57,10 @@ To only disable comparing the variable file to the template, set To run only the tasks related to some of the services, this way doing a partial deployment, you can set the `TAGS` environment variable before calling -`make`. For example, to run only the tasks to deploy Redis and Redis +`make`. For example, to run only the tasks to deploy Redict and Redis Commander, run: - $ DEPLOYMENT=dev TAGS="redis,redis-commander" make deploy + $ DEPLOYMENT=dev TAGS="redict,redis-commander" make deploy Use `make tags` to list the currently available tags. diff --git a/openshift/flower.yml.j2 b/openshift/flower.yml.j2 index 5f01fa5..a1c566b 100644 --- a/openshift/flower.yml.j2 +++ b/openshift/flower.yml.j2 @@ -23,7 +23,7 @@ spec: image: quay.io/packit/flower env: - name: CELERY_BROKER_URL - value: redis://redis:6379/0 + value: redis://{{ redis_hostname }}:6379/0 - name: FLOWER_PORT value: "5555" ports: diff --git a/openshift/packit-service-beat.yml.j2 b/openshift/packit-service-beat.yml.j2 index afca64e..c2a23e0 100644 --- a/openshift/packit-service-beat.yml.j2 +++ b/openshift/packit-service-beat.yml.j2 @@ -41,7 +41,7 @@ spec: - name: APP value: {{ celery_app }} - name: REDIS_SERVICE_HOST - value: redis + value: {{ redis_hostname }} - name: POSTGRESQL_USER valueFrom: {secretKeyRef: {name: postgres-secret, key: database-user}} - name: POSTGRESQL_PASSWORD diff --git a/openshift/packit-service-fedmsg.yml.j2 b/openshift/packit-service-fedmsg.yml.j2 index 61bf6d6..871dab0 100644 --- a/openshift/packit-service-fedmsg.yml.j2 +++ b/openshift/packit-service-fedmsg.yml.j2 @@ -33,7 +33,7 @@ spec: - name: FEDORA_MESSAGING_CONF value: /home/packit/.config/fedora.toml - name: REDIS_SERVICE_HOST - value: redis + value: {{ redis_hostname }} - name: PROJECT value: {{ project }} - name: DEPLOYMENT diff --git a/openshift/packit-service.yml.j2 b/openshift/packit-service.yml.j2 index c136ddc..fd2b0e2 100644 --- a/openshift/packit-service.yml.j2 +++ b/openshift/packit-service.yml.j2 @@ -52,7 +52,7 @@ spec: - name: DISTGIT_NAMESPACE value: {{ distgit_namespace }} - name: REDIS_SERVICE_HOST - value: redis + value: {{ redis_hostname }} - name: POSTGRESQL_USER valueFrom: {secretKeyRef: {name: postgres-secret, key: database-user}} - name: POSTGRESQL_PASSWORD diff --git a/openshift/packit-worker.yml.j2 b/openshift/packit-worker.yml.j2 index a4fbe3b..8e299b4 100644 --- a/openshift/packit-worker.yml.j2 +++ b/openshift/packit-worker.yml.j2 @@ -86,7 +86,7 @@ spec: - name: APP value: {{ celery_app }} - name: REDIS_SERVICE_HOST - value: redis + value: {{ redis_hostname }} - name: POSTGRESQL_USER valueFrom: {secretKeyRef: {name: postgres-secret, key: database-user}} - name: POSTGRESQL_PASSWORD diff --git a/openshift/redict.yml.j2 b/openshift/redict.yml.j2 new file mode 100644 index 0000000..7dfa373 --- /dev/null +++ b/openshift/redict.yml.j2 @@ -0,0 +1,79 @@ +# Copyright Contributors to the Packit project. +# SPDX-License-Identifier: MIT + +--- +kind: Deployment +apiVersion: apps/v1 +metadata: + name: redict +spec: + selector: + matchLabels: + component: redict + template: + metadata: + labels: + component: redict +{% if managed_platform %} + paas.redhat.com/appcode: {{ appcode }} +{% endif %} + spec: + containers: + - name: redict + image: registry.redict.io/redict:7 + ports: + - containerPort: 6379 + volumeMounts: + - mountPath: /data + name: redict-pv + resources: + # requests and limits have to be the same to have Guaranteed QoS + requests: + memory: "64Mi" + cpu: "10m" + limits: + memory: "64Mi" + cpu: "10m" + volumes: + - name: redict-pv + persistentVolumeClaim: + claimName: redict-pvc + replicas: 1 + strategy: + type: Recreate +--- +apiVersion: v1 +kind: Service +metadata: + name: redict +{% if managed_platform %} + labels: + paas.redhat.com/appcode: {{ appcode }} +{% endif %} +spec: + ports: + - name: "6379" + port: 6379 + targetPort: 6379 + selector: + component: redict +--- +apiVersion: v1 +kind: PersistentVolumeClaim +metadata: + name: redict-pvc +{% if managed_platform %} + labels: + paas.redhat.com/appcode: {{ appcode }} + annotations: + kubernetes.io/reclaimPolicy: Delete +{% endif %} +spec: + accessModes: + - ReadWriteOnce + resources: + requests: + storage: 1Gi +{% if managed_platform %} + storageClassName: aws-ebs +{% endif %} diff --git a/openshift/redis-commander.yml.j2 b/openshift/redis-commander.yml.j2 index da30abc..608abf2 100644 --- a/openshift/redis-commander.yml.j2 +++ b/openshift/redis-commander.yml.j2 @@ -23,7 +23,7 @@ spec: image: rediscommander/redis-commander env: - name: REDIS_HOST - value: redis + value: {{ redis_hostname }} - name: REDIS_PORT value: "6379" - name: HTTP_USER diff --git a/playbooks/deploy.yml b/playbooks/deploy.yml index ad02028..61b6285 100644 --- a/playbooks/deploy.yml +++ b/playbooks/deploy.yml @@ -12,6 +12,8 @@ tenant: packit # MP+ tenant with_tokman: true with_fedmsg: true + with_redis: false + with_redict: true with_redis_commander: false with_flower: false with_dashboard: true @@ -183,9 +185,20 @@ ansible.builtin.include_tasks: tasks/k8s.yml loop: - "{{ lookup('template', '{{ project_dir }}/openshift/redis.yml.j2') }}" + when: with_redis tags: - redis + - name: Deploy redict + vars: + k8s_apply: true + ansible.builtin.include_tasks: tasks/k8s.yml + loop: + - "{{ lookup('template', '{{ project_dir }}/openshift/redict.yml.j2') }}" + when: with_redict + tags: + - redict + - name: Deploy fluentd image stream and config ansible.builtin.include_tasks: tasks/k8s.yml loop: diff --git a/playbooks/tasks/set-deployment-facts.yml b/playbooks/tasks/set-deployment-facts.yml index 64238a7..10987dc 100644 --- a/playbooks/tasks/set-deployment-facts.yml +++ b/playbooks/tasks/set-deployment-facts.yml @@ -9,10 +9,10 @@ ansible.builtin.set_fact: mandatory_deploymentconfigs: - postgres-{{ postgres_version }} - - redis - packit-service tags: - always + - name: Set optional_deploymentconfigs fact ansible.builtin.set_fact: optional_deploymentconfigs: @@ -22,8 +22,11 @@ packit-dashboard: "{{ with_dashboard }}" pushgateway: "{{ with_pushgateway }}" nginx: "{{ with_pushgateway }}" + redis: "{{ with_redis }}" + redict: "{{ with_redict }}" tags: - always + - name: Set deploymentconfigs fact ansible.builtin.set_fact: # To know what DCs rollouts to wait for and also to check in tests diff --git a/playbooks/tasks/set-facts.yml b/playbooks/tasks/set-facts.yml index 120bd3c..311dca3 100644 --- a/playbooks/tasks/set-facts.yml +++ b/playbooks/tasks/set-facts.yml @@ -27,3 +27,22 @@ ansible.builtin.set_fact: sandbox_namespace: "{{ service }}-{{ deployment }}-sandbox" when: not managed_platform + +- name: Set redis hostname + tags: + - always + block: + - name: Set redict as the redis hostname + ansible.builtin.set_fact: + redis_hostname: redict + when: with_redict + + - name: Set redis as the redis hostname (backward compatibility) + ansible.builtin.set_fact: + redis_hostname: redis + when: with_redis + + - name: Warn if neither is set + ansible.builtin.assert: + that: with_redict or with_redis + fail_msg: "[FAIL] Neither redict or redis is set to be deployed!" From 3e09de46ba7384f4189531612278c8a71fa23271 Mon Sep 17 00:00:00 2001 From: Matej Focko Date: Wed, 24 Apr 2024 15:27:17 +0200 Subject: [PATCH 2/5] docs(redict): document migration to redict Related to #561 Signed-off-by: Matej Focko --- docs/deployment/specifics/redict.md | 48 +++++++++++++++++++++++++++++ 1 file changed, 48 insertions(+) create mode 100644 docs/deployment/specifics/redict.md diff --git a/docs/deployment/specifics/redict.md b/docs/deployment/specifics/redict.md new file mode 100644 index 0000000..c35d0ec --- /dev/null +++ b/docs/deployment/specifics/redict.md @@ -0,0 +1,48 @@ +--- +title: Redict +--- + +## Seamless migration between providers + +We have tested a seamless migration from Redis to Redict on our production +deployment. To reproduce: + +1. We have deployed Redict to our production cluster. +2. Using remote shell and `redict-cli` run: + + ```sh + replicaof redis 6379 + ``` + + This converts the Redict instance into a read-only replica of the Redis. + +3. After the data exchange is done, change **all** references in variables to + redis to point to the new hostname, in this case `redis → redict`. +4. Simultaneously run the deployment with the changed hostnames and via + `redict-cli` run: + + ```sh + replicaof no one + ``` + + to make the redict deployment the primary one. + +5. (optional) For safety reasons and easier rollback, it's possible to convert + the former Redis deployment into a replica of Redict, just in case it needs + to be reverted without loss of data. For this you can run in `redis-cli`: + + ```sh + replicaof redict 6379 + ``` + +:::warning References to Redis + +Redis is being referenced from: + +- `packit-service` (API endpoint) +- `packit-service-fedmsg` (Fedora Messaging listener) +- `packit-service-beat` (triggers periodic tasks) +- `packit-worker` (runs the jobs provided by API, Fedora Messaging and “beat”) +- `flower` (monitoring of the Celery queues) + +::: From 013d470461550871a12e1c8a4ef067b4dbffd70a Mon Sep 17 00:00:00 2001 From: Matej Focko Date: Thu, 2 May 2024 11:32:25 +0200 Subject: [PATCH 3/5] fix(redict): up the resources MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit As it has been discovered¹, Redict hit some stability issues. The only common culprit appears to be spike in the memory usage, therefore raise the requested memory 2× and limit for the memory 4×. Details of the first occurrence from the issue for migrating from Redis to open-source alternative: - Smallish issue has been hit on the Thursday afternoon, first-response and fix by @nforro; appears to be caused by network flakes - _Redict logs_: Redis requested multiple times resync, failed on timeout and _Connection reset_; cause of restarts is unknown - _Sentry events_: related to the Redis sync and also failed connections to the Redict - @mfocko scaled down Redis on Monday as it doesn't appear we're hitting any blockers and will need to quick swap ¹ on at least 2 occasions Signed-off-by: Matej Focko --- openshift/redict.yml.j2 | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/openshift/redict.yml.j2 b/openshift/redict.yml.j2 index 7dfa373..6486950 100644 --- a/openshift/redict.yml.j2 +++ b/openshift/redict.yml.j2 @@ -29,10 +29,10 @@ spec: resources: # requests and limits have to be the same to have Guaranteed QoS requests: - memory: "64Mi" + memory: "128Mi" cpu: "10m" limits: - memory: "64Mi" + memory: "256Mi" cpu: "10m" volumes: - name: redict-pv From de337be5337ae7f9bc77b537922dafcc20101cf1 Mon Sep 17 00:00:00 2001 From: Matej Focko Date: Thu, 2 May 2024 11:52:06 +0200 Subject: [PATCH 4/5] fix(facts): improve the resolution of Redis-like hostname MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Create a dummy hostname for the purpose of nice debugging message - Improve the assertion to check for »exactly« one of the Redis, or Redict to be deployed - For success print the hostname that will be used. For fail print warning and hint what to change. Signed-off-by: Matej Focko --- playbooks/tasks/set-facts.yml | 21 +++++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/playbooks/tasks/set-facts.yml b/playbooks/tasks/set-facts.yml index 311dca3..8110128 100644 --- a/playbooks/tasks/set-facts.yml +++ b/playbooks/tasks/set-facts.yml @@ -28,21 +28,30 @@ sandbox_namespace: "{{ service }}-{{ deployment }}-sandbox" when: not managed_platform -- name: Set redis hostname +- name: Set Redis-like hostname tags: - always block: - - name: Set redict as the redis hostname + # Needed for nice message of the sanity check + - name: Set default for the hostname + ansible.builtin.set_fact: + redis_hostname: None + + - name: Set Redict as the hostname ansible.builtin.set_fact: redis_hostname: redict when: with_redict - - name: Set redis as the redis hostname (backward compatibility) + - name: Set Redis as the hostname (backward compatibility) ansible.builtin.set_fact: redis_hostname: redis when: with_redis - - name: Warn if neither is set + - name: Sanity check for deploying exactly one of Redis or Redict ansible.builtin.assert: - that: with_redict or with_redis - fail_msg: "[FAIL] Neither redict or redis is set to be deployed!" + that: with_redict != with_redis + success_msg: | + [INFO] Deploying {{ redis_hostname }} + fail_msg: | + [FAIL] Check vars (‹with_redict› and ‹with_redis›). + Cannot deploy none or both of Redis and Redict! From c4a05932aeabfb113fcf1d3746b8ba49e3ac2969 Mon Sep 17 00:00:00 2001 From: Matej Focko Date: Thu, 2 May 2024 11:59:28 +0200 Subject: [PATCH 5/5] docs(redict): improve the steps to reproduce Signed-off-by: Matej Focko --- docs/deployment/specifics/redict.md | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/docs/deployment/specifics/redict.md b/docs/deployment/specifics/redict.md index c35d0ec..29a563e 100644 --- a/docs/deployment/specifics/redict.md +++ b/docs/deployment/specifics/redict.md @@ -8,6 +8,22 @@ We have tested a seamless migration from Redis to Redict on our production deployment. To reproduce: 1. We have deployed Redict to our production cluster. + + - Defaults have been changed to: + + ```yaml + with_redis: false + with_redict: true + ``` + + These can be changed in their respective `vars/` files. + + - Run + + ``` + DEPLOYMENT=prod TAGS=redict make deploy + ``` + 2. Using remote shell and `redict-cli` run: ```sh @@ -18,6 +34,13 @@ deployment. To reproduce: 3. After the data exchange is done, change **all** references in variables to redis to point to the new hostname, in this case `redis → redict`. + + - Run + + ``` + DEPLOYMENT=prod TAGS=packit-service-beat,fedmsg,packit-worker,packit-service make deploy + ``` + 4. Simultaneously run the deployment with the changed hostnames and via `redict-cli` run: