Skip to content

Commit 158f116

Browse files
authored
Remove eck-operator hard dependency for non-ES storage backends (#181)
## Summary - Removed the standalone `eckOperator.enabled` flag. The eck-operator chart dependency now shares the `elasticsearch.enabled` condition, so it is only deployed when Elasticsearch is in use. - BanyanDB and PostgreSQL users no longer need to explicitly disable the ECK operator — setting `elasticsearch.enabled=false` is sufficient. - Pinned `docker/login-action` to the Apache-approved SHA (v4.0.0) in CI workflows. - Updated the ES e2e test to pre-install ECK CRDs separately (required because Helm validates CRs against the API server before applying).
1 parent ef1ca5b commit 158f116

File tree

9 files changed

+31
-29
lines changed

9 files changed

+31
-29
lines changed

.github/workflows/e2e.ci.yaml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ jobs:
5555
steps:
5656
- uses: actions/checkout@v2
5757
- name: Login to ghcr
58-
uses: docker/login-action@v1
58+
uses: docker/login-action@b45d80f862d83dbcd57f89517bcf500b2ab88fb2 # v4.0.0
5959
with:
6060
registry: ghcr.io
6161
username: ${{ github.repository_owner }}

.github/workflows/publish-helm.yaml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ jobs:
4545
steps:
4646
- uses: actions/checkout@v3
4747
- name: Log in to the Container registry
48-
uses: docker/login-action@v1.10.0
48+
uses: docker/login-action@b45d80f862d83dbcd57f89517bcf500b2ab88fb2 # v4.0.0
4949
with:
5050
registry: ${{ env.HUB }}
5151
username: ${{ github.actor }}

CLAUDE.md

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ test/e2e/ # E2E test configs (skywalking-infra-e2e format)
3232
## Chart Dependencies
3333

3434
Defined in `chart/skywalking/Chart.yaml`:
35-
- **eck-operator** (3.3.1) — ECK operator, condition: `eckOperator.enabled`
35+
- **eck-operator** (3.3.1) — ECK operator, condition: `elasticsearch.enabled`
3636
- **eck-elasticsearch** (0.18.1, alias: `elasticsearch`) — ECK-managed ES, condition: `elasticsearch.enabled`
3737
- **postgresql** (12.1.2) — Bitnami PostgreSQL, condition: `postgresql.enabled`
3838
- **skywalking-banyandb-helm** (alias: `banyandb`) — BanyanDB, condition: `banyandb.enabled`
@@ -74,8 +74,7 @@ helm template test chart/skywalking \
7474
--set oap.image.tag=10.3.0 \
7575
--set oap.storageType=elasticsearch \
7676
--set ui.image.tag=10.3.0 \
77-
--set elasticsearch.enabled=false \
78-
--set eckOperator.enabled=false
77+
--set elasticsearch.enabled=false
7978

8079
# Package chart
8180
make package
@@ -104,6 +103,11 @@ When modifying chart configuration, update all of:
104103
4. `chart/skywalking/values-my-es.yaml` — external ES example (if ES-related)
105104
5. `test/e2e/values.yaml` — test overrides (if defaults change)
106105

106+
## GitHub Actions Allow List
107+
108+
Apache enforces an allow list for third-party GitHub Actions. All third-party actions must be pinned to an approved SHA from:
109+
https://github.com/apache/infrastructure-actions/blob/main/approved_patterns.yml
110+
107111
## Git Workflow
108112

109113
- **Do not push directly to master.** Always create a feature branch and open a PR.

README.md

Lines changed: 14 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,6 @@ helm install "${SKYWALKING_RELEASE_NAME}" \
5757
--set oap.image.tag=10.3.0 \
5858
--set oap.storageType=banyandb \
5959
--set ui.image.tag=10.3.0 \
60-
--set eckOperator.enabled=false \
6160
--set elasticsearch.enabled=false \
6261
--set banyandb.enabled=true \
6362
--set banyandb.image.tag=0.9.0
@@ -136,19 +135,25 @@ here are some examples.
136135
helm install "${SKYWALKING_RELEASE_NAME}" ${REPO}/skywalking -n "${SKYWALKING_RELEASE_NAMESPACE}" \
137136
--set oap.image.tag=10.3.0 \
138137
--set oap.storageType=elasticsearch \
139-
--set ui.image.tag=10.3.0
138+
--set ui.image.tag=10.3.0 \
139+
--set eck-operator.installCRDs=false
140+
```
141+
142+
Elasticsearch is deployed via [ECK (Elastic Cloud on Kubernetes)](https://github.com/elastic/cloud-on-k8s).
143+
When `elasticsearch.enabled=true` (the default), the chart deploys both the ECK operator and an Elasticsearch 8.18.8 cluster.
144+
Because Elasticsearch CRDs must exist before the chart can be installed, you need to install them first:
145+
146+
```shell
147+
helm dep up chart/skywalking
148+
tar xzf chart/skywalking/charts/eck-operator-3.3.1.tgz -C /tmp eck-operator/charts/eck-operator-crds
149+
helm install eck-crds /tmp/eck-operator/charts/eck-operator-crds -n "${SKYWALKING_RELEASE_NAMESPACE}" --create-namespace
140150
```
141151

142-
Elasticsearch is now deployed via [ECK (Elastic Cloud on Kubernetes)](https://github.com/elastic/cloud-on-k8s).
143-
By default, the chart deploys the ECK operator and an Elasticsearch 8.18.8 cluster.
144-
If you already have the ECK operator installed, set `eckOperator.enabled=false`.
152+
Then install the chart with `--set eck-operator.installCRDs=false` to avoid duplicating the CRDs.
145153

146-
To use an existing external Elasticsearch instead, disable the embedded deployment:
154+
To use an existing external Elasticsearch instead, disable the embedded deployment (no CRD pre-install needed):
147155

148156
```yaml
149-
eckOperator:
150-
enabled: false
151-
152157
elasticsearch:
153158
enabled: false
154159
config:

chart/skywalking/Chart.yaml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ dependencies:
3333
- name: eck-operator
3434
version: 3.3.1
3535
repository: https://helm.elastic.co/
36-
condition: eckOperator.enabled
36+
condition: elasticsearch.enabled
3737
- name: eck-elasticsearch
3838
alias: elasticsearch
3939
version: 0.18.1

chart/skywalking/README.md

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -118,14 +118,14 @@ The following table lists the configurable parameters of the Skywalking chart an
118118
### Elasticsearch (ECK)
119119

120120
Elasticsearch is deployed via [ECK (Elastic Cloud on Kubernetes)](https://github.com/elastic/cloud-on-k8s).
121-
The chart includes the ECK operator and an `eck-elasticsearch` subchart. Set `eckOperator.enabled=false` if the ECK operator is already installed in your cluster.
121+
The chart includes the ECK operator and an `eck-elasticsearch` subchart, both controlled by `elasticsearch.enabled`.
122+
Because Elasticsearch CRDs must exist before the ES custom resource can be created, the ECK operator CRDs need to be installed separately before deploying the chart. See the main [README](../../README.md) for installation steps.
122123

123124
#### Top-level parameters
124125

125126
| Parameter | Description | Default |
126127
|---|---|---|
127-
| `eckOperator.enabled` | Deploy the ECK operator | `true` |
128-
| `elasticsearch.enabled` | Deploy an ECK-managed Elasticsearch cluster | `true` |
128+
| `elasticsearch.enabled` | Deploy the ECK operator and an ECK-managed Elasticsearch cluster | `true` |
129129
| `elasticsearch.version` | Elasticsearch version to deploy | `8.18.8` |
130130
| `elasticsearch.fullnameOverride` | Override the Elasticsearch resource name. The ECK service will be `{name}-es-http` | `""` |
131131
| `elasticsearch.labels` | Labels applied to the Elasticsearch resource | `{}` |

chart/skywalking/values-my-es.yaml

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -26,9 +26,6 @@ ui:
2626
image:
2727
tag: 10.0.0
2828

29-
eckOperator:
30-
enabled: false
31-
3229
elasticsearch:
3330
enabled: false
3431
config: # For users of an existing elasticsearch cluster, takes effect when `elasticsearch.enabled` is false

chart/skywalking/values.yaml

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -194,12 +194,8 @@ oapInit:
194194
extraPodLabels: {}
195195
# sidecar.istio.io/inject: false
196196

197-
# ECK Operator settings
198-
# Set eckOperator.enabled to false if the ECK operator is already installed in your cluster
199-
eckOperator:
200-
enabled: true
201-
202197
# Elasticsearch managed by ECK (eck-elasticsearch chart)
198+
# When enabled, the ECK operator is also installed as a dependency.
203199
# ref: https://github.com/elastic/cloud-on-k8s
204200
elasticsearch:
205201
enabled: true

test/e2e/e2e-elasticsearch.yaml

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -43,14 +43,14 @@ setup:
4343
- name: Install ECK operator
4444
command: |
4545
helm dep up chart/skywalking
46-
helm -n istio-system install eck-operator chart/skywalking/charts/eck-operator-3.3.1.tgz \
46+
tar xzf chart/skywalking/charts/eck-operator-3.3.1.tgz -C /tmp eck-operator/charts/eck-operator-crds
47+
helm -n istio-system install eck-crds /tmp/eck-operator/charts/eck-operator-crds \
4748
--create-namespace
48-
kubectl -n istio-system rollout status --watch --timeout=120s statefulset/elastic-operator
4949
- name: Install SkyWalking
5050
command: |
5151
helm -n istio-system install skywalking chart/skywalking \
5252
--set fullnameOverride=skywalking \
53-
--set eckOperator.enabled=false \
53+
--set eck-operator.installCRDs=false \
5454
--set oap.env.SW_ENVOY_METRIC_ALS_HTTP_ANALYSIS=k8s-mesh \
5555
--set oap.env.SW_ENVOY_METRIC_ALS_TCP_ANALYSIS=k8s-mesh \
5656
--set oap.env.K8S_SERVICE_NAME_RULE='e2e::${service.metadata.name}' \

0 commit comments

Comments
 (0)