feature: (helm) support custom volumes and volumeMounts for epp

[delavet]

What type of PR is this?

/kind feature

What this PR does / why we need it:
please see #1944 for details
Which issue(s) this PR fixes:

Fixes #1944

Does this PR introduce a user-facing change?:

NONE

[k8s-ci-robot]

Hi @delavet. Thanks for your PR.

I'm waiting for a github.com member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

[netlify]

✅ Deploy Preview for gateway-api-inference-extension ready!

Name	Link
🔨 Latest commit	5ecba15
🔍 Latest deploy log	https://app.netlify.com/projects/gateway-api-inference-extension/deploys/6936a24c62ef240008eaef1e
😎 Deploy Preview	https://deploy-preview-1945--gateway-api-inference-extension.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

---

To edit notification comments on pull requests, go to your Netlify project configuration.

[nirrozenbaum]

/retest

[delavet]

Sorry, it seems that there was an issue with golangci-lint in my branch. I have rebased main to resolve this problem. Please help re-review it.🫡 @nirrozenbaum

[nirrozenbaum]

what is .Values.inferenceExtension.sidecar.configMap option?
I don't see it documented in the README.
additionally - maybe we can standardize the name (instead of configMap.name) and then then only relevant field would be some free form of configMapData?

[delavet]

As far as I know, the ability to deploy a sidecar for EPP was initially introduced in #1821. The primary purpose was to deploy an Envoy as a sidecar for EPP to achieve standalone deployment. The main purpose of ConfigMap is to provide startup configuration for Envoy.

The reason I made changes here is to support the generalization of this capability, since some EPP Sidecars may not require the deployment of a corresponding ConfigMap.

However, a series of fields related to the sidecar that were added at the time were not well documented in the README. I have now updated and supplemented them.

additionally - maybe we can standardize the name (instead of configMap.name) and then then only relevant field would be some free form of configMapData?

I think this comment is very reasonable, so I have made adjustments according to your suggestion. PTAL.

[nirrozenbaum]

/ok-to-test

[nirrozenbaum]

nit/question:
why do we do here:
{{- toYaml .Values.inferenceExtension.volumes | nindent 6 }} (toYaml at the beginning)
and in the configmap:
{{- .Values.inferenceExtension.sidecar.configMapData | toYaml | nindent 2 }} (toYaml after pipelining)?

not a blocker, asking out of curiosity and trying to understand if there is a reason for being inconsistent.

[delavet]

I agree: there are indeed many inconsistencies in syntax within the current Helm chart. I believe this is due to different contributors having varying syntax habits when working with Helm templates. However, in order to maintain the granularity of the current PR, I do not intend to unify all syntax within this PR. I suggest we create a new issue to track this problem. I can create an issue later to gather everyone's thoughts on this matter.👀

[nirrozenbaum]

sounds like a good fit for a new issue.
Thanks 👍

[nirrozenbaum]

/lgtm
/approve

/hold in case you want to fix inconsistency in the chart (see question about the toYaml).
if that was intentional and no need to fix anything just /unhold to get this merged.

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: delavet, nirrozenbaum

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [nirrozenbaum]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[delavet]

/unhold