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

Add using_boltdb_shipper to environments/loki/main.jsonnet sample #2936

Closed
wants to merge 4 commits into from

Conversation

CandiedCode
Copy link

@CandiedCode CandiedCode commented Nov 14, 2020

What this PR does / why we need it:
This resolve error for evaluating jsonnet: RUNTIME ERROR: must define boltdb_shipper_shared_store.

This also updates a failed test due to Charts Deprecation as of Nov 13, 2020. For more information, see the Helm Charts Deprecation and Archive Notice, and Update via https://github.com/helm/charts\#%EF%B8%8F-deprecation-and-archive-notice

Which issue(s) this PR fixes:
Fixes #2828

Special notes for your reviewer:

Checklist

  • Documentation added
  • Tests updated

This resolve error for evaluating jsonnet: RUNTIME ERROR: must define boltdb_shipper_shared_store
and grafana#2828
@CLAassistant
Copy link

CLAassistant commented Nov 14, 2020

CLA assistant check
All committers have signed the CLA.

As of Nov 13, 2020, charts in github.com/helm/charts will no longer be updated. For more information, see the Helm Charts Deprecation and Archive Notice, and Update via https://github.com/helm/charts\#%EF%B8%8F-deprecation-and-archive-notice
@codecov-io
Copy link

Codecov Report

Merging #2936 (8af087d) into master (78bb4bc) will decrease coverage by 0.01%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2936      +/-   ##
==========================================
- Coverage   61.74%   61.73%   -0.02%     
==========================================
  Files         181      181              
  Lines       14711    14711              
==========================================
- Hits         9084     9082       -2     
- Misses       4796     4798       +2     
  Partials      831      831              
Impacted Files Coverage Δ
pkg/querier/queryrange/downstreamer.go 95.29% <0.00%> (-2.36%) ⬇️

@@ -62,6 +62,11 @@ loki + promtail + gateway {
namespace: 'loki',
htpasswd_contents: 'loki:$apr1$H4yGiGNg$ssl5/NymaGFRUvxIV1Nyr.',

// Set to true if using boltdb shipper and uncomment boltdb_shipper_shared_store below
using_boltdb_shipper: false,

Copy link
Contributor

Choose a reason for hiding this comment

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

@sandeepsukhani should we mention about index_period here. I understand it only matters if default value differs during migration. Still wondering should we still make a note? WDYT?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should rewrite this example config with boltdb shipper as index store because we are defaulting to it now.
Regarding comment, I think it should be there in source jsonnet but I don't mind it being added here too.

@owen-d
Copy link
Member

owen-d commented Jan 7, 2021

First of all, sorry for the extremely long delay in getting to this.

Regarding the implementation, I'm hesitant to include using_boltdb_shipper: false, in the installation doc. The reasoning for this is using_boltdb_shipper: true is the default and I want to avoid negating that in the installation doc.

Seeing as the confusion stems from the subsequent access of boltdb_shipper_shared_store, I think we should amend that error message instead.

How about

boltdb_shipper_shared_store: error 'must define boltdb_shipper_shared_store when using_boltdb_shipper=true. If this is not intentional, consider disabling it. boltdb_shipper_shared_store is a backend key from the storage_config, such as (gcs) or (s3)',

edit: Given how long it's been since you submitted this PR, I'll push up a new PR with my suggestion. Feel free to incorporate it yourself if you'd like the contribution credit.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Tanka getting started errors with: must define boltdb_shipper_shared_store
6 participants