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

Updating fluentd docker container mount path #8179

Closed

Conversation

ewolinetz
Copy link
Contributor

@ewolinetz ewolinetz requested a review from richm April 27, 2018 14:45
@openshift-ci-robot openshift-ci-robot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Apr 27, 2018
@openshift-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ewolinetz

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci-robot openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Apr 27, 2018
@ewolinetz
Copy link
Contributor Author

/hold

This may not be something we actually need to change. The fix that this patches should actually be resolved at the lower level. Rich is digging up the discussion that was had surrounding this in a prior email thread.

@openshift-ci-robot openshift-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Apr 27, 2018
@ewolinetz
Copy link
Contributor Author

@ewolinetz
Copy link
Contributor Author

@openshift-ci-robot
Copy link

openshift-ci-robot commented Apr 27, 2018

@ewolinetz: The following tests failed, say /retest to rerun them all:

Test name Commit Details Rerun command
ci/openshift-jenkins/install b83d581 link /test install
ci/openshift-jenkins/gcp b83d581 link /test gcp

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

@@ -61,7 +61,7 @@ spec:
- name: varlog
mountPath: /var/log
- name: varlibdockercontainers
mountPath: /var/lib/docker/containers
mountPath: /var/lib/docker
readOnly: true
Copy link
Member

Choose a reason for hiding this comment

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

Does fluentd pod works with either /var/lib/docker or /var/lib/docker/containers without any modification? I would expect these 2 directories to contain different content.

@gnufied
Copy link
Member

gnufied commented Apr 27, 2018

My main concern same as before is - this will again break in future if docker deployment causes "/var/lib/docker" to be mounted as private mount.

Rather than doing this here - is it possible to disable MountPropagation feature from inventory file that deploys the cluster to free-int? I tried making the same change in openshift-ansible - #7936 but now that I think of, since we are rolling back "slave" mount propagation defaults in upstream, modifying openshift-ansible may not be the right place.

@gnufied
Copy link
Member

gnufied commented Apr 27, 2018

Any change we make in openshift-ansible will be temporary(and not required) to fix this issue once openshift/origin#19364 PR merges. So it is best to workaround this by disabling MountPropagation feature via inventory file.

@jsafrane
Copy link
Contributor

jsafrane commented May 2, 2018

openshift/origin#19364 was merged into Origin 3.10, so this PR is probably not needed. Sorry it took so long.

@ewolinetz
Copy link
Contributor Author

Closing as the origin fix for this has merged

@ewolinetz ewolinetz closed this May 2, 2018
@ewolinetz ewolinetz deleted the fluentd_docker_mount_fix branch May 2, 2018 14:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants