dmacvicar / terraform-provider-libvirt

Terraform provider to provision infrastructure with Linux's KVM using libvirt
Apache License 2.0
1.59k stars 458 forks source link

Update libvirt-go to 7.0.0 #836

Closed nnooney closed 3 years ago

nnooney commented 3 years ago

The current version (5.1.0) does not compile on 32-bit architectures such as the Raspberry Pi. Updating the dependency to the latest version and tweaking build errors allows the unit tests to pass.

Please make sure you read the contributor documentation before opening a Pull Request.

MalloZup commented 3 years ago

thx @nnooney can you rebase commits?

nnooney commented 3 years ago

Rebase complete.

MalloZup commented 3 years ago

thx @nnooney for your work.

Just to update you on what is going on recently on the project.

We plan to migrate to a new libvirt library. https://github.com/dmacvicar/terraform-provider-libvirt/pull/813

this will allow to fix other issue we had, mainly supporting the new release model of terraform registry, more details here: https://github.com/dmacvicar/terraform-provider-libvirt/issues/747

Now I'm not strictly against this change but we are bumping the bindings from 2 major version here from 5 to 7.

As maintainer I would like to have the certitude we don't introduce any breaking change to the existing user of the project.

Could you perhaps post and share your findings what has been changed between these version so we could evaluate that?

Thanks for help and contribution! :sunflower:

nnooney commented 3 years ago

Thanks for the update about the direction terraform-provider-libvirt is heading. I definitely agree that switching libraries to allow for integration with the terraform registry is the best path forward. This PR is primarily motivated so that I can compile the provider for ARM 32bit architectures (Raspberry Pi).

Thankfully the libvirt-go library mentions that they keep their API stable across major versions (examining their README at https://gitlab.com/libvirt/libvirt-go). Unfortunately, I could not find a changelog on their project. I examined the diff between the two releases (https://gitlab.com/libvirt/libvirt-go/-/compare/v5.1.0...v7.0.0) and here's what I found:

It seems that they've added a lot of features to support the libvirt API that could be adopted by this project if desired. Not much was changed/removed except for the Connect functions and flag types.

z3ntu commented 3 years ago

Bump, I also see libvirt-go failing to compile on 32-bit arches while packaging for a distro. With this patch it builds fine.

dmacvicar commented 3 years ago

master does not depend on libvirt-go.

Please check https://github.com/dmacvicar/terraform-provider-libvirt/pull/859#issuecomment-870103094

Pre-release is available here.