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

Sidecar #148

Merged
merged 1 commit into from
Oct 16, 2024
Merged

Sidecar #148

merged 1 commit into from
Oct 16, 2024

Conversation

mipieta
Copy link
Contributor

@mipieta mipieta commented Jul 8, 2024

Description

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added tests to cover my changes.
  • All new and existing tests passed.
  • I have made design choices.
  • My design choices are documented in an Architecture Decision Record in the designated ARD folder of this repo.

Issues this PR fixes

Fixes #

@mipieta mipieta force-pushed the sidecar branch 4 times, most recently from 2562f1c to 5bfe973 Compare July 12, 2024 20:44
java/values.yaml Outdated
Comment on lines 16 to 58
sidecars:
- name: helper1
image:
repository: "repo"
tag: "123"
extraContainerPorts: [8088, 9099]
securityContext:
readOnlyRootFilesystem: true
allowPrivilegeEscalation: false
resources:
limits:
cpu: 100m
memory: 128Mi
requests:
cpu: 100m
memory: 128Mi
environment: { }
secrets: { }
volumeMounts: [ ]
# - name: secret-volume
# mountPath: /app/secrets
# readOnly: true
- name: helper2
image:
repository: "repo"
tag: "456"
extraContainerPorts: [8088, 9099]
securityContext:
readOnlyRootFilesystem: true
allowPrivilegeEscalation: false
resources:
limits:
cpu: 100m
memory: 128Mi
requests:
cpu: 100m
memory: 128Mi
environment: { }
secrets: { }
volumeMounts:
- name: secret-volume
mountPath: /app/secrets
readOnly: true
Copy link
Contributor

Choose a reason for hiding this comment

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

This will force everyone using this chart to use this settings. If they do not explicit override or give a null value they will get this values. There is other company's using this helm chart so this is no go. use sidecars: {} as value and comment out the "example" (see csi: {})

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you for noticing that. Now it should do the job. Please check again.

Comment on lines 17 to 59
sidecars:
- name: helper1
image:
repository: "repo"
tag: "123"
extraContainerPorts: [8088, 9099]
securityContext:
readOnlyRootFilesystem: true
allowPrivilegeEscalation: false
resources:
limits:
cpu: 100m
memory: 128Mi
requests:
cpu: 100m
memory: 128Mi
environment: { }
secrets: { }
volumeMounts: [ ]
# - name: secret-volume
# mountPath: /app/secrets
# readOnly: true
- name: helper2
image:
repository: "repo"
tag: "456"
extraContainerPorts: [8088, 9099]
securityContext:
readOnlyRootFilesystem: true
allowPrivilegeEscalation: false
resources:
limits:
cpu: 100m
memory: 128Mi
requests:
cpu: 100m
memory: 128Mi
environment: { }
secrets: { }
volumeMounts:
- name: secret-volume
mountPath: /app/secrets
readOnly: true
Copy link
Contributor

Choose a reason for hiding this comment

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

This will force everyone using this chart to use this settings. If they do not explicit override or give a null value they will get this values. There is other company's using this helm chart so this is no go. use sidecars: {} as value and comment out the "example" (see csi: {})

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Same here.

@mipieta mipieta merged commit 6b23510 into evry-ace:master Oct 16, 2024
1 check passed
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 this pull request may close these issues.

4 participants