Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use nginx for kubernetes ingress #1158

Merged
merged 3 commits into from
Oct 7, 2019
Merged

Conversation

micw
Copy link
Contributor

@micw micw commented Sep 3, 2019

What type of PR?

enhancement

What does this PR do?

Currently, kubernetes uses a complex ingress setting which is not portable across different ingress controllers. This PR simplifies the ingress and delegates everythins special to Mailu to the front container,

Related issue(s)

Prerequistes

  • In case of feature or enhancement: documentation updated accordingly
  • Unless it's docs or a minor change: add [changelog]

@mergify
Copy link
Contributor

mergify bot commented Sep 3, 2019

Thanks for submitting this pull request.
Bors-ng will now build test images. When it succeeds, we will continue to review and test your PR.

bors try

Note: if this build fails, read this.

bors bot added a commit that referenced this pull request Sep 3, 2019
@mergify
Copy link
Contributor

mergify bot commented Sep 3, 2019

Thanks for submitting this pull request.
Bors-ng will now build test images. When it succeeds, we will continue to review and test your PR.

bors try

Note: if this build fails, read this.

@bors
Copy link
Contributor

bors bot commented Sep 3, 2019

try

Already running a review

@bors
Copy link
Contributor

bors bot commented Sep 3, 2019

try

Build succeeded

@micw micw mentioned this pull request Sep 3, 2019
2 tasks
@kaiyou
Copy link
Member

kaiyou commented Sep 3, 2019

This looks really good. I am setting up a testing k8s at home so hopefully I can test and provide feedback.

@micw
Copy link
Contributor Author

micw commented Sep 17, 2019

also closes #1158

@kaiyou
Copy link
Member

kaiyou commented Sep 18, 2019

Well. This is #1158 🤔

@micw
Copy link
Contributor Author

micw commented Sep 18, 2019

@kaiyou Recursion: see recursion ;-)

I meant: also closes #1128

@fionera
Copy link
Contributor

fionera commented Sep 30, 2019

I tried this but since Nginx wants to bind to :80 and the Pod is configured to use the host network, it fails when having an nginx-ingress handler...

@micw
Copy link
Contributor Author

micw commented Sep 30, 2019

@fionera Thank you for pointing this out. Indeed is "host network" a bad choice for this setup. I have changed it in my own setup but somehow forgot to put this change in the PR. I have updated the PR, you don't need to rebuild the images, just change and redeploy front.yaml

Please let me know if you have further issues. I'd like to have this PR in the master as soon as it's possible, so any testers are highly welcome ;-)

On my setup I'm running a helm chart which is work-in-progress. Once it's finished, it will be a much better way to deploy mailu on kubernetes (no need to edit deployment yaml files, clean upgrade path). This PR is one missing piece to finish it.

Best regards,
Michael.

@kaiyou
Copy link
Member

kaiyou commented Sep 30, 2019

There are so many things that need improving on the k8s side, and your PR is definitely showing the way. After crawling my way through setting the proper pvc and specifying the node affinity rule, it somehow runs fine. I am all for merging this and going forward with improving the k8s docs.

kaiyou
kaiyou previously approved these changes Sep 30, 2019
@fionera
Copy link
Contributor

fionera commented Oct 3, 2019

@micw Yeah now it nearly work out of the box, there are only some problems now... The front.yaml is missing the port 80 definition, webmail-ingress is missing and antispam returns a 502

@micw
Copy link
Contributor Author

micw commented Oct 3, 2019

@fionera
There's no webmail ingress, it's handled by front nginx. If you pass WEBMAIL env to front, it should be there (

{% if WEBMAIL != 'none' %}
)

@micw
Copy link
Contributor Author

micw commented Oct 3, 2019

No idea why antispam returns 502. Are you logged in as admin?

@micw
Copy link
Contributor Author

micw commented Oct 3, 2019

I have added the missing port in front.yml

@mergify mergify bot dismissed kaiyou’s stale review October 3, 2019 18:19

Pull request has been modified.

@fionera
Copy link
Contributor

fionera commented Oct 3, 2019

I use a different domain for the webmail to make it easier. Regarding antispam, yes I am logged it as Admin in Mailu, but as soon as I try to load the antispam I get a 502...

@kaiyou
Copy link
Member

kaiyou commented Oct 4, 2019

Webmail ingress will probably not return, we have debated this a couple times already and it makes things difficult to maintain. We could of course have different flavors but that is not the current direction: we are trying to do simple things well before we do anything complex.

@micw
Copy link
Contributor Author

micw commented Oct 4, 2019

@fionera Can you see in the 502 error if it comes from traefik or from nginx? I'm quite sure it's from nginx. If so, please check and provide the following details:

  • nginx logs when you access the rspamd
  • generated nginx configuration, especially the rspamd part
  • verify that the antispam address is correct in nginx config and it's reachable from the front container

Regarding webmail ingress: you are always free just to deploy another ingress that points to http://front/webmail/. For traefik, you can also add the traefik.ingress.kubernetes.io/app-root annotation to it, so that you get redirected from / to /webmail on the webmail domain.

@micw
Copy link
Contributor Author

micw commented Oct 7, 2019

I have it running by using my helm chart: https://github.com/micw/charts/tree/mailu/incubator/mailu
Webmail, antispam and admin-ui are reachable without any problems.

@kaiyou If you are fine with that, you can merge it. If any unexpected issues occur, I'd fix it in separate issues.

@kaiyou
Copy link
Member

kaiyou commented Oct 7, 2019

bors merge

bors bot added a commit that referenced this pull request Oct 7, 2019
1158: Use nginx for kubernetes ingress r=kaiyou a=micw

## What type of PR?

enhancement

## What does this PR do?

Currently, kubernetes uses a complex ingress setting which is not portable across different ingress controllers. This PR simplifies the ingress and delegates everythins special to Mailu to the front container,

### Related issue(s)
- closes #1121
- closes #1117
- closes #1021
- closes #1045

## Prerequistes

- [x] In case of feature or enhancement: documentation updated accordingly
- [x] Unless it's docs or a minor change: add [changelog]

Co-authored-by: Michael Wyraz <michael@wyraz.de>
@bors
Copy link
Contributor

bors bot commented Oct 7, 2019

Build succeeded

@bors bors bot merged commit b94636b into Mailu:master Oct 7, 2019
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 28, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Proxy rspamd through admin ui for security reasons Kuebernetes Admin UI URL in redirect loop
3 participants