From 4fefa3770d1337c36c6b5f3ea12aff14fc0ef386 Mon Sep 17 00:00:00 2001 From: Matej Focko Date: Fri, 20 Sep 2024 23:06:54 +0200 Subject: [PATCH 1/5] feat: make key-value DB deployment generic MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 1. Rely on a single tag ‹with_kv_database› for deciding whether it should be deployed or not. 2. Introduce a new variable ‹kv_database› that holds the name of the KV database that might be deployed. - Also allows for easier deduction of the hostname and makes sure that there's always at least one deployment of any kind, i.e., it is now not possible to require it present, but have incorrect combination of flags, e.g., neither ‹with_redis› nor ‹with_redict› set. 3. Add it to templates where it's been forgotten. Signed-off-by: Matej Focko --- playbooks/deploy.yml | 18 ++++++---------- playbooks/roles/deploy/defaults/main.yml | 4 ++-- playbooks/roles/deploy/tasks/main.yml | 14 ++++--------- playbooks/tasks/set-deployment-facts.yml | 4 ++-- playbooks/tasks/set-facts.yml | 26 ++---------------------- vars/packit/dev_template.yml | 3 +++ vars/packit/prod_template.yml | 3 +++ vars/packit/stg_template.yml | 3 +++ 8 files changed, 25 insertions(+), 50 deletions(-) diff --git a/playbooks/deploy.yml b/playbooks/deploy.yml index 77f0987..0cddac8 100644 --- a/playbooks/deploy.yml +++ b/playbooks/deploy.yml @@ -12,8 +12,8 @@ tenant: packit # MP+ tenant with_tokman: true with_fedmsg: true - with_redis: false - with_redict: true + kv_database: "redict" + with_kv_database: true with_redis_commander: false with_flower: false with_dashboard: true @@ -183,20 +183,14 @@ tags: - postgres - - name: Deploy redis + - name: Deploy key-value database ({{ kv_database }}) ansible.builtin.include_tasks: tasks/k8s.yml loop: - - "{{ lookup('template', '{{ project_dir }}/openshift/redis.yml.j2') }}" - when: with_redis + - "{{ lookup('template', '{{ project_dir }}/openshift/{{ kv_database }}.yml.j2') }}" + when: with_kv_database tags: + - kv_database - redis - - - name: Deploy redict - 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 diff --git a/playbooks/roles/deploy/defaults/main.yml b/playbooks/roles/deploy/defaults/main.yml index aa3b5b1..8dc3c11 100644 --- a/playbooks/roles/deploy/defaults/main.yml +++ b/playbooks/roles/deploy/defaults/main.yml @@ -5,8 +5,8 @@ deployment: "{{ lookup('env', 'DEPLOYMENT') }}" # noqa: var-naming[no-role-prefi tenant: packit # noqa: var-naming[no-role-prefix] # MP+ tenant with_tokman: true # noqa: var-naming[no-role-prefix] with_fedmsg: true # noqa: var-naming[no-role-prefix] -with_redis: false # noqa: var-naming[no-role-prefix] -with_redict: true # noqa: var-naming[no-role-prefix] +kv_database: "redict" # noqa: var-naming[no-role-prefix] +with_kv_database: true # noqa: var-naming[no-role-prefix] with_redis_commander: false # noqa: var-naming[no-role-prefix] with_flower: false # noqa: var-naming[no-role-prefix] with_dashboard: true # noqa: var-naming[no-role-prefix] diff --git a/playbooks/roles/deploy/tasks/main.yml b/playbooks/roles/deploy/tasks/main.yml index 0ae4603..b546d50 100644 --- a/playbooks/roles/deploy/tasks/main.yml +++ b/playbooks/roles/deploy/tasks/main.yml @@ -141,20 +141,14 @@ tags: - postgres -- name: Deploy redis +- name: Deploy key-value database ({{ kv_database }}) ansible.builtin.include_tasks: tasks/k8s.yml loop: - - "{{ lookup('template', '{{ project_dir }}/openshift/redis.yml.j2') }}" - when: with_redis + - "{{ lookup('template', '{{ project_dir }}/openshift/{{ kv_database }}.yml.j2') }}" + when: with_kv_database tags: + - kv_database - redis - -- name: Deploy redict - 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 diff --git a/playbooks/tasks/set-deployment-facts.yml b/playbooks/tasks/set-deployment-facts.yml index 10987dc..d82c783 100644 --- a/playbooks/tasks/set-deployment-facts.yml +++ b/playbooks/tasks/set-deployment-facts.yml @@ -22,8 +22,8 @@ packit-dashboard: "{{ with_dashboard }}" pushgateway: "{{ with_pushgateway }}" nginx: "{{ with_pushgateway }}" - redis: "{{ with_redis }}" - redict: "{{ with_redict }}" + redis: "{{ with_kv_database and kv_database == 'redis' }}" + redict: "{{ with_kv_database and kv_database == 'redict' }}" tags: - always diff --git a/playbooks/tasks/set-facts.yml b/playbooks/tasks/set-facts.yml index 8110128..a66ba25 100644 --- a/playbooks/tasks/set-facts.yml +++ b/playbooks/tasks/set-facts.yml @@ -31,27 +31,5 @@ - name: Set Redis-like hostname tags: - always - block: - # 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 hostname (backward compatibility) - ansible.builtin.set_fact: - redis_hostname: redis - when: with_redis - - - name: Sanity check for deploying exactly one of Redis or Redict - ansible.builtin.assert: - 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! + ansible.builtin.set_fact: + redis_hostname: "{{ kv_database }}" diff --git a/vars/packit/dev_template.yml b/vars/packit/dev_template.yml index 2d2ed9c..8ee78cd 100644 --- a/vars/packit/dev_template.yml +++ b/vars/packit/dev_template.yml @@ -30,6 +30,9 @@ check_up_to_date: false # edit the queue name in secrets/*/fedora.toml with_fedmsg: false +# kv_database: redict +# with_kv_database: true + with_redis_commander: false with_flower: true diff --git a/vars/packit/prod_template.yml b/vars/packit/prod_template.yml index fee7ae9..e91ab23 100644 --- a/vars/packit/prod_template.yml +++ b/vars/packit/prod_template.yml @@ -32,6 +32,9 @@ api_key: "" # edit the queue name in secrets/*/fedora.toml # with_fedmsg: true +# kv_database: redict +# with_kv_database: true + # with_redis_commander: false with_flower: true diff --git a/vars/packit/stg_template.yml b/vars/packit/stg_template.yml index bf863d8..bc1b143 100644 --- a/vars/packit/stg_template.yml +++ b/vars/packit/stg_template.yml @@ -30,6 +30,9 @@ api_key: "" # edit the queue name in secrets/*/fedora.toml # with_fedmsg: true +# kv_database: redict +# with_kv_database: true + # with_redis_commander: false with_flower: true From cb0e2032c19c49a8d70626641aeb2edaaf41f0bc Mon Sep 17 00:00:00 2001 From: Matej Focko Date: Fri, 20 Sep 2024 23:30:03 +0200 Subject: [PATCH 2/5] feat: add OpenShift definition for Valkey Signed-off-by: Matej Focko --- openshift/valkey.yml.j2 | 79 +++++++++++++++++++++++++++++++++++ vars/packit/prod_template.yml | 2 +- vars/packit/stg_template.yml | 2 +- 3 files changed, 81 insertions(+), 2 deletions(-) create mode 100644 openshift/valkey.yml.j2 diff --git a/openshift/valkey.yml.j2 b/openshift/valkey.yml.j2 new file mode 100644 index 0000000..1deab4f --- /dev/null +++ b/openshift/valkey.yml.j2 @@ -0,0 +1,79 @@ +# Copyright Contributors to the Packit project. +# SPDX-License-Identifier: MIT + +--- +kind: Deployment +apiVersion: apps/v1 +metadata: + name: valkey +spec: + selector: + matchLabels: + component: valkey + template: + metadata: + labels: + component: valkey +{% if managed_platform %} + paas.redhat.com/appcode: {{ appcode }} +{% endif %} + spec: + containers: + - name: valkey + image: valkey/valkey:8.0.0 + ports: + - containerPort: 6379 + volumeMounts: + - mountPath: /data + name: valkey-pv + resources: + # requests and limits have to be the same to have Guaranteed QoS + requests: + memory: "128Mi" + cpu: "10m" + limits: + memory: "256Mi" + cpu: "10m" + volumes: + - name: valkey-pv + persistentVolumeClaim: + claimName: valkey-pvc + replicas: 1 + strategy: + type: Recreate +--- +apiVersion: v1 +kind: Service +metadata: + name: valkey +{% if managed_platform %} + labels: + paas.redhat.com/appcode: {{ appcode }} +{% endif %} +spec: + ports: + - name: "6379" + port: 6379 + targetPort: 6379 + selector: + component: valkey +--- +apiVersion: v1 +kind: PersistentVolumeClaim +metadata: + name: valkey-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/vars/packit/prod_template.yml b/vars/packit/prod_template.yml index e91ab23..c4ff4ba 100644 --- a/vars/packit/prod_template.yml +++ b/vars/packit/prod_template.yml @@ -32,7 +32,7 @@ api_key: "" # edit the queue name in secrets/*/fedora.toml # with_fedmsg: true -# kv_database: redict +# kv_database: valkey # with_kv_database: true # with_redis_commander: false diff --git a/vars/packit/stg_template.yml b/vars/packit/stg_template.yml index bc1b143..7040a1e 100644 --- a/vars/packit/stg_template.yml +++ b/vars/packit/stg_template.yml @@ -30,7 +30,7 @@ api_key: "" # edit the queue name in secrets/*/fedora.toml # with_fedmsg: true -# kv_database: redict +# kv_database: valkey # with_kv_database: true # with_redis_commander: false From 1596ca4c0351ef60deb749c6a404aa1efb6297df Mon Sep 17 00:00:00 2001 From: Maja Massarini Date: Thu, 3 Oct 2024 12:05:50 +0200 Subject: [PATCH 3/5] Fix typo in ephemeral-storage request --- openshift/packit-service-beat.yml.j2 | 2 +- openshift/packit-service.yml.j2 | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/openshift/packit-service-beat.yml.j2 b/openshift/packit-service-beat.yml.j2 index df4708e..cf6eb28 100644 --- a/openshift/packit-service-beat.yml.j2 +++ b/openshift/packit-service-beat.yml.j2 @@ -70,7 +70,7 @@ spec: requests: memory: "160Mi" cpu: "5m" - epehemeral-storage: "80Ki" + ephemeral-storage: "80Ki" limits: memory: "256Mi" cpu: "50m" diff --git a/openshift/packit-service.yml.j2 b/openshift/packit-service.yml.j2 index bdd6637..9961f22 100644 --- a/openshift/packit-service.yml.j2 +++ b/openshift/packit-service.yml.j2 @@ -91,7 +91,7 @@ spec: requests: memory: "320Mi" cpu: "10m" - epehemeral-storage: "300Ki" + ephemeral-storage: "300Ki" limits: # run_httpd.sh does 'alembic upgrade head' which might require more memory # If you see '/usr/bin/run_httpd.sh: line 16: Killed alembic upgrade head' From 64cdcec63077eb97b0ab844dce32f0ffd07c1c8a Mon Sep 17 00:00:00 2001 From: Matej Focko Date: Tue, 8 Oct 2024 12:56:04 +0200 Subject: [PATCH 4/5] feat(kv-db): add default config for redis-like DBs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The leaks in short-running pods result in idle connections to Redict/Valkey, all of these KV-databases have ‹timeout› option in their config that allows for iterative cleanup of hanging connections. This mitigates the issue to the point of still having free connections slots to Redict/Valkey, i.e., the pods shall be killed, but handlers do not end up in a retry-loop trying to connect to Redict/Valkey. Since the config is 1:1 between all Redis, Redict, and Valkey, create one ConfigMap, map the config into the databases and pass the path to the config as an argument. Tested with Redict and Valkey. »NOT« tested with Redis. Related to packit/packit-service#2522 Signed-off-by: Matej Focko --- openshift/configmap-redis_like_config.yml | 8 ++++++++ openshift/redict.yml.j2 | 6 ++++++ openshift/valkey.yml.j2 | 6 ++++++ playbooks/deploy.yml | 1 + playbooks/roles/deploy/tasks/main.yml | 1 + 5 files changed, 22 insertions(+) create mode 100644 openshift/configmap-redis_like_config.yml diff --git a/openshift/configmap-redis_like_config.yml b/openshift/configmap-redis_like_config.yml new file mode 100644 index 0000000..f234dbd --- /dev/null +++ b/openshift/configmap-redis_like_config.yml @@ -0,0 +1,8 @@ +--- +kind: ConfigMap +apiVersion: v1 +metadata: + name: redis-like-config +data: + redis.conf: | + timeout 1800 diff --git a/openshift/redict.yml.j2 b/openshift/redict.yml.j2 index 6486950..80889f9 100644 --- a/openshift/redict.yml.j2 +++ b/openshift/redict.yml.j2 @@ -21,11 +21,15 @@ spec: containers: - name: redict image: registry.redict.io/redict:7 + args: + - "/etc/redislike/redis.conf" ports: - containerPort: 6379 volumeMounts: - mountPath: /data name: redict-pv + - mountPath: /etc/redislike + name: redis-like-config resources: # requests and limits have to be the same to have Guaranteed QoS requests: @@ -38,6 +42,8 @@ spec: - name: redict-pv persistentVolumeClaim: claimName: redict-pvc + - name: redis-like-config + configMap: {name: redis-like-config} replicas: 1 strategy: type: Recreate diff --git a/openshift/valkey.yml.j2 b/openshift/valkey.yml.j2 index 1deab4f..12e95c2 100644 --- a/openshift/valkey.yml.j2 +++ b/openshift/valkey.yml.j2 @@ -21,11 +21,15 @@ spec: containers: - name: valkey image: valkey/valkey:8.0.0 + args: + - "/etc/redislike/redis.conf" ports: - containerPort: 6379 volumeMounts: - mountPath: /data name: valkey-pv + - mountPath: /etc/redislike + name: redis-like-config resources: # requests and limits have to be the same to have Guaranteed QoS requests: @@ -38,6 +42,8 @@ spec: - name: valkey-pv persistentVolumeClaim: claimName: valkey-pvc + - name: redis-like-config + configMap: {name: redis-like-config} replicas: 1 strategy: type: Recreate diff --git a/playbooks/deploy.yml b/playbooks/deploy.yml index 0cddac8..f9cc696 100644 --- a/playbooks/deploy.yml +++ b/playbooks/deploy.yml @@ -186,6 +186,7 @@ - name: Deploy key-value database ({{ kv_database }}) ansible.builtin.include_tasks: tasks/k8s.yml loop: + - "{{ lookup('file', '{{ project_dir }}/openshift/configmap-redis_like_config.yml') }}" - "{{ lookup('template', '{{ project_dir }}/openshift/{{ kv_database }}.yml.j2') }}" when: with_kv_database tags: diff --git a/playbooks/roles/deploy/tasks/main.yml b/playbooks/roles/deploy/tasks/main.yml index b546d50..f35e1cc 100644 --- a/playbooks/roles/deploy/tasks/main.yml +++ b/playbooks/roles/deploy/tasks/main.yml @@ -144,6 +144,7 @@ - name: Deploy key-value database ({{ kv_database }}) ansible.builtin.include_tasks: tasks/k8s.yml loop: + - "{{ lookup('file', '{{ project_dir }}/openshift/configmap-redis_like_config.yml') }}" - "{{ lookup('template', '{{ project_dir }}/openshift/{{ kv_database }}.yml.j2') }}" when: with_kv_database tags: From 660a23a3c04e7163db257cb64e5777e45981119f Mon Sep 17 00:00:00 2001 From: Matej Focko Date: Wed, 9 Oct 2024 09:52:41 +0200 Subject: [PATCH 5/5] fix(valkey): implement remarks from code review - Remove tags for redis and redict, they would not be very helpful, since the specific implementation of KV-DB is taken from the variables anyways - Add missing Valkey to the deploymentconfigs - Add comment with options to the deployment templates Co-authored-by: Maja Massarini Signed-off-by: Matej Focko --- playbooks/deploy.yml | 2 -- playbooks/roles/deploy/tasks/main.yml | 2 -- playbooks/tasks/set-deployment-facts.yml | 1 + vars/packit/dev_template.yml | 6 ++++-- vars/packit/prod_template.yml | 4 +++- vars/packit/stg_template.yml | 4 +++- 6 files changed, 11 insertions(+), 8 deletions(-) diff --git a/playbooks/deploy.yml b/playbooks/deploy.yml index f9cc696..6e427bd 100644 --- a/playbooks/deploy.yml +++ b/playbooks/deploy.yml @@ -191,8 +191,6 @@ when: with_kv_database tags: - kv_database - - redis - - redict - name: Deploy fluentd image stream and config ansible.builtin.include_tasks: tasks/k8s.yml diff --git a/playbooks/roles/deploy/tasks/main.yml b/playbooks/roles/deploy/tasks/main.yml index f35e1cc..4316fc2 100644 --- a/playbooks/roles/deploy/tasks/main.yml +++ b/playbooks/roles/deploy/tasks/main.yml @@ -149,8 +149,6 @@ when: with_kv_database tags: - kv_database - - redis - - redict - name: Deploy fluentd image stream and config ansible.builtin.include_tasks: tasks/k8s.yml diff --git a/playbooks/tasks/set-deployment-facts.yml b/playbooks/tasks/set-deployment-facts.yml index d82c783..24d4200 100644 --- a/playbooks/tasks/set-deployment-facts.yml +++ b/playbooks/tasks/set-deployment-facts.yml @@ -24,6 +24,7 @@ nginx: "{{ with_pushgateway }}" redis: "{{ with_kv_database and kv_database == 'redis' }}" redict: "{{ with_kv_database and kv_database == 'redict' }}" + valkey: "{{ with_kv_database and kv_database == 'valkey' }}" tags: - always diff --git a/vars/packit/dev_template.yml b/vars/packit/dev_template.yml index 8ee78cd..b8e0b61 100644 --- a/vars/packit/dev_template.yml +++ b/vars/packit/dev_template.yml @@ -30,8 +30,10 @@ check_up_to_date: false # edit the queue name in secrets/*/fedora.toml with_fedmsg: false -# kv_database: redict -# with_kv_database: true +# kv_database options: redis redict valkey +# Current default on both deployments: valkey +kv_database: valkey +with_kv_database: true with_redis_commander: false diff --git a/vars/packit/prod_template.yml b/vars/packit/prod_template.yml index c4ff4ba..1048e5a 100644 --- a/vars/packit/prod_template.yml +++ b/vars/packit/prod_template.yml @@ -32,7 +32,9 @@ api_key: "" # edit the queue name in secrets/*/fedora.toml # with_fedmsg: true -# kv_database: valkey +# kv_database options: redis redict valkey +# Current default on both deployments: valkey +kv_database: valkey # with_kv_database: true # with_redis_commander: false diff --git a/vars/packit/stg_template.yml b/vars/packit/stg_template.yml index 7040a1e..45bdc42 100644 --- a/vars/packit/stg_template.yml +++ b/vars/packit/stg_template.yml @@ -30,7 +30,9 @@ api_key: "" # edit the queue name in secrets/*/fedora.toml # with_fedmsg: true -# kv_database: valkey +# kv_database options: redis redict valkey +# Current default on both deployments: valkey +kv_database: valkey # with_kv_database: true # with_redis_commander: false