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

Added init container to fix permissions #28

Merged
merged 1 commit into from
Jan 12, 2022
Merged

Added init container to fix permissions #28

merged 1 commit into from
Jan 12, 2022

Conversation

Langhalsdino
Copy link
Contributor

This pull request added an init container to the deployment.yml.
The #22 shows that this is a bug, when using persistent volume claims, since their permission is incorrect and need to be changed.

Try the fix

helm repo add node-red https://schwarzit.github.io/node-red-chart/
helm repo update
helm upgrade --install node-red node-red/node-red -f values.yml --namespace dev --version 0.3.2

While the values.yml is defined as follows:

replicaCount: 1

env:
  - name: "TZ"
    value: "UTC"
  - name: "NODE_RED_ENABLE_SAFE_MODE"
    value: "false"
  - name: "NODE_RED_ENABLE_PROJECTS"
    value: "false"
  - name: "FLOWS"
    value: "flows.json"

persistence:
  enabled: true
  accessMode: ReadWriteOnce
  size: 10Gi

ingress:
  enabled: false

resources:
  limits:
    cpu: 100m
    memory: 256Mi

service:
  type: NodePort
  port: 1880
kubectl port-forward service/node-red 1880:1880 --namespace dev

Copy link
Collaborator

@dirien dirien left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi. @Langhalsdino,

thanks for the contribution! I had this on my todo list!

Can you do me a favour and add feat: Added init container to fix permissions to the commit message?

And sign it off to.

Easy to do with:

git rebase -i HEAD~2
<chose reword>
git commit --amend -s
git push origin --force
```

All in your branch! Thanks, then I can merge it and release it instantly! 

Thanks for the work!

Copy link
Collaborator

@dirien dirien left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Left some comments

charts/node-red/Chart.yaml Outdated Show resolved Hide resolved
charts/node-red/templates/deployment.yaml Outdated Show resolved Hide resolved
charts/node-red/templates/deployment.yaml Show resolved Hide resolved
@Langhalsdino
Copy link
Contributor Author

Thank you for your comments, i tried to integrate everything and hopefully succeed rebasing the comments and choosing a useful commit message.

@Langhalsdino
Copy link
Contributor Author

I squashed the commits, sorry for the inconvenience with the multiple commits

Copy link
Collaborator

@dirien dirien left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @Langhalsdino, thanks for the changes!

I left one little thing to change, and then we are good to merge!

charts/node-red/templates/deployment.yaml Outdated Show resolved Hide resolved
updated chart version and modified change log

feat: Added init container to fix permissions

Signed-off-by: Langhalsdino <github@tausch.me>
Copy link
Collaborator

@dirien dirien left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @Langhalsdino,

thanks for your contribution! Much appreciated!

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

Successfully merging this pull request may close these issues.

None yet

2 participants