Skip to content
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

Jump to bottom

Fix missing redis configuration #117

Open
wants to merge 11 commits into
base: master
Choose a base branch
from
Open

Fix missing redis configuration #117

wants to merge 11 commits into from

Conversation

pon0marev
Copy link
Collaborator

@pon0marev pon0marev commented Feb 1, 2024 •
edited

Due to the missing Redis configuration in the database-setup.yml file, the upgrade proceeded without clearing the Redis cache, and this led to failures in launching the new version.
This is fixed by adding config maps with Redis variables to the tb-db-setup pod.

Updated Redis to the latest version compatible with the ThingsBoard. Added a note about Redis version compatibility.

smatvienko-tb
smatvienko-tb suggested changes Feb 1, 2024
View reviewed changes
Copy link
Contributor

@smatvienko-tb smatvienko-tb left a comment

Choose a reason for hiding this comment

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

Good progress.
Please take note that Redis can have a password, and you need a secret map like

      - name: REDIS_PASSWORD
        valueFrom:
          secretKeyRef:
            name: tb-redis
            key: password

minikube/tb-redis-configmap.yml Outdated
# Make sure that the value does not contain the port (:6379).
CACHE_TYPE: redis
REDIS_CONNECTION_TYPE: standalone
REDIS_HOST: tb-redis
Copy link
Contributor

Choose a reason for hiding this comment

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

Please enable a non-default pool to unlock a performance. Default pool in only 8 connections wide.

  #REDIS POOL CONFIG
  REDIS_USE_DEFAULT_POOL_CONFIG: "false"

minikube/tb-redis-configmap.yml Outdated
apiVersion: v1
kind: ConfigMap
metadata:
name: tb-redis-config
Copy link
Contributor

Choose a reason for hiding this comment

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

I suggest a tb-cache-config name
See an example on our QA cluster
the cache related env is not only about redis

  #CACHE
  #  CACHE_MAXIMUM_POOL_SIZE: "50" # default 16 # max pool size to process futures for the external cache
  #  TS_KV_PARTITIONS_MAX_CACHE_SIZE: "2000000" # default 100000 # helps to avoid excessive writes to Cassandra if partitioned

azure/microservices/database-setup.yml
@@ -43,6 +43,8 @@ spec:
envFrom:
- configMapRef:
name: tb-node-db-config
- configMapRef:
Copy link
Contributor

Choose a reason for hiding this comment

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

I guess it would be nice to have a complete set of variables and secrets that tb-node has. You never know when developers decide to work on Kafka or Zookeeper during the update.

    envFrom:
      - configMapRef:
          name: tb-node-db-config
      - configMapRef:
          name: tb-node-env-config
      - configMapRef:
          name: tb-kafka-config
      - configMapRef:
          name: tb-cache-config

smatvienko-tb
smatvienko-tb reviewed Feb 5, 2024
View reviewed changes
Copy link
Contributor

@smatvienko-tb smatvienko-tb left a comment

Choose a reason for hiding this comment

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

In general, it looks nominal. Duplicated config found. Some minor comments below.
Overall, I don't like that the same config copied many times and not all projects has a similar configmaps (see kafka-confg for minikube)

azure/microservices/tb-cache-configmap.yml
# Make sure that the value does not contain the port (:6379).
CACHE_TYPE: redis
REDIS_HOST: YOUR_REDIS_ENDPOINT_URL_WITHOUT_PORT
REDIS_PASSWORD: YOU_REDIS_PASS
Copy link
Contributor

Choose a reason for hiding this comment

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

passwords is better to store in secrets

azure/microservices/tb-redis-configmap.yml
@@ -26,3 +26,5 @@ data:
CACHE_TYPE: redis
REDIS_HOST: YOUR_REDIS_ENDPOINT_URL_WITHOUT_PORT
REDIS_PASSWORD: YOU_REDIS_PASS
#REDIS POOL CONFIG
REDIS_USE_DEFAULT_POOL_CONFIG: "false"
Copy link
Contributor

Choose a reason for hiding this comment

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

Duplicated cache conf files. should be a single cache config file
image

gcp/microservices/k8s-deploy-resources.sh
@@ -24,5 +24,5 @@ kubectl apply -f thirdparty.yml
kubectl apply -f tb-node-db-configmap.yml
kubectl apply -f tb-node-configmap.yml
kubectl apply -f tb-kafka-configmap.yml
kubectl apply -f tb-redis-configmap.yml
Copy link
Contributor

Choose a reason for hiding this comment

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

it is very minor, but the order of the commands is different for k8s-deploy-resources.sh for different providers.

openshift/tb-cache-configmap.yml
# Make sure that the value does not contain the port (:6379).
CACHE_TYPE: redis
REDIS_CONNECTION_TYPE: standalone
REDIS_HOST: tb-redis
Copy link
Contributor

Choose a reason for hiding this comment

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

does the database setup has no changes for the OpenShift ?

@pon0marev pon0marev force-pushed the fix-upgrade-pod-misconfiguration branch from 3b527b2 to b9d4331 Compare June 10, 2024 13:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@smatvienko-tb smatvienko-tb smatvienko-tb requested changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@pon0marev @smatvienko-tb