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

fix(helm): fix extraObjects #13107

Merged
merged 2 commits into from
Jun 18, 2024
Merged

Conversation

hervenicol
Copy link
Contributor

What this PR does / why we need it:

Fixes a regression introduced with #12397

When we had multiple extraObjects the rendering was missing linefeed before the separation triple-dashes (---).
Like here:

apiVersion: testResource/v1alpha1
kind: testResource
metadata:
  name: test1
spec: void---
apiVersion: testResource/v1alpha1
kind: testResource
metadata:
  name: test2
spec: void---
apiVersion: testResource/v1alpha1
kind: testResource
metadata:
  name: test3
spec: void
---

With the fix it looks like this:

apiVersion: testResource/v1alpha1
kind: testResource
metadata:
  name: test1
spec: void
---
# Source: loki/templates/extra-manifests.yaml
apiVersion: testResource/v1alpha1
kind: testResource
metadata:
  name: test2
spec: void
---
# Source: loki/templates/extra-manifests.yaml
apiVersion: testResource/v1alpha1
kind: testResource
metadata:
  name: test3
spec: void
---

Special notes for your reviewer:

Checklist

  • Reviewed the CONTRIBUTING.md guide (required)
  • Documentation added
  • Tests updated
  • Title matches the required conventional commits format, see here
    • Note that Promtail is considered to be feature complete, and future development for logs collection will be in Grafana Alloy. As such, feat PRs are unlikely to be accepted unless a case can be made for the feature actually being a bug fix to existing behavior.
  • Changes that require user attention or interaction to upgrade are documented in docs/sources/setup/upgrade/_index.md
  • For Helm chart changes bump the Helm chart version in production/helm/loki/Chart.yaml and update production/helm/loki/CHANGELOG.md and production/helm/loki/README.md. Example PR
  • If the change is deprecating or removing a configuration option, update the deprecated-config.yaml and deleted-config.yaml files respectively in the tools/deprecated-config-checker directory. Example PR

@hervenicol hervenicol requested a review from a team as a code owner June 3, 2024 12:16
@hervenicol hervenicol force-pushed the helm-fix-extraobjects branch from 1476b9e to 8334179 Compare June 3, 2024 12:19
@QuentinBisson
Copy link
Contributor

@MichelHollands could you please take a look at this patch?

@hervenicol
Copy link
Contributor Author

Maybe @TheRealNoob would be interested in having a look at it as well.

@hervenicol hervenicol force-pushed the helm-fix-extraobjects branch from 8334179 to 14f1ff6 Compare June 17, 2024 07:59
@QuentinBisson
Copy link
Contributor

@MichelHollands @jeschkies is there a way to help merging this PR? The change is really small and it fixes a real issue

@QuentinBisson
Copy link
Contributor

Thank you :)

@jeschkies jeschkies merged commit b7fcf2b into grafana:main Jun 18, 2024
62 checks passed
@TheRealNoob
Copy link
Contributor

@hervenicol sorry I missed this notification. Apologies for not catching this! Thank you for fixing it

@QuentinBisson
Copy link
Contributor

@TheRealNoob I think we forgot to bump the chart version in this PR

mraboosk pushed a commit to mraboosk/loki that referenced this pull request Oct 7, 2024
Co-authored-by: Herve Nicol <12008875+hervenicol@users.noreply.github.com>
Co-authored-by: Karsten Jeschkies <karsten.jeschkies@grafana.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants