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

ci: add helm-unittest & test hybrid params #188

Merged
merged 3 commits into from
Nov 27, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions .github/workflows/helm_checks.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,12 @@ jobs:
- name: Run chart-testing (lint)
run: ct lint --target-branch ${{ github.event.repository.default_branch }} --charts charts/${{ matrix.chart-name }}

- name: Install Helm Unittest
run: helm plugin install https://github.com/helm-unittest/helm-unittest.git --version 0.7.0

- name: Run chart unittest
run: helm unittest charts/${{ matrix.chart-name }}

- name: Create kind cluster
uses: helm/[email protected]

Expand Down
2 changes: 2 additions & 0 deletions charts/langsmith/.helmignore
Original file line number Diff line number Diff line change
Expand Up @@ -21,3 +21,5 @@
.idea/
*.tmproj
.vscode/
# Helm Unittest
tests/
2 changes: 2 additions & 0 deletions charts/langsmith/templates/_helpers.tpl
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,8 @@ Template containing common environment variables that are used by several servic
secretKeyRef:
name: {{ include "langsmith.redisSecretsName" . }}
key: connection_url
- name: CLICKHOUSE_HYBRID
value: {{ .Values.clickhouse.external.hybrid | quote }}
- name: CLICKHOUSE_DB
valueFrom:
secretKeyRef:
Expand Down
46 changes: 46 additions & 0 deletions charts/langsmith/tests/langsmith_hybrid_test.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
suite: test hybrid environment variables
values:
- ./values/blob-storage-defaults-enabled.yaml
templates:
- backend/clickhouse-migrations.yaml
- backend/deployment.yaml
- backend/postgres-migrations.yaml
- platform-backend/deployment.yaml
- queue/deployment.yaml
tests:
- it: should not have hybrid overrides set
asserts:
- contains:
path: spec.template.spec.containers[0].env
content:
name: CLICKHOUSE_HYBRID
value: "false"
- contains:
path: spec.template.spec.containers[0].env
content:
name: MIN_BLOB_STORAGE_SIZE_KB
value: "20"
- contains:
path: spec.template.spec.containers[0].env
content:
name: FF_CH_SEARCH_ENABLED
value: "true"
- it: should have hybrid overrides set
set:
clickhouse.external.hybrid: true
asserts:
- contains:
path: spec.template.spec.containers[0].env
content:
name: CLICKHOUSE_HYBRID
value: "true"
- contains:
path: spec.template.spec.containers[0].env
content:
name: MIN_BLOB_STORAGE_SIZE_KB
value: "0"
- contains:
path: spec.template.spec.containers[0].env
content:
name: FF_CH_SEARCH_ENABLED
value: "false"
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
config:
blobStorage:
enabled: true
6 changes: 3 additions & 3 deletions charts/langsmith/values.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -113,10 +113,10 @@ config:
engine: "S3"
# If you are using langsmith-managed-clickhouse, you may not want inputs to be stored in clickhouse for search.
# Set this as false to ensure that inputs/outputs/errors are not stored in clickhouse.
# hosting: "hybrid" overrides this to false.
# 'clickhouse.external.hybrid: true' overrides this to false.
chSearchEnabled: true
# Set this to change the threshold for payloads to be stored in blob storage
# hosting: "hybrid" overrides this to 0
# 'clickhouse.external.hybrid: true' overrides this to 0
minBlobStorageSizeKb: "20"
# If you are using workload identity, you may not need to store the S3 credentials in the secrets.
# Instead, you will need to add the workload identity annotation to the backend and queue service accounts.
Expand Down Expand Up @@ -396,7 +396,7 @@ clickhouse:
# If enabled, use the following values to connect to an external database. This will also disable the
# creation of a clickhouse stateful-set and service.
enabled: false
# -- Set to true if using managed ClickHouse
# -- Must be set to true if using managed ClickHouse
hybrid: false
host: ""
port: "8123"
Expand Down