ignore dns servers from dhcp when providing nameservers #4023

tylerschultz · 2022-11-28T22:34:36Z

What this PR does / why we need it

When users specify nameservers via CONTROL_PLANE_NODE_NAMESERVERS or WORKER_NODE_NAMESERVERS, CAPV is configured to ignore nameservers configured from DHCP. Without this change nameservers from DHCP will be added to the list of available nameservers.

CAPV v1.5+ was recently bumped, which contains the ability to configure DHCP overrides, so this change can be validated

Which issue(s) this PR fixes

Fixes #1103

Describe testing done for PR

Deployed clusters while using WORKER_NODE_NAMESERVERS and CONTROL_PLANE_NODE_NAMESERVERS and saw that CAPV was correctly configured with the VSphereMachineTemplate .spec.template.spec.network.dhcpOverrides.useDNS is set to false. SSH onto node vms and verfiy networkctl is configured correctly.

Release note

When WORKER_NODE_NAMESERVERS or CONTROL_PLANE_NODE_NAMESERVERS variables are set, nameservers from DHCP are ignored.

Additional information

Special notes for your reviewer

github-actions · 2022-11-28T22:47:19Z

Cluster Generation A/B Results:
https://storage.googleapis.com/tkg-clustergen/4023/20221128224717/clustergen.diff.txt
Author/reviewers:
Please review to verify that the effects on the generated cluster configurations are exactly what the PR intended, and give a thumbs-up if so.

codecov · 2022-11-29T00:11:28Z

Codecov Report

Merging #4023 (99a8d0f) into main (88865cb) will decrease coverage by 0.10%.
The diff coverage is n/a.

❗ Current head 99a8d0f differs from pull request most recent head a601b39. Consider uploading reports for the commit a601b39 to get more accurate results

@@            Coverage Diff             @@
##             main    #4023      +/-   ##
==========================================
- Coverage   48.53%   48.43%   -0.11%     
==========================================
  Files         446      434      -12     
  Lines       44110    43345     -765     
==========================================
- Hits        21410    20993     -417     
+ Misses      20657    20359     -298     
+ Partials     2043     1993      -50

Impacted Files	Coverage Δ
cli/runtime/config/clientconfig_factory.go	`60.52% <0.00%> (-13.16%)`	⬇️
cli/runtime/config/config_factory.go	`51.61% <0.00%> (-12.10%)`	⬇️
cli/runtime/config/lock.go	`54.28% <0.00%> (-6.69%)`	⬇️
tkg/client/common.go	`15.81% <0.00%> (-5.62%)`	⬇️
...cli/plugin-admin/codegen/generators/feature/gen.go	`38.09% <0.00%> (-3.94%)`	⬇️
cli/runtime/config/contexts.go	`57.82% <0.00%> (-3.57%)`	⬇️
cli/runtime/config/legacy_clientconfig.go	`31.37% <0.00%> (-3.12%)`	⬇️
addons/controllers/machine_controller.go	`65.65% <0.00%> (-3.04%)`	⬇️
...eaturegates/client/pkg/featuregateclient/client.go	`53.43% <0.00%> (-2.37%)`	⬇️
tkg/managementcomponents/helper.go	`88.35% <0.00%> (-1.95%)`	⬇️
... and 44 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

tylerschultz · 2022-12-01T00:51:04Z

/test install-vc7

github-actions · 2022-12-01T00:59:17Z

Cluster Generation A/B Results:
https://storage.googleapis.com/tkg-clustergen/4023/20221201005915/clustergen.diff.txt
Author/reviewers:
Please review to verify that the effects on the generated cluster configurations are exactly what the PR intended, and give a thumbs-up if so.

alfredthenarwhal · 2022-12-01T03:26:25Z

@tylerschultz: /test install-vc7
Commit: 9d8c52a

Build failed! Build no: 3418

christianang · 2022-12-01T18:13:35Z

/test install-vc7

github-actions · 2022-12-01T19:21:08Z

Cluster Generation A/B Results:
https://storage.googleapis.com/tkg-clustergen/4023/20221201192106/clustergen.diff.txt
Author/reviewers:
Please review to verify that the effects on the generated cluster configurations are exactly what the PR intended, and give a thumbs-up if so.

adobley · 2022-12-02T17:34:40Z

Looks like the install-vc7 tests passed, we should be good to go after a review

github-actions · 2022-12-06T18:39:37Z

Cluster Generation A/B Results:
https://storage.googleapis.com/tkg-clustergen/4023/20221206183936/clustergen.diff.txt
Author/reviewers:
Please review to verify that the effects on the generated cluster configurations are exactly what the PR intended, and give a thumbs-up if so.

adobley · 2022-12-07T18:26:22Z

/test install-vc7

github-actions · 2022-12-07T19:00:37Z

Cluster Generation A/B Results:
https://storage.googleapis.com/tkg-clustergen/4023/20221207190034/clustergen.diff.txt
Author/reviewers:
Please review to verify that the effects on the generated cluster configurations are exactly what the PR intended, and give a thumbs-up if so.

alfredthenarwhal · 2022-12-07T20:36:53Z

@adobley: /test install-vc7
Commit: a601b39

Build failed! Build no: 3501

tylerschultz · 2022-12-08T00:44:45Z

/test install-vc7

github-actions · 2022-12-08T18:57:40Z

Cluster Generation A/B Results:
https://storage.googleapis.com/tkg-clustergen/4023/20221208185737/clustergen.diff.txt
Author/reviewers:
Please review to verify that the effects on the generated cluster configurations are exactly what the PR intended, and give a thumbs-up if so.

tylerschultz · 2022-12-12T17:38:23Z

/test install-vc7

alfredthenarwhal · 2022-12-12T20:18:55Z

@tylerschultz: /test install-vc7
Commit: f809d42

Tests failed! Build no: 3561

adobley · 2023-01-03T21:44:24Z

/test install-vc7

github-actions · 2023-01-03T21:52:00Z

Cluster Generation A/B Results:
https://storage.googleapis.com/tkg-clustergen/4023/20230103215158/clustergen.diff.txt
Author/reviewers:
Please review to verify that the effects on the generated cluster configurations are exactly what the PR intended, and give a thumbs-up if so.

alfredthenarwhal · 2023-01-04T00:13:37Z

@adobley: /test install-vc7
Commit: 762860e

Build failed! Build no: 3726

adobley · 2023-01-05T00:39:14Z

/test install-vc7

alfredthenarwhal · 2023-01-05T03:36:36Z

@adobley: /test install-vc7
Commit: 762860e

Tests failed! Build no: 3748

adobley · 2023-01-05T18:11:27Z

/test install-vc7

github-actions · 2023-01-06T22:34:22Z

Cluster Generation A/B Results:
https://storage.googleapis.com/tkg-clustergen/4023/20230106223420/clustergen.diff.txt
Author/reviewers:
Please review to verify that the effects on the generated cluster configurations are exactly what the PR intended, and give a thumbs-up if so.

tylerschultz · 2023-01-06T22:34:38Z

/test install-vc7

adobley · 2023-01-11T19:42:17Z

It looks like the tests didn't run the last two times :(

adobley · 2023-01-11T19:42:28Z

/test install-vc7

alfredthenarwhal · 2023-01-11T22:10:10Z

@adobley: /test install-vc7
Commit: 4978716

Build failed! Build no: 3833

tylerschultz · 2023-01-17T23:44:42Z

/test install-vc7

github-actions · 2023-01-17T23:50:51Z

Cluster Generation A/B Results:
https://storage.googleapis.com/tkg-clustergen/4023/20230117235049/clustergen.diff.txt
Author/reviewers:
Please review to verify that the effects on the generated cluster configurations are exactly what the PR intended, and give a thumbs-up if so.

navidshaikh · 2023-01-18T19:04:10Z

/test install-vc7

alfredthenarwhal · 2023-01-18T21:39:54Z

@navidshaikh: /test install-vc7
Commit: b2e5267

Build failed! Build no: 3889

adobley · 2023-01-19T21:51:39Z

/test install-vc7

alfredthenarwhal · 2023-01-20T00:50:23Z

@adobley: /test install-vc7
Commit: b2e5267

Tests failed! Build no: 3905

adobley · 2023-01-20T17:39:43Z

/test install-vc7

adobley

LGTM

Co-authored-by: Tyler Schultz <tschultz@vmware.com> Co-authored-by: Edwin Xie <exie@vmware.com> Co-authored-by: Christian Ang <angc@vmware.com>

github-actions · 2023-01-26T18:09:41Z

Cluster Generation A/B Results:
https://storage.googleapis.com/tkg-clustergen/4023/20230126180940/clustergen.diff.txt
Author/reviewers:
Please review to verify that the effects on the generated cluster configurations are exactly what the PR intended, and give a thumbs-up if so.

imikushin

LGTM

This feature had been feature-flagged because it had unexpected behavior. Previously, when setting custom nameservers, they would not always apply becuase nameservers supplied by DHCP would 'compete' with the configured nameservers. Typically the OS would only apply 3 nameservers and ignoring the rest. This meant that often the addresses supplied by DHCP would take precedence over the configured nameservers. Recent changes, made in PR vmware-tanzu#4023, causes the OS to ignore nameservers supplied from DHCP when using the WORKER_NODE_NAMESERVERS or CONTROL_PLANE_NAMESERVERS variables. Now that the functionality for specifying custom nameservers works as expected, the feature flags can be removed. Co-authored-by: Edwin Xie <exie@vmware.com>

tylerschultz added the do-not-merge/wip Still work in progress label Nov 28, 2022

tylerschultz requested review from a team as code owners November 28, 2022 22:34

vmwclabot added the cla-not-required label Nov 28, 2022

tylerschultz force-pushed the dhcp-ignore-dns-when-providing-nameservers branch from b57dc06 to 9d8c52a Compare December 1, 2022 00:50

tylerschultz removed the do-not-merge/wip Still work in progress label Dec 1, 2022

tylerschultz changed the title ~~WIP: ignore dns servers from dhcp when providing nameservers~~ ignore dns servers from dhcp when providing nameservers Dec 1, 2022

flawedmatrix force-pushed the dhcp-ignore-dns-when-providing-nameservers branch from 9d8c52a to 07dba41 Compare December 1, 2022 19:11

christianang approved these changes Dec 2, 2022

View reviewed changes

flawedmatrix force-pushed the dhcp-ignore-dns-when-providing-nameservers branch from 07dba41 to a601b39 Compare December 6, 2022 18:30

tylerschultz force-pushed the dhcp-ignore-dns-when-providing-nameservers branch from a601b39 to 0896e22 Compare December 7, 2022 18:49

flawedmatrix force-pushed the dhcp-ignore-dns-when-providing-nameservers branch from 0896e22 to f809d42 Compare December 8, 2022 18:48

tylerschultz force-pushed the dhcp-ignore-dns-when-providing-nameservers branch from f809d42 to 4c47eb4 Compare December 13, 2022 00:23

tylerschultz force-pushed the dhcp-ignore-dns-when-providing-nameservers branch from 98dd506 to 762860e Compare January 3, 2023 21:43

tylerschultz force-pushed the dhcp-ignore-dns-when-providing-nameservers branch from 762860e to 4978716 Compare January 6, 2023 22:27

flawedmatrix force-pushed the dhcp-ignore-dns-when-providing-nameservers branch from 4978716 to b2e5267 Compare January 17, 2023 23:44

adobley approved these changes Jan 20, 2023

View reviewed changes

ignore dns servers from dhcp when providing nameservers

cd44e89

Co-authored-by: Tyler Schultz <tschultz@vmware.com> Co-authored-by: Edwin Xie <exie@vmware.com> Co-authored-by: Christian Ang <angc@vmware.com>

tylerschultz force-pushed the dhcp-ignore-dns-when-providing-nameservers branch from b2e5267 to cd44e89 Compare January 26, 2023 18:03

imikushin approved these changes Jan 27, 2023

View reviewed changes

imikushin added the ok-to-merge PRs should be labelled with this before merging label Jan 27, 2023

tylerschultz merged commit 49aa1e6 into vmware-tanzu:main Jan 27, 2023

tylerschultz deleted the dhcp-ignore-dns-when-providing-nameservers branch January 27, 2023 21:56

ignore dns servers from dhcp when providing nameservers #4023

ignore dns servers from dhcp when providing nameservers #4023

Conversation

tylerschultz commented Nov 28, 2022 • edited Loading

What this PR does / why we need it

Which issue(s) this PR fixes

Describe testing done for PR

Release note

Additional information

Special notes for your reviewer

github-actions bot commented Nov 28, 2022

codecov bot commented Nov 29, 2022 • edited Loading

Codecov Report

tylerschultz commented Dec 1, 2022

github-actions bot commented Dec 1, 2022

alfredthenarwhal commented Dec 1, 2022

christianang commented Dec 1, 2022

github-actions bot commented Dec 1, 2022

adobley commented Dec 2, 2022

github-actions bot commented Dec 6, 2022

adobley commented Dec 7, 2022

github-actions bot commented Dec 7, 2022

alfredthenarwhal commented Dec 7, 2022

tylerschultz commented Dec 8, 2022

github-actions bot commented Dec 8, 2022

tylerschultz commented Dec 12, 2022

alfredthenarwhal commented Dec 12, 2022

adobley commented Jan 3, 2023

github-actions bot commented Jan 3, 2023

alfredthenarwhal commented Jan 4, 2023

adobley commented Jan 5, 2023

alfredthenarwhal commented Jan 5, 2023

adobley commented Jan 5, 2023

github-actions bot commented Jan 6, 2023

tylerschultz commented Jan 6, 2023

adobley commented Jan 11, 2023

adobley commented Jan 11, 2023

alfredthenarwhal commented Jan 11, 2023

tylerschultz commented Jan 17, 2023

github-actions bot commented Jan 17, 2023

navidshaikh commented Jan 18, 2023

alfredthenarwhal commented Jan 18, 2023

adobley commented Jan 19, 2023

alfredthenarwhal commented Jan 20, 2023

adobley commented Jan 20, 2023

adobley left a comment

Choose a reason for hiding this comment

github-actions bot commented Jan 26, 2023

imikushin left a comment

Choose a reason for hiding this comment

tylerschultz commented Nov 28, 2022 •

edited

Loading

codecov bot commented Nov 29, 2022 •

edited

Loading