Bug 1865743: Add a prestart line to change ownership of openvswitch dir #2316
Conversation
openshift-ci-robot
added
bugzilla/severity-urgent
|
@abhat: This pull request references Bugzilla bug 1865743, which is valid. The bug has been moved to the POST state. The bug has been updated to refer to the pull request using the external bug tracker. 3 validation(s) were run on this bug
In response to this:
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. |
|
/assign @cgwalters. I also thought about doing Also, there are repercussions about using that option for persistent directories and how to set up StateDirectory/LogsDirectory etc. correctly without breaking existing assumptions about how openvswitch service works. Given that, this seemed to be a prudent choice. |
kikisdeliveryservice
requested review from
yuqi-zhang
and removed request for
sinnykumari
|
/approve tentatively, will leave review to colin |
openshift-ci-robot
added
the
approved
|
/test e2e-aws-serial |
|
Ok so there is openvswitch.useropts file that seems to be setting the I don't know who owns that env file and how it gets populated during ignition (via some config). But it would seem to make sense to use the same env. variable in this PR potentially? @yuqi-zhang, @cgwalters any ideas? |
The openvswitch dirs should be owned by the user running the openvswitch and ovsdb-server processes. This is controller either by contents of default.conf or /etc/config/openvswitch file. Signed-off-by: Aniket Bhat <anbhat@redhat.com>
|
The PR seems to be just applying the workaround mentioned in this bugzilla, so overall LGTM |
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: abhat, danielmellado, yuqi-zhang 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 |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
2 similar comments
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/cherry-pick release-4.6 |
|
@cuppett: once the present PR merges, I will cherry-pick it on top of release-4.6 in a new PR and assign it to you. In response to this:
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. |
| @@ -4,3 +4,6 @@ dropins: | |||
| contents: | | |||
| [Service] | |||
| Restart=always | |||
| ExecStartPre=-/bin/sh -c '/usr/bin/chown -R :$${OVS_USER_ID##*:} /var/lib/openvswitch' | |||
There was a problem hiding this comment.
Are you sure about the leading : to only change the group?
There's quite a lot of syntatic sorcery going on here.
OK $$ is to quote it from systemd's expansion, so the shell sees it.
The ##*: is...I don't quite follow that, what is it doing?
There was a problem hiding this comment.
I leveraged this sorcery from an existing ovs-vswitchd service file on a deployed cluster. I am assuming it works :).
|
@abhat: The following test failed, say
Full PR test history. Your PR dashboard. 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. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
@cuppett: new pull request created: #2331 In response to this:
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. |
|
@abhat: All pull requests linked via external trackers have merged: Bugzilla bug 1865743 has been moved to the MODIFIED state. In response to this:
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. |
Temporary fix for making sure the ownership of openvswitch directory is set to
openvswitchuser till we fix coreos/rpm-ostree#49.Signed-off-by: Aniket Bhat anbhat@redhat.com