From 36bb644a9cc7fe449edbfabdd62bd932ea34a84e Mon Sep 17 00:00:00 2001 From: Brian Vander Schaaf Date: Wed, 27 Nov 2024 14:06:13 -0500 Subject: [PATCH 1/3] add CLICKHOUSE_HYBRID env var to indicate hybrid mode --- charts/langsmith/templates/_helpers.tpl | 2 ++ charts/langsmith/values.yaml | 6 +++--- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/charts/langsmith/templates/_helpers.tpl b/charts/langsmith/templates/_helpers.tpl index d2d3ea6..460d34f 100644 --- a/charts/langsmith/templates/_helpers.tpl +++ b/charts/langsmith/templates/_helpers.tpl @@ -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: diff --git a/charts/langsmith/values.yaml b/charts/langsmith/values.yaml index 36606a7..86d8a2d 100644 --- a/charts/langsmith/values.yaml +++ b/charts/langsmith/values.yaml @@ -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. @@ -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" From bb768ff6ea538c210c51dfa27fd308debbef9b98 Mon Sep 17 00:00:00 2001 From: Brian Vander Schaaf Date: Wed, 27 Nov 2024 14:49:38 -0500 Subject: [PATCH 2/3] run in ci --- .github/workflows/helm_checks.yaml | 6 +++ charts/langsmith/.helmignore | 2 + .../tests/langsmith_hybrid_test.yaml | 46 +++++++++++++++++++ .../values/blob-storage-defaults-enabled.yaml | 3 ++ 4 files changed, 57 insertions(+) create mode 100644 charts/langsmith/tests/langsmith_hybrid_test.yaml create mode 100644 charts/langsmith/tests/values/blob-storage-defaults-enabled.yaml diff --git a/.github/workflows/helm_checks.yaml b/.github/workflows/helm_checks.yaml index 95edd06..f14c322 100644 --- a/.github/workflows/helm_checks.yaml +++ b/.github/workflows/helm_checks.yaml @@ -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@0.7.0 + + - name: Run chart unittest + run: helm unittest charts/${{ matrix.chart-name }} + - name: Create kind cluster uses: helm/kind-action@v1.7.0 diff --git a/charts/langsmith/.helmignore b/charts/langsmith/.helmignore index 0e8a0eb..00644fa 100644 --- a/charts/langsmith/.helmignore +++ b/charts/langsmith/.helmignore @@ -21,3 +21,5 @@ .idea/ *.tmproj .vscode/ +# Helm Unittest +tests/ diff --git a/charts/langsmith/tests/langsmith_hybrid_test.yaml b/charts/langsmith/tests/langsmith_hybrid_test.yaml new file mode 100644 index 0000000..28b7bbb --- /dev/null +++ b/charts/langsmith/tests/langsmith_hybrid_test.yaml @@ -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" diff --git a/charts/langsmith/tests/values/blob-storage-defaults-enabled.yaml b/charts/langsmith/tests/values/blob-storage-defaults-enabled.yaml new file mode 100644 index 0000000..a0b14f6 --- /dev/null +++ b/charts/langsmith/tests/values/blob-storage-defaults-enabled.yaml @@ -0,0 +1,3 @@ +config: + blobStorage: + enabled: true From fa408f83179a3f2ca65ba67b73d0819578e70736 Mon Sep 17 00:00:00 2001 From: Brian Vander Schaaf Date: Wed, 27 Nov 2024 14:54:47 -0500 Subject: [PATCH 3/3] fix --- .github/workflows/helm_checks.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/helm_checks.yaml b/.github/workflows/helm_checks.yaml index f14c322..1230ee6 100644 --- a/.github/workflows/helm_checks.yaml +++ b/.github/workflows/helm_checks.yaml @@ -36,7 +36,7 @@ jobs: 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@0.7.0 + 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 }}