Skip to content

Conversation

@xieandrew
Copy link
Contributor

Fixes the helm chart when using the default values.yaml for EKS to pass all tests in python/tests. Improves documentation for using the helm chart on EKS.

Copy link
Member

@hodgesrm hodgesrm left a comment

Choose a reason for hiding this comment

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

This looks good to me with a cosmetic change to remove commented-out fields.

Question 1: how does Helm match the manifest directory contents now?

- host: keeper-keeper
port: 2181
users:
root/password: topsecret
Copy link
Member

Choose a reason for hiding this comment

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

This can wait for a later PR but we should use secrets rather than expose passwords. See https://altinity.com/blog/clickhouse-confidential-using-kubernetes-secrets-with-the-altinity-operator#using-secrets-to-protect-clickhouse-user-passwords for more information.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will look into using secrets. But this section was added so the test_vector_sever_version auth header here works. If the test is changed to not use the auth header, this root password is not needed. Not sure if that's an issue with the test or if we do need a root password specified, because I don't see this password in the docker compose setup.

@xieandrew
Copy link
Contributor Author

The helm chart is almost identical to the manifest directory when values.yaml is applied, I believe the swarm in helm has a shardsCount of 1 instead of 4 though.

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.

2 participants