Conversation
Signed-off-by: HMetcalfeW <106991365+HMetcalfeW@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR addresses permission issues when downloading Azure billing exports to disk by creating an empty /var/configs directory mount. It also fixes a Helm template bug where empty manifest separators were being generated when certain features were disabled.
Changes:
- Added emptyDir volume mount for
/var/configsdirectory to fix permission denied errors when downloading Azure billing data - Fixed manifest separator placement in ingress and httproute templates to prevent empty manifest generation
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| charts/opencost/templates/deployment.yaml | Added var-configs emptyDir volume and mount to create the /var/configs directory before mounting cloud integration files |
| charts/opencost/templates/ingress.yaml | Moved --- manifest separators inside conditional blocks to prevent empty manifest output when features are disabled |
| charts/opencost/templates/httproute.yaml | Moved --- manifest separators inside conditional blocks to prevent empty manifest output when features are disabled |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
ameijer
left a comment
There was a problem hiding this comment.
bump the chart version please
ameijer
left a comment
There was a problem hiding this comment.
@HMetcalfeW looks good to us, can you bump chart version?
|
@HMetcalfeW can you bump the chart? then ping ishaan and we will get this merged! |
Signed-off-by: Hunter <106991365+HMetcalfeW@users.noreply.github.com>
Signed-off-by: HMetcalfeW <106991365+HMetcalfeW@users.noreply.github.com>
|
@ameijer, I've fixed the merge conflicts and have bumped the Helm Chart version |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -40,8 +40,8 @@ spec: | |||
| port: {{ default $.Values.opencost.ui.uiPort .port }} | |||
| {{- end }} | |||
| {{- end }} | |||
| {{- if $cloudIntegrationSecretName }} | ||
| - name: var-configs | ||
| mountPath: /var/configs | ||
| - name: cloud-integration | ||
| mountPath: /var/configs/cloud-integration.json | ||
| subPath: cloud-integration.json |
| {{- end }} | ||
| {{- if $cloudIntegrationSecretName }} | ||
| - name: var-configs | ||
| emptyDir: {} |
| --- | ||
| {{- end }} |
Description
Issue identified when testing the addition of GZIP Azure billing exports. The default option is to download exports to file vs. streaming them.
Also fixed two minor issues where empty manifests are generated because the
---break between manifests wasn't included within the if blocks.Example error
Related Issues
Relates to opencost/opencost#3572
User Impact
Without the env variable
AZURE_DOWNLOAD_BILLING_DATA_TO_DISKset to false, cloud-costs will not work.opencost:
env:
- name: AZURE_DOWNLOAD_BILLING_DATA_TO_DISK
value: "false"
Testing
See testing steps as part of: opencost/opencost#3572
Tested with these Helm values
Current develop branch with values above
Deployed with modified Helm Chart