helm: ingress improvements #422

Closed
patrickjahns wants to merge 3 commits from patrickjahns/garage:helm-refactor-ingress into main
Contributor

As discussed in the chat yesterday, I want to propose to disable the ingress per default.

The motivation behind this change is, that per default the ingress is "misconfigured" meaning it can not work with the default values and requires a user of the chart to add additional configuration. When installing the chart per default, I would not expect to already expose garage publicly without my explicit configuration to do so

Commenting the ingressClass resource also allows for relying only on annotations - otherwise the ingressClass would be always set to nginx or require a user to override it with ingressClass: null

A small change on top, I've added the ability to specify user defined labels per ingress

As discussed in the chat yesterday, I want to propose to disable the ingress per default. The motivation behind this change is, that per default the ingress is "misconfigured" meaning it can not work with the default values and requires a user of the chart to add additional configuration. When installing the chart per default, I would not expect to already expose garage publicly without my explicit configuration to do so Commenting the `ingressClass` resource also allows for relying only on annotations - otherwise the ingressClass would be always set to nginx or require a user to override it with `ingressClass: null` A small change on top, I've added the ability to specify user defined labels per ingress
patrickjahns added 3 commits 2022-11-16 11:29:34 +00:00
The default values forces people to create an ingress resources,
where per default an ingress is not necessary to start garage.

If someone wants to utilize an ingress, he would need to define
the values for the ingress either way, so enabling the ingress
explicitly makes more sense, then requiring it to be disabled per default
chore(helm): bump chart number
All checks were successful
continuous-integration/drone/pr Build is passing
1dd3869199
Owner

@maximilien can you review this?

@maximilien can you review this?
maximilien self-assigned this 2022-11-18 09:21:29 +00:00
Contributor

This compiles properly and should deploy fine although I did not run end to end tests.

I agree those changes are welcome, especially diabling the default ingress class, which inteferes with many setups.

This compiles properly and should deploy fine although I did not run end to end tests. I agree those changes are welcome, especially diabling the default ingress class, which inteferes with many setups.
maximilien requested review from maximilien 2022-12-11 23:12:21 +00:00
maximilien approved these changes 2022-12-11 23:13:03 +00:00
Owner

Sorry @patrickjahns I had to 'manually' merge this PR because I tried to rebase it though the interface and Gitea got itself stuck on the rebase hook, because of a somewhat cryptic incompatibility issue between musl/alpine and docker (see here the notes about faccessat2). The commits retained your name, but the end result ended up under mine. Please acccepts my appologies for that, next time I'll remember to use Co-authored-by:...

Sorry @patrickjahns I had to 'manually' merge this PR because I tried to rebase it though the interface and Gitea got itself stuck on the rebase hook, because of a somewhat cryptic incompatibility issue between musl/alpine and docker (see [here](https://wiki.alpinelinux.org/wiki/Release_Notes_for_Alpine_3.14.0) the notes about `faccessat2`). The commits retained your name, but [the end result](https://git.deuxfleurs.fr/Deuxfleurs/garage/commit/980572a8872c56ea9572ff03579ebb9a65013775) ended up under mine. Please acccepts my appologies for that, next time I'll remember to use `Co-authored-by:`...
maximilien closed this pull request 2022-12-12 00:04:00 +00:00
Some checks are pending
continuous-integration/drone/pr Build is passing
ci/woodpecker/pr/debug
Required

Pull request closed

Sign in to join this conversation.
No reviewers
No milestone
No project
No assignees
4 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: Deuxfleurs/garage#422
No description provided.