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

Helm: Allows loki s3 http_config block to be configured #7041

Merged
merged 1 commit into from
Sep 15, 2022
Merged

Helm: Allows loki s3 http_config block to be configured #7041

merged 1 commit into from
Sep 15, 2022

Conversation

j4rj4r
Copy link
Contributor

@j4rj4r j4rj4r commented Sep 3, 2022

Signed-off-by: CAJNA Jarod [email protected]

What this PR does / why we need it:

This will allow users of the chart to configure the s3 http_config block as documented here https://grafana.com/docs/loki/latest/configuration/#s3_storage_config through values.

Which issue(s) this PR fixes:
Fixes grafana/helm-charts#1762

-->
Checklist

  • Documentation added
  • Tests updated
  • Is this an important fix or new feature? Add an entry in the CHANGELOG.md.
  • Changes that require user attention or interaction to upgrade are documented in docs/sources/upgrading/_index.md

@j4rj4r j4rj4r requested a review from a team as a code owner September 3, 2022 11:28
@CLAassistant
Copy link

CLAassistant commented Sep 3, 2022

CLA assistant check
All committers have signed the CLA.

@grafanabot
Copy link
Collaborator

./tools/diff_coverage.sh ../loki-main/test_results.txt test_results.txt ingester,distributor,querier,querier/queryrange,iter,storage,chunkenc,logql,loki

Change in test coverage per package. Green indicates 0 or positive change, red indicates that test coverage for a package fell.

+           ingester	0%
+        distributor	0%
+            querier	0%
+ querier/queryrange	0%
+               iter	0%
+            storage	0%
+           chunkenc	0%
+              logql	0%
+               loki	0%

Copy link
Contributor

@jeschkies jeschkies left a comment

Choose a reason for hiding this comment

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

Thanks. Let's wait for @trevorwhitney to take a look.

@j4rj4r j4rj4r changed the title helm: Allows loki s3 http_config block to be configured Helm: Allows loki s3 http_config block to be configured Sep 7, 2022
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.

Thanks for the addition, LGTM!

@trevorwhitney
Copy link
Collaborator

looks like helm ci is broken, are you using the correct version of helm-docs?

@j4rj4r
Copy link
Contributor Author

j4rj4r commented Sep 8, 2022

My bad, I'm on vacation, I'm watching Sunday

@j4rj4r
Copy link
Contributor Author

j4rj4r commented Sep 10, 2022

@trevorwhitney It should be good now !

@j4rj4r j4rj4r requested review from jeschkies and trevorwhitney and removed request for trevorwhitney and jeschkies September 15, 2022 14:17
@grafanabot
Copy link
Collaborator

./tools/diff_coverage.sh ../loki-main/test_results.txt test_results.txt ingester,distributor,querier,querier/queryrange,iter,storage,chunkenc,logql,loki

Change in test coverage per package. Green indicates 0 or positive change, red indicates that test coverage for a package fell.

+           ingester	0%
+        distributor	0%
+            querier	0%
+ querier/queryrange	0%
+               iter	0%
+            storage	0%
+           chunkenc	0%
+              logql	0%
+               loki	0%

@jeschkies jeschkies merged commit 41861bd into grafana:main Sep 15, 2022
lxwzy pushed a commit to lxwzy/loki that referenced this pull request Nov 7, 2022
**What this PR does / why we need it**:

This will allow users of the chart to configure the s3 http_config block
as documented here
[https://grafana.com/docs/loki/latest/configuration/#s3_storage_config](https://grafana.com/docs/loki/latest/configuration/#s3_storage_config)
through values.

**Which issue(s) this PR fixes**:
Fixes grafana/helm-charts#1762

**Checklist**
- [x] Documentation added
- [x] Tests updated
- [ ] Is this an important fix or new feature? Add an entry in the
`CHANGELOG.md`.
- [ ] Changes that require user attention or interaction to upgrade are
documented in `docs/sources/upgrading/_index.md`

Signed-off-by: CAJNA Jarod <[email protected]>
mraboosk pushed a commit to mraboosk/loki that referenced this pull request Oct 7, 2024
**What this PR does / why we need it**:

This will allow users of the chart to configure the s3 http_config block
as documented here
[https://grafana.com/docs/loki/latest/configuration/#s3_storage_config](https://grafana.com/docs/loki/latest/configuration/#s3_storage_config)
through values.

**Which issue(s) this PR fixes**:
Fixes grafana/helm-charts#1762

**Checklist**
- [x] Documentation added
- [x] Tests updated
- [ ] Is this an important fix or new feature? Add an entry in the
`CHANGELOG.md`.
- [ ] Changes that require user attention or interaction to upgrade are
documented in `docs/sources/upgrading/_index.md`

Signed-off-by: CAJNA Jarod <[email protected]>
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.

[Loki-simple-scalable] S3 http_config parameter is not used
5 participants