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

Even an isolated network has dhcp #767

Merged
merged 1 commit into from
Aug 17, 2020
Merged

Conversation

ngyuki
Copy link
Contributor

@ngyuki ngyuki commented Aug 10, 2020

Fix #500

libvirt's isolated network can have dhcp.

There is no forward element in this case.

Copy link
Collaborator

@MalloZup MalloZup left a comment

Choose a reason for hiding this comment

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

seems ok to me. I will need to search a bit more on this. thx for PR!

@MalloZup
Copy link
Collaborator

@mrostecki how does it look for you this one networking speaking?

Copy link
Contributor

@vadorovsky vadorovsky left a comment

Choose a reason for hiding this comment

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

@MalloZup @ngyuki Looks good to me as well. Sorry for late reaction!

@MalloZup MalloZup merged commit d263aa9 into dmacvicar:master Aug 17, 2020
@MalloZup
Copy link
Collaborator

@ngyuki I tried locally to run after this change the testacc and they are failing after this PR.

TestAccLibvirtDomain_NetworkInterface
this test namely fails.

Can you try to run: make testacc? https://github.com/dmacvicar/terraform-provider-libvirt/blob/master/CONTRIBUTING.md#running-the-tests-testacc

@ngyuki
Copy link
Contributor Author

ngyuki commented Aug 17, 2020

@MalloZup

Hmm, testacc in TestAccLibvirtDomain_NetworkInterface seems to be OK on locally of me.

# git log -1
commit d263aa9398eb78521ab4c4939efe53e23adc8681 (HEAD -> master, origin/master, origin/HEAD)
Merge: ba579168 1d6e05d6
Author: Dario Maiocchi <MalloZup@users.noreply.github.com>
Date:   Mon Aug 17 15:43:11 2020 +0200

    Merge pull request #767 from ngyuki/master

    Even an isolated network has dhcp

# make testacc TEST_ARGS="-run TestAccLibvirtDomain_NetworkInterface"
./travis/run-tests-acceptance -run TestAccLibvirtDomain_NetworkInterface
+ go test -v -covermode=count -coverprofile=profile.cov -timeout=1200s -run TestAccLibvirtDomain_NetworkInterface ./libvirt
=== RUN   TestAccLibvirtDomain_NetworkInterface
--- PASS: TestAccLibvirtDomain_NetworkInterface (46.77s)
PASS
coverage: 27.7% of statements
ok      github.com/dmacvicar/terraform-provider-libvirt/libvirt 46.792s coverage: 27.7% of statements

@MalloZup
Copy link
Collaborator

@ngyuki hi thx for feedback. I will try to test a bit.

can you run on your site the whole testsuite and let me know if you find any errors locally? In travis we skip 2/3 tests

TIA

@ngyuki
Copy link
Contributor Author

ngyuki commented Aug 18, 2020

@MalloZup
When run all tests on my locally, then only TestAccLibvirtDomainConsoles failed.

# make testacc
...snip...

=== RUN   TestAccLibvirtDomainConsoles
    testing.go:635: Step 0 error: errors during apply:

        Error: Error creating libvirt domain: virError(Code=1, Domain=10, Message='internal error: process exited while connecting to monitor: 2020-08-18T06:36:28.096092Z qemu-system-x86_64: -chardev tty,id=charserial0,path=/dev/pts/1: Could not open '/dev/pts/1': Input/output error')

          on /tmp/tf-test142194802/main.tf line 2:
          (source code not available)


--- FAIL: TestAccLibvirtDomainConsoles (0.19s)

...snip...

But, this fail even if revert my PR.

# git revert --no-edit -m 1 @
[master 277e8de7] Revert "Merge pull request #767 from ngyuki/master"
 Date: Tue Aug 18 16:11:26 2020 +0900
 2 files changed, 2 insertions(+), 5 deletions(-)

# git log -1
commit 277e8de736a4dd7b984a500a39a5e14c75789ceb (HEAD -> master)
Author: Toshiyuki Goto <ngyuki.jp@gmail.com>
Date:   Tue Aug 18 16:11:26 2020 +0900

    Revert "Merge pull request #767 from ngyuki/master"

    This reverts commit d263aa9398eb78521ab4c4939efe53e23adc8681, reversing
    changes made to ba5791683ecb0c1822c2c541bda16b3b49f2319c.

# make testacc TEST_ARGS="-run TestAccLibvirtDomainConsoles"
...snip...

=== RUN   TestAccLibvirtDomainConsoles
    testing.go:635: Step 0 error: errors during apply:

        Error: Error creating libvirt domain: virError(Code=1, Domain=10, Message='internal error: process exited while connecting to monitor: 2020-08-18T07:12:46.066777Z qemu-system-x86_64: -chardev tty,id=charserial0,path=/dev/pts/2: Could not open '/dev/pts/2': Input/output error')

          on /tmp/tf-test828471238/main.tf line 2:
          (source code not available)


--- FAIL: TestAccLibvirtDomainConsoles (0.30s)

...snip...

@MalloZup
Copy link
Collaborator

@ngyuki thank you for verification. Ok that sounds ok to me then. !

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.

Can not assign address to interface connected to isolated network
3 participants