Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Check resolv.conf file for updated --dns-server instead of guestinfo. #7824

Merged
merged 11 commits into from
Apr 27, 2018
Merged

Check resolv.conf file for updated --dns-server instead of guestinfo. #7824

merged 11 commits into from
Apr 27, 2018

Conversation

vburenin
Copy link
Contributor

@vburenin vburenin commented Apr 24, 2018

Ready for review.

Fixes #7775

[specific ci=6-16-Config]

@@ -179,6 +179,10 @@ END {
ensure_apt_packages() {
local install

if [ -x /usr/local/bin/xorriso -a -x /usr/bin/cpio ]; then
Copy link
Member

Choose a reason for hiding this comment

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

Why? We're passing in the package names we depend on to confirm and instead you've hardcoded a check for specific binaries.
I also see absolutely no correlation between this change and #7775

Copy link
Contributor Author

Choose a reason for hiding this comment

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

this is just to trigger CI, nothing else.

@vburenin vburenin dismissed hickeng’s stale review April 25, 2018 00:07

not relevant

@codecov-io
Copy link

codecov-io commented Apr 25, 2018

Codecov Report

Merging #7824 into master will decrease coverage by 0.88%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #7824      +/-   ##
==========================================
- Coverage   26.58%   25.69%   -0.89%     
==========================================
  Files          36       35       -1     
  Lines        5183     5125      -58     
==========================================
- Hits         1378     1317      -61     
- Misses       3699     3701       +2     
- Partials      106      107       +1
Impacted Files Coverage Δ
cmd/tether/attach.go 62.21% <0%> (-0.62%) ⬇️
cmd/vic-machine/common/completion.go

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 26ee2f9...7585121. Read the comment docs.

@vburenin vburenin changed the title [WIP] reproducing bug that happens on HAAS only Check resolv.conf file for updated --dns-server instead of guestinfo. Apr 25, 2018
Copy link
Contributor

@mhagen-vmware mhagen-vmware left a comment

Choose a reason for hiding this comment

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

lgtm, just one question

${rc} ${output}= Run And Return Rc and Output sshpass -p %{TEST_PASSWORD} ssh -o StrictHostKeyChecking=no root@%{VCH-IP} cat /etc/resolv.conf
Should Be Equal As Integers ${rc} 0
Should Not Contain ${output} nameserver 10.118.81.1
Should Not Contain ${output} nameserver 10.118.81.2
Copy link
Contributor

Choose a reason for hiding this comment

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

This too?

  • ${rc} ${output}= Run And Return Rc and Output sshpass -p %{TEST_PASSWORD} ssh -o StrictHostKeyChecking=no root@%{VCH-IP} cat /etc/resolv.conf | grep nameserver | wc -l
  • Should Be Equal As Integers ${rc} 0
  • Should Contain ${output} 2

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I thought about it, but since it is dependent on DHCP configuration the number of DNS servers may vary, so can't really use it here.

@vburenin vburenin merged commit 5b15145 into vmware:master Apr 27, 2018
mhagen-vmware pushed a commit that referenced this pull request May 16, 2018
… file for updated --dns-server instead of guestinfo. (#7824)

* Enable SSH and check /etc/resolv.conf directly for the appropriate config changes.
* DNS vic-machine test has been updated to look for the real data.
* Removed guest info check.
* Improved behavior with DHCP DNS server assignment. Ignore DHCP settings if DNS servers are set manually via --dns-server option.
* Added more logs to track which DHCP options are set.
mhagen-vmware added a commit that referenced this pull request May 16, 2018
…k resolv.conf file for updated --dns-server instead of guestinfo. (#7824) (#7957)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants