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

fix(domain): update domain running state when refreshed #668

Merged
merged 1 commit into from
Feb 11, 2020

Conversation

Naewis
Copy link
Contributor

@Naewis Naewis commented Nov 21, 2019

System Information

Linux distribution

Debian GNU/Linux 10 (buster)

Terraform version

Terraform v0.12.12

Provider and libvirt versions

Compiled against library: libvirt 5.0.0
Using library: libvirt 5.0.0
Running hypervisor: QEMU 3.1.0
Running against daemon: 5.0.0

Checklist

Your issue is it a bug or something that does not work as expected.

Description of Issue

I'm currently using libvirt locally on my PC. Whenever I shutdown my computer, all the domains are stopped (which is correct, I don't wan't some tests domain to restart whenever I restart the host). Until then, all's good.

But when I want to restart my stack after a reboot, terraform says that there is nothing to do. As the domains are in stopped state, I expected terraform to put them online again (in order to get immutable infrastructure). I noticed that the running property doesn't get updated when terraform tries a refresh.

What did I do

I used the method domainIsRunning in order to get the domain state, and updated the schema running property. Tests were all good, but there seems to be a bug which is not related to this PR.


Additional information:

Apparmor is disabled.

First PR, and first GO code : sorry if I did something wrong in the way.
Please tell me in order to improve.

On a side note, there is also something wrong with the network state refresh.
When I ran the tests, i got this :

Error: Error creating libvirt domain: virError(Code=55, Domain=19, Message='Requested operation is not valid: network 'default' is not active')

$ sudo virsh net-list --all
 Name          State      Autostart   Persistent
--------------------------------------------------
 default       inactive   no          yes

@MalloZup
Copy link
Collaborator

@Naewis thx for PR. I will have look soonish 👍

@MalloZup MalloZup self-requested a review November 22, 2019 15:59
@MalloZup
Copy link
Collaborator

MalloZup commented Nov 23, 2019

related on same context : #622 (however 2 diff issues)

@Naewis
Copy link
Contributor Author

Naewis commented Nov 25, 2019

As far as I've tested, this patch fixes the issue #622, but the fix is not really elegant, I must agreed, as I know nothing of Golang (automatic test code seems too much at my current level)

Sadly, I have a little time to fix this issue anymore (as learning Go is not in my todo list for now, maybe later), but if I were fluent in Go, I would fix some issues by not re-reading domain global state when we create the resource (l. 569 : err = resourceLibvirtDomainRead(d, meta) ) but instead use a separate variable and re-inject explicitly only needed properties into our domain variable, because I don't know which property is actually erased implicitly vs not voluntarily erased (would also fix the state matter with more style)

@Naewis Naewis changed the title fix(domain): update domain running state when refresh fix(domain): update domain running state when refreshed Nov 25, 2019
@Naewis
Copy link
Contributor Author

Naewis commented Dec 11, 2019

Just to be sure, are you waiting for me to do something on this request ?

@MalloZup MalloZup merged commit a26f5b5 into dmacvicar:master Feb 11, 2020
@MalloZup
Copy link
Collaborator

thx @Naewis for pr and sorry for delay 🏓

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.

None yet

2 participants