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.

Sign up for GitHub

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

Jump to bottom

feat: [helm] Support for PVC Annotations for Non-Distributed Modes #12023

Merged
merged 26 commits into from
May 23, 2024
Merged

feat: [helm] Support for PVC Annotations for Non-Distributed Modes #12023

merged 26 commits into from
May 23, 2024

Conversation

onelapahead
Copy link
Contributor

@onelapahead onelapahead commented Feb 21, 2024 •
edited
Loading

What this PR does / why we need it:

This adds the ability to specify PVC annotations in the volume claim templates of all StatefulSets in the chart (read, write, backend, and singleBinary).

CSI drivers often allow for customizing PVCs, or modifying existing PVCs, via annotations. For example, in the open-source the AWS EBS CSI driver allows for changing IOPS, storage type, etc. via annotations: https://github.com/kubernetes-sigs/aws-ebs-csi-driver/blob/master/docs/modify-volume.md.

Supporting PVC annotations is quite common in Helm charts. For example, the minio chart that is a dependency of the loki chart also allows for such customization and this PR additionally sets that value so it is more clearly documented for loki chart users.

Which issue(s) this PR fixes:
N/A

Special notes for your reviewer:

Checklist

  • Reviewed the CONTRIBUTING.md guide (required)
  • Documentation added
  • Tests updated
  • CHANGELOG.md updated
    • If the change is worth mentioning in the release notes, add add-to-release-notes label
  • 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
@onelapahead onelapahead requested a review from a team as a code owner February 21, 2024 15:49
@CLAassistant
Copy link

CLAassistant commented Feb 21, 2024 •
edited
Loading

CLA assistant check
All committers have signed the CLA.
@onelapahead onelapahead changed the title [helm] Support for PVC Annotations Feb 21, 2024
@github-actions github-actions bot added sig/operator type/docs Issues related to technical documentation; the Docs Squad uses this label across many repositories labels Feb 22, 2024
@onelapahead
Copy link
Contributor Author

Apologies messed up a rebase, one sec
@onelapahead
[helm] Support for PVC Annotations
48453b6 
Signed-off-by: hfuss <hayden.fuss@kaleido.io>
@github-actions github-actions bot removed sig/operator type/docs Issues related to technical documentation; the Docs Squad uses this label across many repositories labels Feb 22, 2024
@onelapahead
restore chart changes
8e12364 
Signed-off-by: hfuss <hayden.fuss@kaleido.io>
@onelapahead
Copy link
Contributor Author

There's some extra checks / reviewers still on the PR due to the rebase I messed up, but the PR is now ready for review again and up-to-date.
@danielfrg
Copy link

Could we get some eyes on this one please.
Hopefully should be a quick one to review.
trevorwhitney
trevorwhitney approved these changes Apr 22, 2024
View reviewed changes
Copy link
Collaborator

@trevorwhitney trevorwhitney left a comment

Choose a reason for hiding this comment

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

Looks good to me, thanks! Approving to unblock, but I don't understand why we have to wrap the with in an if?
docs/Makefile
@@ -10,6 +10,7 @@ include docs.mk
PODMAN := $(shell if command -v podman >/dev/null 2>&1; then echo podman; else echo docker; fi)
BUILD_IN_CONTAINER ?= true

.PHONY: sources/setup/install/helm/reference.md
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm confused why this needs to be a .PHONY when this file does exist. maybe you need to clean first? But I'm guessing it failed because the file existed even though the contents weren't what they should be, and the .PHONY just forces it to always run? I guess that's a fair trade off vs having to remember to clean everytime.
production/helm/loki/CHANGELOG.md Outdated Show resolved Hide resolved
production/helm/loki/templates/backend/statefulset-backend.yaml Outdated Show resolved Hide resolved
@onelapahead
merge from main
d7330a3 
Signed-off-by: hfuss <hayden.fuss@kaleido.io>
@onelapahead onelapahead changed the title feat: [helm] Support for PVC Annotations May 9, 2024
@onelapahead
update docs
8d79690 
Signed-off-by: hfuss <hayden.fuss@kaleido.io>
@onelapahead
changelog
7570cf8 
Signed-off-by: hfuss <hayden.fuss@kaleido.io>
@onelapahead onelapahead changed the title feat: [helm] Support for PVC Annotations for SSD Mode May 9, 2024
@onelapahead
changelog
b86f32a 
Signed-off-by: hfuss <hayden.fuss@kaleido.io>
@onelapahead
Copy link
Contributor Author

Thanks @trevorwhitney for the feedback and patience, believe everything has been addressed now! Noting that all PVCs / claim templates in the new distributed mode of deployment had annotations support, so this PR is largely just adding this support to single binary and simple scalable modes now.
@onelapahead
Copy link
Contributor Author

@trevorwhitney @MichelHollands can this be given a final review soon before this PR falls out-of-date again ?
@trevorwhitney trevorwhitney enabled auto-merge (squash) May 23, 2024 20:22
@trevorwhitney trevorwhitney merged commit efdae3d into grafana:main May 23, 2024
61 of 62 checks passed
trevorwhitney added a commit that referenced this pull request May 24, 2024
@trevorwhitney
Squashed commit of the following:
d9e8b48 
commit 0bfd0ad
Merge: 68aa188 efdae3d
Author: Trevor Whitney <trevorjwhitney@gmail.com>
Date:   Thu May 23 17:04:32 2024 -0600

    Merge branch 'main' into sample-count-and-bytes

commit 68aa188
Author: Trevor Whitney <trevorjwhitney@gmail.com>
Date:   Thu May 23 17:03:32 2024 -0600

    feat: guard aggregation behavior behind a feature flag

commit efdae3d
Author: hayden <haydenfuss@gmail.com>
Date:   Thu May 23 16:25:50 2024 -0400

    feat(helm): Support for PVC Annotations for Non-Distributed Modes (#12023)

    Signed-off-by: hfuss <hayden.fuss@kaleido.io>
    Co-authored-by: J Stickler <julie.stickler@grafana.com>
    Co-authored-by: Trevor Whitney <trevorjwhitney@gmail.com>

commit f0d6a92
Author: Trevor Whitney <trevorjwhitney@gmail.com>
Date:   Thu May 23 14:03:32 2024 -0600

    feat: reject filter queries to /patterns endpoint

commit dc620e7
Author: Trevor Whitney <trevorjwhitney@gmail.com>
Date:   Wed May 8 14:08:44 2024 -0600

    feat: collect and serve pre-agg bytes and count

    * pre-aggregate bytes and count per stream in the pattern ingester
    * serve bytes_over_time and count_over_time queries from the patterns
      endpoint

commit 97212ea
Author: Jay Clifford <45856600+Jayclifford345@users.noreply.github.com>
Date:   Thu May 23 12:10:48 2024 -0400

    feat: Added Interactive Sandbox to Quickstart tutorial (#12701)

commit 1111595
Author: Vladyslav Diachenko <82767850+vlad-diachenko@users.noreply.github.com>
Date:   Thu May 23 13:18:16 2024 +0300

    feat: new stream count limiter (#13006)

    Signed-off-by: Vladyslav Diachenko <vlad.diachenko@grafana.com>
    Co-authored-by: JordanRushing <rushing.jordan@gmail.com>

commit 987e551
Author: Quentin Bisson <quentin@giantswarm.io>
Date:   Thu May 23 02:15:52 2024 +0200

    fix: allow cluster label override in bloom dashboards (#13012)

    Signed-off-by: QuentinBisson <quentin@giantswarm.io>

commit d3c9cec
Author: Quentin Bisson <quentin@giantswarm.io>
Date:   Thu May 23 01:59:28 2024 +0200

    fix: upgrade old plugin for the loki-operational dashboard. (#13016)

    Signed-off-by: QuentinBisson <quentin@giantswarm.io>

commit 8d9fb68
Author: Quentin Bisson <quentin@giantswarm.io>
Date:   Wed May 22 22:00:08 2024 +0200

    fix: remove unneccessary disk panels for ssd read path (#13014)

    Signed-off-by: QuentinBisson <quentin@giantswarm.io>

commit 1948899
Author: Quentin Bisson <quentin@giantswarm.io>
Date:   Wed May 22 15:16:29 2024 +0200

    fix: Mixins - Add missing log datasource on loki-deletion (#13011)

commit efd8f5d
Author: Salva Corts <salva.corts@grafana.com>
Date:   Wed May 22 10:43:32 2024 +0200

    refactor(blooms): Add queue to bloom planner and enqueue tasks (#13005)

commit d6f29fc
Author: Vitor Gomes <41302394+vitoorgomes@users.noreply.github.com>
Date:   Wed May 22 04:34:42 2024 +1200

    docs: update otlp ingestion with correct endpoint and add endpoint to reference api docs (#12996)

commit 3195036
Author: Salva Corts <salva.corts@grafana.com>
Date:   Tue May 21 13:12:24 2024 +0200

    refactor(bloom planner): Compute gaps and build tasks from metas and TSDBs  (#12994)

commit 7a3338e
Author: Jonathan Davies <jpds@protonmail.com>
Date:   Tue May 21 10:41:42 2024 +0100

    feat: loki/main.go: Log which config file path is used on startup (#12985)

    Co-authored-by: Michel Hollands <42814411+MichelHollands@users.noreply.github.com>

commit bf8a278
Author: Ashwanth <iamashwanth@gmail.com>
Date:   Tue May 21 12:56:07 2024 +0530

    chore: remove duplicate imports (#13001)

commit 1f5291a
Author: Ashwanth <iamashwanth@gmail.com>
Date:   Tue May 21 12:38:02 2024 +0530

    fix(indexstats): do not collect stats from "IndexStats" lookups for other query types (#12978)

commit 8442dca
Author: Jay Clifford <45856600+Jayclifford345@users.noreply.github.com>
Date:   Mon May 20 17:52:17 2024 -0400

    feat: Added getting started video (#12975)

commit 75ccf21
Author: Christian Haudum <christian.haudum@gmail.com>
Date:   Mon May 20 17:14:40 2024 +0200

    feat(blooms): Separate page buffer pools for series pages and bloom pages (#12992)

    Series pages are much smaller than bloom pages and therefore can make use of a separate buffer pool with different buckets.

    The second commit fixes a possible panic.

    Signed-off-by: Christian Haudum <christian.haudum@gmail.com>

commit 94d610e
Author: Yarden Shoham <git@yardenshoham.com>
Date:   Mon May 20 18:05:50 2024 +0300

    docs: Fix broken link in the release notes (#12990)

    Co-authored-by: J Stickler <julie.stickler@grafana.com>

commit 31a1314
Author: choeffer <christian.hoeffer@maibornwolff.de>
Date:   Mon May 20 16:39:25 2024 +0200

    docs(install-monolithic): add quotation marks (#12982)

    Co-authored-by: Michel Hollands <42814411+MichelHollands@users.noreply.github.com>

commit 8978ecf
Author: Salva Corts <salva.corts@grafana.com>
Date:   Mon May 20 12:36:22 2024 +0200

    feat: Boilerplate for new bloom build planner and worker components. (#12989)
This was referenced May 27, 2024
Open
Open
This was referenced Jun 10, 2024
Open
Merged
@loki-gh-app loki-gh-app bot mentioned this pull request Jun 24, 2024
Open
@loki-gh-app loki-gh-app bot mentioned this pull request Jul 1, 2024
Open
@grafanabot grafanabot mentioned this pull request Jul 2, 2024
Open
This was referenced Jul 3, 2024
Open
Closed
Open
@RodrigoCMoraes RodrigoCMoraes mentioned this pull request Jul 12, 2024
Closed
@loki-gh-app loki-gh-app bot mentioned this pull request Jul 15, 2024
Open
@loki-gh-app loki-gh-app bot mentioned this pull request Jul 22, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/helm size/M type/docs Issues related to technical documentation; the Docs Squad uses this label across many repositories
5 participants
@onelapahead @CLAassistant @danielfrg @JStickler @trevorwhitney