tryretool / retool-helm

MIT License
45 stars 57 forks source link

Support Istio Clusters #144

Open alexlo03 opened 6 months ago

alexlo03 commented 6 months ago

Hello,

We are able to use version: 5.0.10 in our Istio cluster, but not 6.0.13 because the chart renders things that do not pass our Istio checks.

Error: 3 errors occurred:
    * Service/my-retool/my-retool-workflow-backend: service "my-retool-workflow-backend/my-retool/:" has an unnamed port. This is not recommended, See https://istio.io/v1.19/docs/ops/deployment/requirements/
    * Service/my-retool/my-retool-workflow-worker: service "my-retool-workflow-worker/my-retool/:" port "retool" does not follow the Istio naming convention. See https://istio.io/v1.19/docs/ops/deployment/requirements/
    * Service/my-retool/my-retool-workflow-worker: service "my-retool-workflow-worker/my-retool/:" port "metrics" does not follow the Istio naming convention. See https://istio.io/v1.19/docs/ops/deployment/requirements/

Error 1

port is not named - issue here: https://github.com/tryretool/retool-helm/blob/main/charts/retool/templates/deployment_workflows.yaml#L310

Possible fix: Can a configurable port name be added?

The render:

---
# Source: retool/templates/deployment_workflows.yaml
apiVersion: v1
kind: Service
metadata:
  name: my-retool-workflow-backend
  namespace: my-retool
spec:
  selector:
    retoolService: my-retool-workflow-backend
  ports:
    - protocol: TCP
      port: 80
      targetPort: 3000
---

Error 2 and 3

port names are unable to follow convention

https://github.com/tryretool/retool-helm/blob/main/charts/retool/templates/deployment_workflows_worker.yaml#L234-L239

Possible fix: Can these port names be configurable?

The render:

---
# Source: retool/templates/deployment_workflows_worker.yaml
apiVersion: v1
kind: Service
metadata:
  name: my-retool-workflow-worker
  namespace: my-retool
spec:
  selector:
    retoolService: my-retool-workflow-worker
  ports:
    - protocol: TCP
      port: 3005
      targetPort: 3005
      name: retool
    - protocol: TCP
      port: 9090
      targetPort: metrics
      name: metrics
---

My proposed fixes above should not be breaking changes.

Thanks

alexlo03 commented 6 months ago

Note that chart v5.0.10 works for us which we will continue to use for the time being.