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

Feature request: Add support for Redis with persistent memory #111

Closed
marcemq opened this issue Feb 11, 2019 · 13 comments
Closed

Feature request: Add support for Redis with persistent memory #111

marcemq opened this issue Feb 11, 2019 · 13 comments

Comments

@marcemq
Copy link
Contributor

marcemq commented Feb 11, 2019

Would be nice that the operator supports usage of persistent memory along with Optane™ DC Persistent Memory hardware too.
So, if a user have access to such hardware, she/he can ask for persistent volumes using persistent volume claims which can be provided by pmem-csi, which stands for Persistent Memory Container Storage Interface Driver

@jchanam
Copy link
Collaborator

jchanam commented Feb 12, 2019

Hi @marcemq,

Thanks for opening the issue! Currently, any PVC can be used in redis failovers. This is done because the redis has the option to add a full PVC template inside it's spec.

You can find an example here.

Does this covers the needs for using Optane?

@marcemq
Copy link
Contributor Author

marcemq commented Feb 18, 2019

Not entirely, some changes needs to be added, i.e:

  • usage of a different image that contains persistent memory usage
  • and changes for the operator to be able to talk to the pmem-csi driver

@jchanam
Copy link
Collaborator

jchanam commented Feb 19, 2019

Correct me please if I'm wrong. After reading the documentation, I see that the following has to be done:

  • Enable pmem:
    • Mark the nodes as supported for using pmem
    • Deploy the driver to K8S
    • Define a storageClass
  • Use pmem:
    • Define a PVC with pmem storageClass
    • Use that PVC

In the case of RedisOperator, all the part of enabling pmem I think is out of its scope. That has to be done by the owners of each cluster (the same way as enabling AWS EFS storage, for example).

When the preparation part is done, a PVC of the type pmem can be used. This is fully supported, as the Storage of Redis can use directly the persistentVolumeClaim template.

About the image prepared, it's also a variable, so you can use any redis image that you need to. For example this one: https://github.com/pmem/pmem-redis

Where is an example of a redisFailover CR that would potentially use the pmem if I understood correctly:

apiVersion: storage.spotahome.com/v1alpha2
kind: RedisFailover
metadata:
  name: redisfailover-pmem
spec:
  sentinel:
    replicas: 3
  redis:
    replicas: 3
    image: pmem-redis # From https://github.com/pmem/pmem-redis (not actual available public docker image, needs to be built and uploaded to a repository)
    version: latest # Or any version of the image, depending on how our image is tagged
    storage:
      persistentVolumeClaim: # Taken from https://github.com/intel/pmem-CSI/blob/devel/deploy/kubernetes-1.11/pmem-pvc.yaml
        metadata:
          name: redisfailover-pmem-data
        spec:
          resources:
            requests:
              storage: 8Gi
          storageClassName: pmem-csi-sc

Does the operator need to talk directly to the pmem-csi? Why is that needed?

@marcemq
Copy link
Contributor Author

marcemq commented Feb 23, 2019

Sorry for the statement that I've used, which is confusing 🙊

changes for the operator to be able to talk to the pmem-csi driver

I wanted to meant that some changes needs to be made in order to use the pmem-csi-sc storage class, i.e: pmdir and protected-mode extra configuration when launching redis and sentinel instances.

I've took the liberty to send a PR for it and tested with two provisioners, and worked just fine 😊 🎉

@jchanam
Copy link
Collaborator

jchanam commented Feb 25, 2019

Hi @marcemq,

After reading the PR, I think that this configuration can be all done via customConfig. Could you try to do it that way? What are the problems that you face using the generic way?

I think it's better to work to add the possibility to make the operator more configurable rather than add code for a very specific use cases.

@marcemq
Copy link
Contributor Author

marcemq commented Feb 26, 2019

Hi there 😊

I've manage to test having protected-mode no via customConfig and works as expected.
But, was not the same for pmemDirParam and I've updated the PR accordingly.
And I've tested using:

  • Test a
customConfig:
     - "protected-mode no"
     - "pmdir /pmem 100Mi"
  • Test b
customConfig:
      - "protected-mode no"
      - "pmdir /data/pmem 100Mi"     #Since /data is always the volume to be mounted.

From logs:

$ kubectl get po
NAME                                           READY   STATUS             RESTARTS   AGE
pmem-csi-controller-0                          4/4     Running            0          126m
pmem-csi-tqprr                                 2/2     Running            0          126m
redisfailover-redisoperator-85f6f54988-rtfrp   1/1     Running            9          108m
rfr-redisfailover-pmem-0                       0/1     CrashLoopBackOff   8          18m
rfs-redisfailover-pmem-85f8764db-p6snc         1/1     Running            0          18m
rfs-redisfailover-pmem-85f8764db-zf5pk         1/1     Running            0          18m
rfs-redisfailover-pmem-85f8764db-zz7zm         1/1     Running            0          18m

## Event section from rfr-redisfailover-pmem-0
Events:
  Type     Reason                  Age                    From                              Message
  ----     ------                  ----                   ----                              -------
  Warning  FailedScheduling        6m34s (x5 over 6m34s)  default-scheduler                 pod has unbound immediate PersistentVolumeClaims
  Normal   Scheduled               6m34s                  default-scheduler                 Successfully assigned default/rfr-redisfailover-pmem-0 to k8s-ubuntu-pmem-worker1
  Normal   SuccessfulAttachVolume  6m33s                  attachdetach-controller           AttachVolume.Attach succeeded for volume "pvc-3135de2f-3a15-11e9-b18b-0242ac110002"
  Normal   Pulling                 5m41s (x4 over 6m24s)  kubelet, k8s-ubuntu-pmem-worker1  pulling image "hub.docker.intel.com/redis_pmem/redis:latest"
  Normal   Pulled                  5m41s (x4 over 6m23s)  kubelet, k8s-ubuntu-pmem-worker1  Successfully pulled image "hub.docker.intel.com/redis_pmem/redis:latest"
  Normal   Created                 5m40s (x4 over 6m23s)  kubelet, k8s-ubuntu-pmem-worker1  Created container
  Normal   Started                 5m40s (x4 over 6m23s)  kubelet, k8s-ubuntu-pmem-worker1  Started container
  Warning  BackOff                 80s (x31 over 6m20s)   kubelet, k8s-ubuntu-pmem-worker1  Back-off restarting failed container

## logs for rfr-redisfailover-pmem-0
$ kubectl logs rfr-redisfailover-pmem-0
8:C 26 Feb 22:45:04.715 # oO0OoO0OoO0Oo Redis is starting oO0OoO0OoO0Oo
8:C 26 Feb 22:45:04.715 # Redis version=4.0.8, bits=64, commit=2075a0b3, modified=1, pid=8, just started
8:C 26 Feb 22:45:04.715 # Configuration loaded
/usr/local/bin/redis-server: line 6: /redis/whatwascalled2: Read-only file system
Please specify the location for memkind allocations with given size.
Example: ./redis-server --pmdir /mnt/pmem/ 1g

## But the pvc get successfully attached:
$ kubectl get pvc,pv
NAME                                                                     STATUS   VOLUME                                     CAPACITY   ACCESS MODES   STORAGECLASS   AGE
persistentvolumeclaim/redisfailover-pmem-data-rfr-redisfailover-pmem-0   Bound    pvc-3135de2f-3a15-11e9-b18b-0242ac110002   100Mi      RWO            pmem-csi-sc    19m

NAME                                                        CAPACITY   ACCESS MODES   RECLAIM POLICY   STATUS   CLAIM                                                      STORAGECLASS   REASON   AGE
persistentvolume/pvc-3135de2f-3a15-11e9-b18b-0242ac110002   100Mi      RWO            Delete           Bound    default/redisfailover-pmem-data-rfr-redisfailover-pmem-0   pmem-csi-sc             19m

## From worker bash
ubuntu@k8s-ubuntu-pmem-worker1:~$ sudo lsblk
sudo: unable to resolve host k8s-ubuntu-pmem-worker1: Resource temporarily unavailable
NAME    MAJ:MIN RM  SIZE RO TYPE MOUNTPOINT
vda     252:0    0   50G  0 disk
├─vda1  252:1    0 49.9G  0 part /
├─vda14 252:14   0    4M  0 part
└─vda15 252:15   0  106M  0 part /boot/efi
vdb     252:16   0  374K  0 disk
pmem0   259:0    0    2G  0 disk
└─ndbus0region0fsdax-317b6adb--3a15--11e9--94bb--0a580af40111
        253:0    0  100M  0 lvm  /var/lib/kubelet/pods/31398685-3a15-11e9-b18b-0242ac110002/volumes/kubernetes.io~csi/pvc-3135de2f-3a15-11e9-b18b-0242

I can erase adding the pmemDirParam from code but I think we need to add a flexibility to mount a different path. What do you think?

@jchanam
Copy link
Collaborator

jchanam commented Feb 27, 2019

Hi @marcemq,

It looks it's a problem with the options passed to the redis-server:

/usr/local/bin/redis-server: line 6: /redis/whatwascalled2: Read-only file system
Please specify the location for memkind allocations with given size.
Example: ./redis-server --pmdir /mnt/pmem/ 1g

For me, instead of addid specific configuration for pmem, I'd rather add the possibility to configure the commands passed to the redises. What do you think?

@marcemq
Copy link
Contributor Author

marcemq commented Feb 28, 2019

I understand that redis-server needs the pmdir command with its proper parameters, which typically gets read from redis.conf when launching redis instances, that's why I went deeper into how the customConfig gets set for the redises (see issue124).

I have queries from the conversation in these both issues:

  • setting the pmdir /path memSize into customConfig section will never be written into redis.conf which is the expected behaviour. Would it cause any harm if we send a CONFIG REWRITE? If not, I think by adding it we can have a redis-pmem deployment successfully.
  • What do you mean by add the possibility to configure the commands passed to the redises? Are you referring to set/execute this option within the containers after an attempt of redis cluster deployment, which will end up being a manual step? Or is it an enhancement for pmem-csi driver? Or is it an enhancement for the operator? If so the PR19 can be used then?

What will be the best approach?

@marcemq
Copy link
Contributor Author

marcemq commented Mar 4, 2019

Answering my own questions 🙊😊

  • Would it cause any harm if we send a CONFIG REWRITE?
    Ans: I think, it won't but it would not be enough to have a redis-pmem deployment successfully because it needs to have a redis instance already running, and I haven't being have to have it yet.

  • I think I get what you mean by configure the commands passed to the redises, and I've tried to add the next into my yaml deployment under redis block, but it didn't work:

 command: ["redis-server"]
 args: ["--pmdir", "/data", "100Mi"]

Also, I've tried to add this command by kubectl edit pod rfr-redisfailover-pmem-0 but it seems that such operation is not allowed, which make sense for me.

So, do you know how to pass this command?

@marcemq
Copy link
Contributor Author

marcemq commented Mar 4, 2019

As a resume:

  • I need to pass the command redis-server --pmdir /path <storage> when launching the redises.

We have two options:

  1. add it in the default configuration file, such changes will be similar to these lines L120-L129.
  2. add the possibility to pass initCustomCommand trough yaml deployment, having these steps by default.

@jchanam what do you think? Do you know any other way to pass this command?

@jchanam
Copy link
Collaborator

jchanam commented Mar 5, 2019

Hi @marcemq,

For me, the goal of changes should be to create the most open option, so it's generic and not attached to a specific product or service. That's why, for me it'd be better to make the commands configurable.

@jchanam
Copy link
Collaborator

jchanam commented Mar 5, 2019

@marcemq, I've created a branch with the command configurable for both redis and sentinel: https://github.com/spotahome/redis-operator/tree/make-command-configurable

You can test the operator with this image: quay.io/spotahome/redis-operator:make-command-configurable

Does this cover your needs?

@marcemq
Copy link
Contributor Author

marcemq commented Mar 5, 2019

@jchanam I've tested your branch make-command-configurable along with the deployment below and it worked!! ☺️ 🎉

Are you planning to merge your branch to master? If so, I can send a PR with the deployment used as an example, what do you say?


apiVersion: storage.spotahome.com/v1alpha2
kind: RedisFailover
metadata:
  name: redisfailover-pmem
spec:
  sentinel:
    replicas: 3
    command: ["redis-server", "/redis/sentinel.conf", "--sentinel", "--protected-mode", "no"]
  redis:
    replicas: 3
    image: redis_pmem     #From https://github.com/pmem/pmem-redis
    version: latest
    command: ["redis-server", "/redis/redis.conf", "--pmdir",  "/data 100Mb", "--protected-mode", "no"]
    storage:
      persistentVolumeClaim:
        metadata:
          name: redisfailover-pmem-data
        spec:
          accessModes:
          - ReadWriteOnce
          resources:
            requests:
              storage: 100Mi
          storageClassName: pmem-csi-sc

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants