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

Remove mount in the docker driver. #2295

Merged
merged 2 commits into from
Jul 13, 2020

Conversation

cyriltovena
Copy link
Contributor

#2054 Introduced a way to mount file automatically in the docker driver. However we realized that for some operating systems if you don't have a data folder, installing the driver will fail.

Since we now have many other ways to pass pipeline stages I think we should remove this to make sure the driver onboarding experience is great.

Fixes #2269
Fixes #2147

For those who still want to use this feature you can pull and install this version grafana/loki-docker-driver:master-5e0fe09.

Signed-off-by: Cyril Tovena cyril.tovena@gmail.com

Signed-off-by: Cyril Tovena <cyril.tovena@gmail.com>
@codecov-commenter
Copy link

Codecov Report

Merging #2295 into master will increase coverage by 0.02%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2295      +/-   ##
==========================================
+ Coverage   62.29%   62.31%   +0.02%     
==========================================
  Files         158      158              
  Lines       12766    12766              
==========================================
+ Hits         7952     7955       +3     
+ Misses       4201     4198       -3     
  Partials      613      613              
Impacted Files Coverage Δ
pkg/logql/evaluator.go 92.13% <0.00%> (-0.41%) ⬇️
pkg/promtail/targets/filetarget.go 70.48% <0.00%> (+1.80%) ⬆️
pkg/querier/queryrange/downstreamer.go 97.93% <0.00%> (+2.06%) ⬆️

@owen-d
Copy link
Member

owen-d commented Jul 6, 2020

I'm not a user of this, but it seems like a nice addition to have. That being said, if it's difficult to make work for all cases I'm fine removing it.

It looks like there's a small issue building it now -- let's get that fixed and then LGTM.

Signed-off-by: Cyril Tovena <cyril.tovena@gmail.com>
@slim-bean
Copy link
Collaborator

I would also mention another alternative now is to have the docker driver send to a promtail instance directly as promtail now implements the Loki push api as of #2296, this may be an easier way to configure pipelines, especially if you want to use metric stages.

@slim-bean
Copy link
Collaborator

I would also note that it's dangerous to use the linked master-5e0fe09 as we do not guarantee this won't be deleted someday, perhaps stick with the 1.5.0 tagged version.

@darkl0rd
Copy link

darkl0rd commented Aug 4, 2020

Having the 'mount' option was extremely convenient. I ended up here, because I was setting up a few new nodes and my playbook failed due to the data.source plugin option having disappeared.

Is there no way for you to continue supporting this?

@cyriltovena
Copy link
Contributor Author

You can set that value in the json file of the installed plugin if you go to /var/lib/docker/plugins/<plugin_id>/config.json

You can also use an older version of the plugin. The problem is that this makes the plugin fails for a lot of users.

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