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

Add Support for Volumes (docker-compose.uffizzi.yml) #126

Open
jpthurman opened this issue Apr 6, 2022 · 29 comments · Fixed by #90
Open

Add Support for Volumes (docker-compose.uffizzi.yml) #126

jpthurman opened this issue Apr 6, 2022 · 29 comments · Fixed by #90
Assignees

Comments

@jpthurman
Copy link
Member

Add Uffizzi Compose support for volumes (short and long syntax) as define by Docker Compose here:

https://docs.docker.com/compose/compose-file/compose-file-v3/#volumes

@jpthurman
Copy link
Member Author

Now I think we want "generic ephemeral inline volumes". They're new, in beta in k8s 1.21 and will be "stable" in 1.23.
k8s feature ticket here kubernetes/enhancements#1698
Docs here (same URL as above) https://kubernetes.io/docs/concepts/storage/ephemeral-volumes/#generic-ephemeral-volumes
We should set a size limit for these volumes. Like our CPU resource limit, it should be proportional to the node's memory.

@jpthurman
Copy link
Member Author

@gadkins
Copy link
Member

gadkins commented Apr 12, 2022

@zipofar Please see this example for generic ephemeral volumes.

https://kubernetes.io/docs/concepts/storage/ephemeral-volumes/#generic-ephemeral-volumes

To support the ./healthchecks example, assume:

  • Volume type ephemeral (Generic).
  • Storage is local (Assume the volume to mount exists as a local directory in the git repo — similar to how we use config files).
  • Volume max size is 500MB

@zipofar zipofar self-assigned this Apr 18, 2022
@zipofar
Copy link
Contributor

zipofar commented Apr 20, 2022

@gadkins What do you think about this method of determining volumes:

For example user have compose file like this:

services:
  web:
    image: my_web_service

  nginx:
    image: nginx
    volumes:
      - ./images:/image_volumes

Our idea is that the user add rules like this:

x-uffizzi-volumes:
  - target_service: nginx
    source_service: web
    source_root_dir: /app

@zipofar
Copy link
Contributor

zipofar commented Apr 20, 2022

Why we should use some additional things for volumes.
There are three types of volumes:

  1. Anonymous volumes - /var/some_folder - it's very easy
  2. Named volumes - data:/var/some_folder - this is use the persistent volumes (we don't want to support this)
  3. Bind mounts:
- /source_host_folder:/target_container_folder
- ./source_host_folder:/target_container_folder
- ~/source_host_folder:/target_container_folder

For Bind mounts case we should deliver files from host folder to k8s container folder.

Let's consider a scenario when we upload some files from a host to the our uffizzi database. Next we send these files to the uffizzi controller(it's in k8s). In this case there is no way to load these files into container, that locate in a different pod.

That's why we offer the solution above with additional tag x-uffizzi-volumes:
In this case user create the image my_web_service from his project. Add tag x-uffizzi-volumes to compose file. Into k8s we start the two containers web and nginx in single pod and share the folder /app/images between this containers.

@zipofar
Copy link
Contributor

zipofar commented Apr 25, 2022

Waiting for controller update

@zipofar
Copy link
Contributor

zipofar commented Apr 25, 2022

By now we updated design of x-uffizzi-volume-host:

x-uffizzi-volume-host:
  service: web
  root_path: /app

service - this is the name of the service that is the source of the files for volumes
root_path - absolute path for relative sources (./some_dir:/volume_dir)

Example compose file:

services:
  web:
    image: web_service:latest
    volumes:
      - /var/nginx/images
      - share_data:/cache

  nginx:
    image: nginx:latest
    volumes:
      - ./images:/var/nginx/images_2
      - ./html:/var/nginx/html

  nginx_2:
    image: nginx:latest
    volumes:
      - /app/public:/var/nginx/public

  nginx_3:
    image: nginx:latest
    volumes:
      - ~/assets:/var/nginx/assets

  nginx_4:
    image: nginx:latest
    volumes:
      - source: share_data
        target: /cache

volumes:
  share_data:

x-uffizzi-volume-host:
  service: web
  root_path: /app

@zipofar
Copy link
Contributor

zipofar commented Apr 29, 2022

@zipofar zipofar removed the waiting label May 16, 2022
@zipofar zipofar transferred this issue from UffizziCloud/uffizzi_core May 25, 2022
@zipofar
Copy link
Contributor

zipofar commented May 25, 2022

@jpthurman @axisofentropy @gadkins

After long discussion was find 2 variants how we can implement this:

1. With addition extra config item

x-uffizzi-volumes:
  service: ${main_service}
  root_path: ${path_to_project_directory_into_container} 

For example use this docker-compose
We should add:

x-uffizzi-volumes:
  service: allhands22
  root_path: /test 

root_path: /test - because Dockerfile has WORKDIR /test

Advantages:

  • There's no need to install and support another service or cloud storage

Disadvantages:

  • Need to add additional syntax to compose file
  • This not working if an image does not contains files/dirs that use in volume.

x-uffizzi

2. With addition self hosted cloud storage** (https://min.io/product/kubernetes) to kubernetes cluster.

In this case we can upload files for volumes from cli or uffizzi app

Advantages:

  • It should work for all cases

Disadvantages:

  • This solution requires install third-party service to kubernetes cluster
  • It requires some support on our side

Untitled Diagram (1)

@zipofar zipofar added the waiting label Jun 2, 2022
@axisofentropy
Copy link
Member

After long discussion was find 2 variants how we can implement this:

Thank you for composing this.

I don't think option 1 is what we're looking for.

Option 2 is good, but what's special about MinIO or other object storage? Couldn't the init container fetch the files from uffizzi_app for example? We can set a small size limit for the files, which should probably be in something like a gzipped archive.

@axisofentropy
Copy link
Member

I see that the Kompose project translates Compose volume declarations into PersistentVolumeClaims. Obviously we would need to figure out appropriate PersistentVolumes, which may be different on our open-source and proprietary platforms. But maybe the controller could create a PVC at the same time it creates the Namespace? Then it could be populated either by the controller or an initContainer?

@axisofentropy
Copy link
Member

We discussed this today and I think we're going to shrink the scope of this ticket to support only "empty" volumes. A use case for one of our customers: mounting an empty volume to /var/lib/postgresql/data so that they don't have to wait for the postgres container to reinitialize and re-seed when they push a new commit to their Pull Request.

I know we say it a lot, but we want this to behave much like Kompose. I tested it and it translated a volume in Docker Compose to a PersistentVolumeClaim. I applied this manifest to an EKS Cluster and It Just Worked; I didn't need to specify a PersistentVolume. I think this will make implementing empty volumes pretty straightforward.

Probably more specifications and protoyping coming soon, but let me know what you think!

(We will specify a new ticket soon for the other big use case of volumes that should have some files in them before preview containers start. I think Kubernetes' new "Init Containers" are the best way to implement this. Maybe they can even pull files directly from GitHub so we don't have to store them?)

@zipofar
Copy link
Contributor

zipofar commented Jun 6, 2022

@axisofentropy

Maybe they can even pull files directly from GitHub so we don't have to store them?

But for opensource version we do not use github

@moklidia
Copy link
Collaborator

moklidia commented Jun 6, 2022

The lifetime of the volume should be limited by the lifecycle of a namespace. If a pull request is closed - the volume is removed. If a commit is added to the branch - it should persist.

@moklidia
Copy link
Collaborator

moklidia commented Jun 8, 2022

  1. Create a namespace volumes-test kubectl create namespace volumes-test

  2. Create a config map

apiVersion: v1
kind: ConfigMap
metadata:
  name: postgres-config
  namespace: volumes_test
  labels:
    app: postgres
data:
  POSTGRES_DB: postgresdb
  POSTGRES_USER: admin
  POSTGRES_PASSWORD: test123
  1. Create a volume claim (volume is created and bound automatically) and specify the standard storage name
apiVersion: v1
kind: PersistentVolumeClaim
metadata:
  name: postgres-pv-claim
  namespace: volumes-test
  labels:
    app: postgres
spec:
  storageClassName: standard
  accessModes:
    - ReadWriteOnce
  resources:
    requests:
      storage: 50M
  1. Create a service
apiVersion: v1
kind: Service
metadata:
  name: postgres
  namespace: volumes-test
  labels:
    app: postgres
spec:
  type: NodePort
  ports:
   - port: 5432
  selector:
   app: postgres
  1. Create a deployment
apiVersion: apps/v1
kind: Deployment
metadata:
  name: postgres
  namespace: volumes-test
spec:
  replicas: 1
  selector:
    matchLabels:
      app: postgres
  template:
    metadata:
      labels:
        app: postgres
    spec:
      containers:
        - name: postgres
          image: postgres:10.1
          imagePullPolicy: "IfNotPresent"
          ports:
            - containerPort: 5432
          envFrom:
            - configMapRef:
                name: postgres-config
          volumeMounts:
            - mountPath: /var/lib/postgresql/data
              subPath: pgdata
              name: postgredb
      volumes:
        - name: postgredb
          persistentVolumeClaim:
            claimName: postgres-pv-claim
  1. Connect to the pod and start postgres psql -h localhost -U admin --password -p 5432 postgresdb

  2. Create tables

  3. Restart deployment kubectl rollout restart [deployment_name]

  4. Connect to the new pod: tables created in step 7 are still available

When a namespace is deleted - the PVC and PV gets deleted too

As users can have their own storage classes, the storage class should be provided in an ENV variable, standard by default

@moklidia
Copy link
Collaborator

moklidia commented Jun 8, 2022

Next points to research:

  • A pvc with the ReadWriteMany access mode hangs endlessly in pending status

A GKE persistent disk can be attached only to one node. Possible solutions are either using a CloudFilestore or setting up our own NFS server as described here

  • A PVC with a 50M request triggers a volume with 1Gi capacity

@gadkins
Copy link
Member

gadkins commented Jun 9, 2022

If more than one container in the compose mounts the same volume, then only one container should have Read + Write permissions. All others should be Read-only. In this case, the you should be required to add the ro and rw MODE syntax (https://docs.docker.com/compose/compose-file/compose-file-v3/#short-syntax-3). If they user mounts the same volume to more than one container and does not specify ro and rw, then we should return an error that explains they must designate which container has rw.

@gadkins
Copy link
Member

gadkins commented Jun 9, 2022

Please note that volumes should be supported in both open-source and closed source Uffizzi. This is different that was discussed here in Slack.

@zipofar
Copy link
Contributor

zipofar commented Jun 22, 2022

@gadkins @axisofentropy

There is one more question about anonymous volume like:

volumes:
  - /var/log

Should we implement this type volumes in this issue?
Should this type volume save state after update deployment?

@axisofentropy
Copy link
Member

Use case example: when a customer makes a deployment with postgres and seeds database. If they say /var/lib/psql is a volume, then when they push a new commit and a new Pod is created, they won't have to wait for database seeding again (it's about six minutes now for one customer.) This is a common practice for people who run relational databases in docker compose.

@gadkins
Copy link
Member

gadkins commented Jun 22, 2022

@zipofar Yes, that is the way we want people to define anonymous volumes and use them to persist state after updates to the preview deployment.

@zipofar
Copy link
Contributor

zipofar commented Jun 24, 2022

Add new StorageClass. Because was problem like this.

allowVolumeExpansion: true
apiVersion: storage.k8s.io/v1
kind: StorageClass
metadata:
  annotations:
    storageclass.kubernetes.io/is-default-class: "true"
  labels:
    addonmanager.kubernetes.io/mode: EnsureExists
  name: uffizzi-standard
parameters:
  type: pd-standard
provisioner: kubernetes.io/gce-pd
reclaimPolicy: Delete
volumeBindingMode: WaitForFirstConsumer

@zipofar
Copy link
Contributor

zipofar commented Jun 24, 2022

HOW TO TEST:

  • Create a preview
  • Create some data
  • Update preview
  • Check that data is exists

We can use this compose file for check anonymous volumes:

services:
  web:
    image: zipofar/uffizzi_test_rails_simple:latest
    volumes:
      - /db

x-uffizzi:
  ingress:
    service: web
    port: 3000

We can use this compose file for check named volumes:

services:
  web:
    image: zipofar/uffizzi_test_rails_simple:latest
    volumes:
      - db_share:/db

volumes:
  db_share:

x-uffizzi:
  ingress:
    service: web
    port: 3000

@zipofar
Copy link
Contributor

zipofar commented Jun 24, 2022

TEST REMOVE UNUSED VOLUMES:

  • create volume (for example)
services:
  web:
    image: zipofar/uffizzi_test_rails_simple:latest
    volumes:
      - share_db:/db

volumes:
  share_db:

x-uffizzi:
  ingress:
    service: web
    port: 3000
  • Add some data to DB
  • Change volume name
services:
 web:
   image: zipofar/uffizzi_test_rails_simple:latest
   volumes:
     - share_db_2:/db
volumes:
 share_db_2:

x-uffizzi:
 ingress:
   service: web
   port: 3000
  • Update preview
  • Return volume name back
services:
 web:
   image: zipofar/uffizzi_test_rails_simple:latest
   volumes:
     - share_db:/db

volumes:
 share_db:

x-uffizzi:
 ingress:
   service: web
   port: 3000
  • Update preview

Expected result: db should have init (empty) state

@zipofar
Copy link
Contributor

zipofar commented Jun 27, 2022

Long syntax for named volume:

services:
 web:
   image: zipofar/uffizzi_test_rails_simple:latest
   volumes:
     - source: share_db
       target: /db
       read_only: true
       

volumes:
 share_db:

x-uffizzi:
 ingress:
   service: web
   port: 3000

@zipofar
Copy link
Contributor

zipofar commented Jun 27, 2022

Read only for short syntax:

services:
 web:
   image: zipofar/uffizzi_test_rails_simple:latest
   volumes:
     - share_db:/db:ro
     
volumes:
 share_db:

x-uffizzi:
 ingress:
   service: web
   port: 3000
services:
 web:
   image: zipofar/uffizzi_test_rails_simple:latest
   volumes:
     - /db:ro

x-uffizzi:
 ingress:
   service: web
   port: 3000

@moklidia
Copy link
Collaborator

The test app repository: https://github.com/zipofar/test_rails

@NealArw
Copy link
Contributor

NealArw commented Jun 29, 2022

TESTED - OK

read_only: true option works only for updating previews. You can not create preview with enabled read_only option.

@NealArw
Copy link
Contributor

NealArw commented Jul 13, 2022

TESTED with GitHub controller - OK

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

Successfully merging a pull request may close this issue.

6 participants