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.

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

various chart improvments #1

Open
wants to merge 23 commits into
base: main
Choose a base branch
from
Open

Conversation

Spirit-act
Copy link

@Spirit-act Spirit-act commented Feb 14, 2024

Hello,

I tested this Helm Chart for our setup and discovered some drawbacks.

Changes:

Add support for environment variables

  • Add default envs for a simple deployment with the included Redis
  • Add a helper to the _helpers.tpl to convert the envs from the value.yaml to the correct format
  • I needed to build a workaround for the remotecv pod. For some reason remotecv does not take the env vars automatically
args:
  - "--host=$(REDIS_HOST)"
  - "--port=$(REDIS_PORT)"
  - "--with-healthcheck"
  - "--server-port=$(HTTP_SERVER_PORT)"
  - "--password=$(REDIS_PASSWORD)"

Add initContainer to remotecv

Remotecv requires connection to Redis. If Redis is deployed with this Helm Chart, it is not accessible right from the start. I added an initContainer which waits until the configured Redis is accessible.

initContainers:
  - name: wait-for-redis
    image: busybox
    command:
      - /bin/sh
      - -c
      - >
        COUNT=0
        while [ $(echo -e "AUTH $REDIS_PASSWORD\r\nPING\r\n" | nc $REDIS_HOST $REDIS_PORT | grep -c "PONG") -ne 1 ]; do
          if [[ $COUNT -ge 10 ]]; then
            echo "Waited to long restarting POD"
            exit 1;
          fi

          echo "Waiting for Redis...";
          sleep 1;
          ((COUNT++))
        done

This feature should probably be added to the remotecv container.

Config / values.yaml changes

  • move thumbor config in a configmap but keep thumbor.key in the secret
  • control Redis and remotecv installation via separate variables (not within the thumbor_config)
  • scale redis to only the master pod in the default deployment. For small deployments with only one Pod for each component (thumbor, remotecv, redis) it should be enough. It can easily be scaled up again, if replicas are needed (documented variable).

Comments

  • add more documenting comments to the values.yaml
  • regenerate README.md with helm-docs

Other

  • I have increased the Chart version to 1.0.0 because of the possible breaking changes (We will be using this Chart in prod).
  • I have changed the Markdown Syntax in the top README.md according to the linter.

If something is not clear, please ask. I am happy to answer.

Spirit-act and others added 23 commits February 8, 2024 14:17
helper supports key:value and valueFrom maps

Signed-off-by: Spirit-act <[email protected]>
- make helper use relativ structure
- add workaround for env vars. thumbor remotecv for some reason does not
  use the supplied env variables. They need to be commandline switches
- change debug level to warning

Signed-off-by: Spirit-act <[email protected]>
Signed-off-by: Spirit-act <[email protected]>
- move thumbor config in a confimag
- update dependcies
- thumbor remotecv and redis need each other. If no remotecv why install
  redis. So make redis dependend on remotecv
- for small deployments a replicated redis is not needed: no redis
  replicas by default
- minimize thumbor_config variable

Signed-off-by: Spirit-act <[email protected]>
- add an initContainer which waits until the configured redis ist
  accessable
- split remotecv and redis installation in two different variables to
  allow remotecv installation without redis
  -> This also allows redis installation without remotecv. I have not
  found a away to check for both variables

Signed-off-by: Spirit-act <[email protected]>
- update values with comments
- regenerate README.md
- increment Chart version to major version because of the possible
  breaking changes

Signed-off-by: Spirit-act <[email protected]>
update helm-docs to latest version (1.12.0)

Signed-off-by: Spirit-act <[email protected]>
trigger pipelines on prod branch aswell
allow manual pipeline triggers
if thumbor_key.manage was set to false, the --keyfile start flag was not
    removed.

Signed-off-by: Spirit-act <[email protected]>
Wait for 10 Seconds and fail afterwards to restart POD.
For some reason the lookup just hangs or does simply not succeed, this
would mean, that the POD stuck in the init forever. If the POD is killed
and deploys new, the connection works directly.

Signed-off-by: Spirit-act <[email protected]>
Signed-off-by: Spirit-act <[email protected]>
Signed-off-by: Spirit-act <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant