Support custom sidecar containers in deployment spec, plus test. #371
Conversation
pega-talba
requested review from
dcasavant,
taz-pega-work and
a team
as code owners
| custom: | ||
| sidecarContainers: | ||
| - name: SIDECAR_NAME | ||
| image: SIDECAR_IMAGE_URL |
There was a problem hiding this comment.
IDK if it's worth calling out in this README, since if you're using a sidecar container you probably know or have some guide for the required spec... but a container needs some command and the resources requests. From first part of comment and sake of keeping README concise, I totally understand if it's intentional to leave this as is
There was a problem hiding this comment.
Updated with more indication that you do need more than simply the name and image. I don't think the command is always necessary, but resources should be there at least, and there's likely to be more. Using ... to denote extra stuff, other examples here use that notation.
There was a problem hiding this comment.
I vote for leaving them out since they're not required and might make things more confusing. Although maybe we could have some sort of phrasing that indicates you could expand upon the name and image to include more fields describing the container.
Edit: Oops, I just saw your update Adam
| custom: | ||
| sidecarContainers: | ||
| - name: SIDECAR_NAME | ||
| image: SIDECAR_IMAGE_URL |
There was a problem hiding this comment.
I vote for leaving them out since they're not required and might make things more confusing. Although maybe we could have some sort of phrasing that indicates you could expand upon the name and image to include more fields describing the container.
Edit: Oops, I just saw your update Adam
charts/pega/README.md
Outdated
| @@ -529,6 +529,25 @@ tier: | |||
| serviceAccountName: MY_SERVICE_ACCOUNT_NAME | |||
| ``` | |||
|
|
|||
| ### Sidecar Containers | |||
|
|
|||
| If the pod needs to run with one or more [sidecar containers](https://kubernetes.io/docs/concepts/workloads/pods/#how-pods-manage-multiple-containers), you can specify custom 'sidecarContainers' for your deployment tier. Each sidecar container must include a complete container definition, including a name, image, resources, etc. | |||
There was a problem hiding this comment.
The lack of a comprehensive reason for using sidecars makes this readme addition unhelpful. Please provide a client-centric journey for using sidecars. You are linking to a section in k8s docs that doesn't even have ''sidecar" in the section head...we can do better. I get keeping it simple, but please consider a client journey. How about somthing like
Pega supports adding one or more "sidecar" containers to manage a variety of requirements for your Pega application services that you must manage outside of the basic Pega pod deployment. You can specify custom 'sidecarContainers' for your deployment tier in the Pega Helm chart as shown in the example below. Each sidecar container in your tier must include a complete container definition, including a name, image and resources.
For an overview of the versatility sidecar containers present, see How Pods manage multiple containers.
There was a problem hiding this comment.
Updated with some refactored text based on these concerns.
| Example: | ||
|
|
||
| ```yaml | ||
| tier: |
There was a problem hiding this comment.
In which chart have you located these editable parameters for clients?? Because it's in the Pega readme, I thought it'd be in the values.yaml chart, but don't see that in this lis of changes ;-/ I see it in the terratest values chart, but how's a client supposed to use this?
There was a problem hiding this comment.
We're going to exclude it from the values.yaml itself because it would be a little messy to include it and it's not the typical expectation to have this.
Support custom sidecar containers within a pod, adjacent to the tomcat container.
Adopts the pull request #239 created by @mathijs55