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

Deploy "front" as DaemonSet #46

Closed
iptizer opened this issue May 4, 2020 · 6 comments · Fixed by #67
Closed

Deploy "front" as DaemonSet #46

iptizer opened this issue May 4, 2020 · 6 comments · Fixed by #67
Labels
enhancement New feature or request

Comments

@iptizer
Copy link

iptizer commented May 4, 2020

Is your feature request related to a problem? Please describe.
Currently the front pod will be placed on some node as a deployment with replicas=1. Sure, it is possible to tune the node on which the nginx will be places, but nevertheless, it will be one node. If this node dies, nothing works anymore. Or maybe the pod is rescheduled (when using affinity), but it has no value, as the records are not set correctly.

Describe the solution you'd like
When offering the ability to deploy the front nginx as DaemonSet, the nginx will be placed on every node. Even though some things in regards to mail may not work as expected. But at least the web/admin frontend and maybe some limited mail function would work.

One example where the same approach is followed, is the nginx-ingress controller. See the helm chart where the option controller.kind offers the ability to deploy as a DaemonSet. Together with the options controller.daemonset.useHostPort and controller.hostNetwork the controller binds to the host port and therefore enables defacto LoadBalacner features.

Describe alternatives you've considered
We could of course keep it the current way, but as the nginx is stateless, I also don't see any downturns of this approach. We could also keep the default value to the current deployment way.

Additional context
I would be happy to submit a PR, but let us discuss this approach at first, before I put work into this and it is out of scope or sth like that.

@iptizer iptizer added the enhancement New feature or request label May 4, 2020
@micw
Copy link
Contributor

micw commented May 4, 2020

Funny, just changed it at user request: #28
Edit: We probably should make it configurable.

@micw
Copy link
Contributor

micw commented May 4, 2020

Seems the difference is not too big:
3f7c3fd

@micw
Copy link
Contributor

micw commented May 4, 2020

A PR is very welcome. It should:

  • introduce a variable "front.controller.kind" which can be "DaemonSet" and "Deployment"
  • values.yaml should default to "Deployment"
  • conditional switches in front.yaml (or does it make sense to have 2 yamls and support "DaemonSet" and "Deployment" at the same time?
  • Some description in mailu/README.md

@iptizer
Copy link
Author

iptizer commented May 5, 2020

A PR is very welcome. It should:

* introduce a variable "front.controller.kind" which can be "DaemonSet" and "Deployment"

* values.yaml should default to "Deployment"

* conditional switches in front.yaml (or does it make sense to have 2 yamls and support "DaemonSet" and "Deployment" at the same time?

* Some description in mailu/README.md

Sounds like a plan. That's sth for the weekend :).

Just a few additions:

  • as the frontends are connected to ingress, the DaemonSet thing will not improve these.
  • but, What are the implications & problems with the DaemonSet implementation? We should mention that in the README.

@micw
Copy link
Contributor

micw commented May 5, 2020

as the frontends are connected to ingress, the DaemonSet thing will not improve these

there's nothing to improve. But if you'd like to use nginx for http(s) ingress too, there are recently a PR that adds this feature (but in this case, no other ingress must listen on 80/443)

What are the implications & problems with the DaemonSet implementation

There are none, actually it ran fine as DaemonSet before I got the change request. DaemonSet vs Deployment are just different use cases. For a one-node deployment it does not matter. For a cloud provider (like Google or AWS) Deployment is better because TCP pods are exposed via LoadBalancer services. Up to here I thought we only need to support Deployment.
Now, with your use case, we need both.

@iptizer
Copy link
Author

iptizer commented May 6, 2020

Hmm I am not so sure. I'm thinking in the DNS direction. Because as of now replicas is not configureable. So I would set the A record of my mail domain to that server address. Ingress and Egress IPs should always be the same of this host. Everything works.

With multiple servers this gets more complicated.
Ingress mails: So when I want to make use of multiple ingress servers, all IP addresses of them will have to be set as an A record. Otherwise others won't know where to contact the server.
Egress mails: When the other service pods shift, the egress IP address may change. For SPF to work, the egress IPs need to be set as an A record in the mail domain. Do they?

This problem is not specific to the DaemonSet but to Kubernetes itself. Maybe this has already been taken care of and I just didn't stumble upon.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants