Skip to content
This repository has been archived by the owner on Jan 19, 2024. It is now read-only.

Support additional volumes inside the containers #54

Conversation

gwvandesteeg
Copy link
Contributor

Add ability to specify additional volumes inside the containers so that
they can be utilised and mounted at various locations inside the pod as
needed.

This will need testing and verification.

Add ability to specify additional volumes inside the containers so that
they can be utilised and mounted at various locations inside the pod as
needed.
@gwvandesteeg gwvandesteeg marked this pull request as ready for review July 15, 2020 03:43
@moxious moxious merged commit 5feff7a into neo4j-contrib:master Jul 15, 2020
@JnMik
Copy link

JnMik commented Jul 30, 2020

Hello guys @gwvandesteeg @moxious

How do you exactly use this ?

I am trying to use this in the values.yml


  ## specify additional volumes to mount in the core container, this can be used
  ## to specify additional storage of material or to inject files from ConfigMaps
  ## into the running container
  additionalVolumes:
  - name: restore-service-key
    secret:
      secretName: neo4j-cronjob-backup
      defaultMode: 0555
  - name: restore-script
    configMap:
      name: "neo4j-cronjob-script"
      defaultMode: 0777
  - name: "neo4j-tls"
    secret:
      secretName: "neo4j-tls"

  ## specify where the additional volumes are mounted in the core container
  additionalVolumeMounts:
  - name: "neo4j-tls"
    mountPath: "/certs/bolt/public.crt"
    subPath: "tls.crt"
    readOnly: true
  - name: "neo4j-tls"
    mountPath: "/certs/bolt/private.key"
    subPath: "tls.key"
    readOnly: true

But keep getting these errors.
coalesce.go:196: warning: cannot overwrite table with non table for additionalVolumeMounts (map[])
coalesce.go:196: warning: cannot overwrite table with non table for additionalVolumes (map[])

Should the properties be written with [] instead of {} ? Like this

  ## specify additional volumes to mount in the core container, this can be used
  ## to specify additional storage of material or to inject files from ConfigMaps
  ## into the running container
  additionalVolumes: []

  ## specify where the additional volumes are mounted in the core container
  additionalVolumeMounts: []

Edited: Seems changing the curly for square brackets in the values.yml fix my issues actually, but I don't know if you have another way to use it that makes more sens.

@moxious
Copy link
Contributor

moxious commented Jul 30, 2020

It's possible that the helm template expansion is mucking something up. Can you please use helm template instead of helm install and paste the part of your expanded YAML that is getting applied? This would let us see what's wrong with your volumes and volume mounts that's going wrong

@JnMik
Copy link

JnMik commented Jul 30, 2020

Just so you know, helm template is triggering the warning too

coalesce.go:196: warning: cannot overwrite table with non table for additionalVolumeMounts (map[])
coalesce.go:196: warning: cannot overwrite table with non table for additionalVolumes (map[])

Some extract from the helm template command :

# Source: neo4j/templates/core-statefulset.yaml
apiVersion: "apps/v1"
kind: StatefulSet
metadata:
  name: "neo4j-v1-neo4j-core"
[...]
        - name: init-script
          mountPath: /helm-init
        - name: datadir
          mountPath: "/data"
        - name: plugins
          mountPath: /plugins
        - mountPath: /certs/bolt/public.crt
          name: neo4j-tls
          readOnly: true
          subPath: tls.crt
        - mountPath: /certs/bolt/private.key
          name: neo4j-tls
          readOnly: true
          subPath: tls.key
[...]
      volumes:
        - name: init-script
          configMap: 
            name: "neo4j-v1-init-script"
        - name: plugins
          emptyDir: {}
        - name: restore-service-key
          secret:
            defaultMode: 365
            secretName: neo4j-cronjob-backup
        - configMap:
            defaultMode: 511
            name: neo4j-cronjob-script
          name: restore-script
        - name: neo4j-tls
          secret:
            secretName: neo4j-tls
  [...]

It seems like the values are there ? Is that what you expected ?

@JnMik
Copy link

JnMik commented Jul 30, 2020

I run this just before my helm install / upgrade and it'S working

sed -i 's/additionalVolumes: {}/additionalVolumes: []/g' $MYDIR/neo4j-helm/values.yaml
sed -i 's/additionalVolumeMounts: {}/additionalVolumeMounts: []/g' $MYDIR/neo4j-helm/values.yaml

@moxious
Copy link
Contributor

moxious commented Jul 30, 2020

@JnMik ok - that's helpful. Please open an issue and I'll catch this in a PR sometime next week.

@JnMik
Copy link

JnMik commented Jul 30, 2020

Thanks mate !

@gwvandesteeg gwvandesteeg deleted the feature/add-additional-volumes-to-containers branch July 31, 2020 06:13
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.

None yet

3 participants