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 privileged option so as mb to access data dir in Openshift #17606

Merged
merged 3 commits into from
Apr 9, 2020

Conversation

ChrsMark
Copy link
Member

@ChrsMark ChrsMark commented Apr 8, 2020

What does this PR do?

This PR adds privileged: true in securityContext of Metricbeat Daemonset spec file so as to enable access to data volume added in #17429.

Tested with minishift v1.34.2+83ebaab.

Why is it important?

Metricbeat is not able to start in Openshift without this option:

oc -n kube-system logs -f metricbeat-zsfwt
2020-04-06T10:58:56.615Z        INFO    instance/beat.go:622    Home path: [/usr/share/metricbeat] Config path: [/usr/share/metricbeat] Data path: [/usr/share/metricbeat/data] Logs path: [/usr/share/metricbeat/logs]
2020-04-06T10:58:56.615Z        INFO    instance/beat.go:372    metricbeat stopped.
2020-04-06T10:58:56.616Z        ERROR   instance/beat.go:933    Exiting: Failed to create Beat meta file: open /usr/share/metricbeat/data/meta.json.new: permission denied

Related to #17516.

cc: @exekias @jsoriano @blakerouse

Signed-off-by: chrismark <chrismarkou92@gmail.com>
@ChrsMark ChrsMark added bug Team:Platforms Label for the Integrations - Platforms team labels Apr 8, 2020
@ChrsMark ChrsMark requested review from a team April 8, 2020 09:14
@ChrsMark ChrsMark self-assigned this Apr 8, 2020
@elasticmachine
Copy link
Collaborator

Pinging @elastic/integrations-platforms (Team:Platforms)

Signed-off-by: chrismark <chrismarkou92@gmail.com>
@exekias
Copy link
Contributor

exekias commented Apr 8, 2020

Thanks for opening this! I wonder if we can better set permissions on the data folder to avoid needing this? It would be awesome if we could avoid privileged mode. Not sure myself if it's actually possible, but worth exploring

@jsoriano
Copy link
Member

jsoriano commented Apr 8, 2020

Thanks for opening this! I wonder if we can better set permissions on the data folder to avoid needing this? It would be awesome if we could avoid privileged mode. Not sure myself if it's actually possible, but worth exploring

We are solving it the same way in filebeat (here), so I think we could go on as this by now, but I agree that it would worth exploring alternatives to avoid running on privileged mode.

Same change will be probably needed in Auditbeat manifest too.

@ChrsMark
Copy link
Member Author

ChrsMark commented Apr 8, 2020

From https://kubernetes.io/docs/concepts/storage/volumes/#hostpath:
the files or directories created on the underlying hosts are only writable by root. You either need to run your process as root in a privileged Container or modify the file permissions on the host to be able to write to a hostPath volume

Given this, I don't see an "automatic" way to do this other than giving permissions to the directory on each one of the hosts.

However Openshift suggests to not use hostPath in production, but even if one do so then docs mention Pod should be privileged if I understand it correctly from the content.

In general, I don't see any value in having this data volume since the suggestion we provide with this is not so robust. How about removing it completely?

@exekias
Copy link
Contributor

exekias commented Apr 8, 2020

We need data path to store meta.json (which holds the beat UUID) and the registry file (in Filebeat). The registry is specially important as it keeps account of what has been read & sent, to avoid sending the same file over again. Because of this, I don't think we can get rid of a persistent data volume.

I'm ok with moving this forward as it is

@exekias exekias added the needs_backport PR is waiting to be backported to other branches. label Apr 8, 2020
@ChrsMark
Copy link
Member Author

ChrsMark commented Apr 8, 2020

We need data path to store meta.json (which holds the beat UUID) and the registry file (in Filebeat). The registry is specially important as it keeps account of what has been read & sent, to avoid sending the same file over again. Because of this, I don't think we can get rid of a persistent data volume.

I'm ok with moving this forward as it is

Ok for Filebeat it is mandatory, we cannot remove it. But is it important for Metricbeat too since it only keeps the UUID?

@exekias
Copy link
Contributor

exekias commented Apr 8, 2020

The UUID is used by stack monitoring to identify metricbeats, so I would say this is something we want to keep

@ChrsMark
Copy link
Member Author

ChrsMark commented Apr 9, 2020

Merging this, and will open a separate issue for Auditbeat since its not only privileged: true that required maybe because I also noticed another issue:

kubectl -n kube-system logs -f auditbeat-5nvd2                                           
2020-04-09T12:39:17.644Z	INFO	instance/beat.go:622	Home path: [/usr/share/auditbeat] Config path: [/usr/share/auditbeat] Data path: [/usr/share/auditbeat/data] Logs path: [/usr/share/auditbeat/logs]
2020-04-09T12:39:17.645Z	INFO	instance/beat.go:630	Beat ID: 11baa327-354b-4094-802f-813320e32375
2020-04-09T12:39:17.645Z	INFO	instance/beat.go:372	auditbeat stopped.
2020-04-09T12:39:17.645Z	ERROR	instance/beat.go:933	Exiting: error initializing processors: error unpacking add_process_metadata.target_fields: field 'container.id' not found
Exiting: error initializing processors: error unpacking add_process_metadata.target_fields: field 'container.id' not found

@ChrsMark ChrsMark merged commit eb4769c into elastic:master Apr 9, 2020
ChrsMark added a commit to ChrsMark/beats that referenced this pull request Apr 9, 2020
@ChrsMark ChrsMark removed the needs_backport PR is waiting to be backported to other branches. label Apr 9, 2020
ChrsMark added a commit to ChrsMark/beats that referenced this pull request Apr 9, 2020
ChrsMark added a commit that referenced this pull request Apr 9, 2020
ChrsMark added a commit that referenced this pull request Apr 9, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Team:Platforms Label for the Integrations - Platforms team v7.7.0 v7.8.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants