[omada-controller] Provide useful defaults #1632
Conversation
ghost
added
size/S
|
Hi @dimakuz, thanks for the PR! Looks like you haven't updated the changelog in the |
| @@ -26,15 +26,48 @@ service: | |||
| ports: | |||
| http: | |||
| port: 8043 | |||
| management: | |||
There was a problem hiding this comment.
I don't think it really adds much value to run three separate Services for this. Could you please take a look at how the Unifi Controller chart (https://github.com/k8s-at-home/charts/blob/master/charts/stable/unifi/values.yaml#L47) handles this, and implement it in a similar way?
| management: | ||
| enabled: false | ||
| ports: | ||
| tcp29811: |
There was a problem hiding this comment.
Would it be possible to add descriptive names to these ports?
There was a problem hiding this comment.
FYI, 29811, 29812, 29813 are all device upgrade/management ports.
| # annotations: | ||
| # nginx.ingress.kubernetes.io/backend-protocol: HTTPS | ||
| # nginx.ingress.kubernetes.io/proxy-redirect-from: https://$http_host:8043/ | ||
| # nginx.ingress.kubernetes.io/proxy-redirect-to: https://$http_host/ | ||
| # nginx.ingress.kubernetes.io/configuration-snippet: | | ||
| # proxy_set_header Host $http_host:8043; |
There was a problem hiding this comment.
We try to keep our charts Ingress-agnostic.
Even though the annotations are commented out, it would be preferred if you would describe the required configuration in the README_CONFIG.md.gotmpl file.
I don't know the Omada controller well enough, but I'm also not sure all 4 of these are 100% required? I can imagine that it depends on how the rest of the Ingress (controller) is set up.
|
|
||
| # -- Configure persistence settings for the chart under this key. | ||
| # @default -- See values.yaml | ||
| persistence: | ||
| data: | ||
| enabled: false | ||
| mountPath: /opt/tplink/EAPController/data |
dimakuz
requested review from
onedr0p,
billimek,
carpenike and
truxnell
as code owners
Description of the change
Added values for:
Benefits
Simpler set up
Possible drawbacks
N/A