ptp: only override DNS conf if DNS settings provided #388
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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 <sipsma@amazon.com>
|
👍 looks good to me. |
Kern--
added 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 <walster@amazon.com>
Kern--
added 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 <walster@amazon.com>
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 <walster@amazon.com>
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 <walster@amazon.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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 sipsma@amazon.com