Skip to content

ptp: only override DNS conf if DNS settings provided#388

Merged
squeed merged 1 commit intocontainernetworking:masterfrom
sipsma:fix-ptpdns
Sep 25, 2019
Merged

ptp: only override DNS conf if DNS settings provided#388
squeed merged 1 commit intocontainernetworking:masterfrom
sipsma:fix-ptpdns

Conversation

@sipsma
Copy link
Copy Markdown
Contributor

@sipsma sipsma commented Sep 18, 2019

Previously, if an IPAM plugin provided DNS settings in the result to the PTP
plugin, those settings were always lost because the PTP plugin would always
provide its own DNS settings in the result even if the PTP plugin was not
configured with any DNS settings.

This was especially problematic when trying to use, for example, the host-local
IPAM plugin's support for retrieving DNS settings from a resolv.conf file on
the host. Before this change, those DNS settings were always lost when using the
PTP plugin and couldn't be specified as part of PTP instead because PTP does not
support parsing a resolv.conf file.

This change checks to see if any fields were actually set in the PTP plugin's
DNS settings and only overrides any previous DNS results from an IPAM plugin in
the case that settings actually were provided to PTP. In the case where no
DNS settings are provided to PTP, the DNS results of the IPAM plugin (if any)
are used instead.

Signed-off-by: Erik Sipsma [email protected]

Previously, if an IPAM plugin provided DNS settings in the result to the PTP
plugin, those settings were always lost because the PTP plugin would always
provide its own DNS settings in the result even if the PTP plugin was not
configured with any DNS settings.

This was especially problematic when trying to use, for example, the host-local
IPAM plugin's support for retrieving DNS settings from a resolv.conf file on
the host. Before this change, those DNS settings were always lost when using the
PTP plugin and couldn't be specified as part of PTP instead because PTP does not
support parsing a resolv.conf file.

This change checks to see if any fields were actually set in the PTP plugin's
DNS settings and only overrides any previous DNS results from an IPAM plugin in
the case that settings actually were provided to PTP. In the case where no
DNS settings are provided to PTP, the DNS results of the IPAM plugin (if any)
are used instead.

Signed-off-by: Erik Sipsma <[email protected]>
Copy link
Copy Markdown
Contributor

@bboreham bboreham left a comment

Choose a reason for hiding this comment

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

lgtm, thanks!

@squeed
Copy link
Copy Markdown
Member

squeed commented Sep 25, 2019

👍 looks good to me.

@squeed squeed merged commit 0f19aa2 into containernetworking:master Sep 25, 2019
Kern-- pushed a commit to Kern--/plugins that referenced this pull request Apr 21, 2022
Previously, the bridge plugin ignored DNS settings returned
from an IPAM plugin (e.g. the host-local plugin parsing
resolv.conf to configure DNS). With this change, the bridge plugin
uses IPAM DNS settings.

Similarly to containernetworking#388, this change will use incoming DNS settings if set,
otherwise IPAM plugin returned DNS settings

Signed-off-by: Kern Walster <[email protected]>
Kern-- pushed a commit to Kern--/plugins that referenced this pull request Apr 21, 2022
Previously, the bridge plugin ignored DNS settings returned
from an IPAM plugin (e.g. the host-local plugin parsing
resolv.conf to configure DNS). With this change, the bridge plugin
uses IPAM DNS settings.

Similarly to containernetworking#388, this change will use incoming DNS settings if set,
otherwise IPAM plugin returned DNS settings

Signed-off-by: Kern Walster <[email protected]>
mccv1r0 pushed a commit to mccv1r0/plugins that referenced this pull request Jan 4, 2023
Previously, the bridge plugin ignored DNS settings returned
from an IPAM plugin (e.g. the host-local plugin parsing
resolv.conf to configure DNS). With this change, the bridge plugin
uses IPAM DNS settings.

Similarly to containernetworking#388, this change will use incoming DNS settings if set,
otherwise IPAM plugin returned DNS settings

Signed-off-by: Kern Walster <[email protected]>
mccv1r0 pushed a commit to mccv1r0/plugins that referenced this pull request Jan 10, 2023
Previously, the bridge plugin ignored DNS settings returned
from an IPAM plugin (e.g. the host-local plugin parsing
resolv.conf to configure DNS). With this change, the bridge plugin
uses IPAM DNS settings.

Similarly to containernetworking#388, this change will use incoming DNS settings if set,
otherwise IPAM plugin returned DNS settings

Signed-off-by: Kern Walster <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants