Do not perform network namespace detection on AV update #912
Conversation
|
@Luap99 Do you think we can test this in CI? The netns stuff we're doing in Netavark testing confuses me a bit |
There was a problem hiding this comment.
code LGTM, for tests you can copy paste the netavark command in run_netavark and run it with run_helper instead then we will not join the same netns so it should trigger this warning. Note that this is only printed and not a error exit code so you have to check the full output.
| @@ -150,8 +150,9 @@ fw_driver=iptables | |||
| run_helper ps "$aardvark_pid" | |||
| assert "${lines[1]}" =~ ".*aardvark-dns --config $NETAVARK_TMPDIR/config/aardvark-dns -p $dns_port run" "aardvark not running or bad options" | |||
|
|
|||
| NETAVARK_DNS_PORT="$dns_port" run_netavark --file ${TESTSDIR}/testfiles/dualstack-bridge-network-container-dns-server.json \ | |||
| NETAVARK_DNS_PORT="$dns_port" run_helper $NETAVARK --config "$NETAVARK_TMPDIR/config" --rootless "$rootless" --file ${TESTSDIR}/testfiles/dualstack-bridge-network-container-dns-server.json \ | |||
There was a problem hiding this comment.
a comment on what we do this here may be appropriate as it may be forgotten otherwise
Update is not required to be in the same network namespace as everything else, because we're just restarting an existing server with an updated config. Make the check for Aardvark's netns only happen if we are not updating as such. Fixes containers#911 Signed-off-by: Matthew Heon <matthew.heon@pm.me>
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: Luap99, mheon 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 |
|
/hold cancel |
|
We can get a version cut this afternoon and work with that
…On Wed, Jan 31, 2024 at 12:28 Ed Santiago ***@***.***> wrote:
Thank you! @mheon <https://github.com/mheon> @lsm5
<https://github.com/lsm5> any idea when we can expect a rawhide bodhi?
—
Reply to this email directly, view it on GitHub
<#912 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AB3AOCFJAIXJEDX6IL4LSCTYRJ5LTAVCNFSM6AAAAABCRWMWPOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSMJZGU3TGNRVGU>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
Update is not required to be in the same network namespace as everything else, because we're just restarting an existing server with an updated config. Make the check for Aardvark's netns only happen if we are not updating as such.
Fixes #911