Feature: Ingress & Service#40
Conversation
|
Nice! Could you also create the release tarball? Instructions are in the README :-) |
Here you go |
chrisjohnson00
left a comment
There was a problem hiding this comment.
A couple suggestions, plus 2 things that I think must be fixed:
- The version number doesn't follow semver, it should be 2.6.0 next, not 2.6.5
- The addition of the annotations for the ingress look like it will fail if none are provided, so a default empty set should be included in the
values.yamlfor folks who aren't using annotations.
| type: application | ||
| urls: | ||
| - https://pcktdmp.github.io/charts/fahclient-2.6.5.tgz | ||
| version: 2.6.5 |
There was a problem hiding this comment.
| version: 2.6.5 | |
| version: 2.6.0 |
And the corresponding changes elsewhere.
| @@ -0,0 +1,15 @@ | |||
| apiVersion: v1 | |||
There was a problem hiding this comment.
This whole file should be behind a feature flag, just like the ingress. In my deployment, I don't need a service, so wouldn't want one created.
| hostname: chart-example.local | ||
| path: / | ||
|
|
||
| # Enable TLS for Ingress e.g. letsencrypt using cert-manager |
There was a problem hiding this comment.
You should probably mention here that the secret name has an expected value, or add a new variable to specify it, for those not using cert-manager.
| tls: false | ||
|
|
||
| # Ingress Annotations | ||
| annotations: |
There was a problem hiding this comment.
| annotations: | |
| annotations: {} |
IIRC, the chart will fail to deploy because the value isn't provided here for annotations.
|
@Nold360 thank you for your contribution! I'm sorry for not following up accordingly after your last action. |
I'm unclear of the use case or functionality. I generally just look at log output to get a sense for what job id was assigned and it's progress. Perhaps the web ui provides a more user friendly way to see this information? |
Hi,
i implemented simple ingress and service resources & bumped the version.
Don't know how this works out with horizontal scaling enabled, tho.