search

spotahome / redis-operator

Redis Operator creates/configures/manages high availability redis with sentinel automatic failover atop Kubernetes.
Apache License 2.0
1.49k stars 356 forks source link

Add master and slave redis services #627

Closed hashemi-soroush closed 11 months ago

hashemi-soroush commented 1 year ago

Fixes #626 .

Changes proposed on the PR:

ebuildy commented 11 months ago

Good one again!

Is follow master pod if master changes?

hashemi-soroush commented 11 months ago

Good one again!

Is follow master pod if master changes?

It does. Both Master and Replica services use label selectors to select their corresponding pods. So, when the master changes, their selected pods change too. As for the clients who are using these services, since these aren't headless services, the clients wouldn't notice any changes in the IP of the master or replica pods, and as long as the services select their corresponding pods correctly, the clients won't confuse master and replicas with each other.

ebuildy commented 11 months ago

I am not sure labels change, see https://github.com/spotahome/redis-operator/issues/608

hashemi-soroush commented 11 months ago

I am not sure labels change, see #608

The functionality is definitely implemented. Nonetheless, I will try reproducing it and share the results on that issue thread.

ese commented 11 months ago

Thanks!

ese commented 10 months ago

@hashemi-soroush was it on porpose to use loadBalancer services? I think that can generate too unexpected hassle to existing clusters due creation of external load balancers in addition to security issues. Probably changing to internal services before release

hashemi-soroush commented 10 months ago

@ese there are a number of reasons I chose the loadbalancer type.

I assume the users of this operator come from two groups:

  1. Those who want to set up Redis for themselves, for example, they set up and use Redis in the same team
  2. Those who want to set up Redis for others, for example, the infrastructure teams of companies or companies that provide Redis as a service.

The first group probably starts its infrastructure with a single Kubernetes cluster, but, in my experience, quickly realises Kubernetes isn't a good fit for hosting stateful software off-the-shelf. It needs several configuration customisations which most likely makes the cluster not-well-suited for stateless software. So, it's best to have at least two clusters, one for stateful software and the other for stateless. Now, obviously you need to enable communication between the two clusters, so every database must be exposed externally one way or another. Also, to keep the database safe from the possibility of weak load balancing by its clients, which can result in the downtime of the database, we better expose it with a load balancer layer in the middle. Hence the loadbalancer type.

As for the second group of users, if it's the infrastructure team of a company, the reasoning is exactly same as above, and if it's a separate company that's providing Redis as a service in their own cloud, they have their own Kubernetes cluster and the rest of the reasoning is the same as above.

ese commented 10 months ago

All that makes sense. From my point of view if we had implemented configuration options to set up external visibility of these endpoints I had no problem with it letting users set the use case they want. Setting they to load-balancer and open externally the cluster by default its probably an issue for the users that don't want it. It would be ideal to have this possibility in the spec. Given the current behavior, I'm not comfortable forcing all users to have load balancers exposing their Redis endpoints, at the moment who needs to maintain these services always can create them by other means since they don't need any maintenance because redis operator is taking care of the pod labeling

hashemi-soroush commented 10 months ago

I understand that there are users who might not prefer the current behaviour (yet). So what exactly is your suggestion? A simple flag in the Redisfailover CRD? Something more detailed?

nastynaz commented 10 months ago

I found this repo as I was looking for a way to easily define and deploy HA redis for apps in my cluster. The automatic loadbalancer setup means the current version isn't a good fit for my use case, and I assume it might be the same for others.

It'd be great if it was off by default but opt-in via a flag for three main reasons:

  1. It's secure by default. Important when deploying redis across many namespaces/apps where it's easy to lose track which apps need to be private and which don't.
  2. It doesn't create extra resources. Depending on cloud providers load balancers can add additional cost.
  3. It doesn't accidentally leak private data. Users can't always rely on security rules to block ingress (sometimes we set allow all ingress from 0.0.0.0/0), so if a new loadbalancer is created with external access it's automatically accessible everywhere. Whilst this isn't the most secure approach, it would be beneficial if the 'enable external access' was explicit rather than implicit (i.e. external load balancers need to be opt-in)
hashemi-soroush commented 10 months ago

@nastynaz you can find out about the last updates on the issue you're referring to in issue #647 .