Refactor aro-dnsmasq-pre.sh #4501
Conversation
ventifus
requested review from
bennerv,
cadenmarchese,
fahlmant,
hawkowl,
hlipsig,
jharrington22,
kimorris27,
mociarain,
mrWinston,
pepedocs,
rogbas,
sankur-codes,
tiguelu,
tsatam,
wanghaoran1988 and
yjst2012
as code owners
ventifus
force-pushed
the
ventifus/ARO-23104-dnsmasq-pre-robustness
branch
from
71c8abd to
c3fc857
Compare
Derive all information from NetworkManager directly to avoid setting dnsmasq's name server to the node IP in some circumstances
|
/azp run ci |
|
Azure Pipelines successfully started running 1 pipeline(s). |
mociarain
left a comment
mociarain
left a comment
There was a problem hiding this comment.
This looks a good change and based on the context in the JIRA I think it's good to merge as is but I wonder if it's worth testing the operator change for "real". AFAIK @ehvs wrote this process up recently. I tried to find it in the wiki but gave up. Do you have the link?
I'll approve anyway but wdyt?
|
Test best test for this is to perform an install of a UDR cluster with invalid DNS on it's VNET. This requires a working gateway so will likely need Canary. |
kimorris27
left a comment
kimorris27
left a comment
There was a problem hiding this comment.
+1 to Maitiu. I think it could merge as-is, but testing definitely won't hurt.
hlipsig
left a comment
hlipsig
left a comment
There was a problem hiding this comment.
LGTM, is a test in canary the only way, or can we install a dev cluster into production and validate that way? Can do a hive-less cluster install via local RP, and I know we can do it with a local build ARO operator as well. Amber's done that in the past.
|
We need to test with egress lockdown enabled and a working gateway, I don't think we can do that in an environment less than canary. |
Derive all information from NetworkManager directly to avoid setting dnsmasq's name server to the node IP in some circumstances
Which issue this PR addresses:
Fixes ARO-23104
What this PR does / why we need it:
In some circumstances where dnsmasq.service's ExecStopPost fails to fire, we end up consuming our own NetworkManager configuration resulting in resolv.conf.dnsmasq containing the node's IP not the upstream DNS servers' IPs. To avoid that, we now look directly at NetworkManager's configuration as obtained by DHCP. As a result, we no longer need to delete the NetworkManager drop-in with dnsmasq's ExecStopPost, and re-executing aro-dnsmasq-pre.sh is idempotent.
Test plan for issue:
Is there any documentation that needs to be updated for this PR?
How do you know this will function as expected in production?