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
Add harvester provider support #1120
Conversation
Inspired by libvirt provider adapted to harvester resources. Until harvester/harvester#2255 is fixed not work in one go and after initial provisioning terraform needs to reapply to provision salt part.
- add memory conversion to bytes - remove ignition disk, always assume cloud init - instead of hardcoded additional network gateway, compute one - cosmetic fixess
Ready to review, thing to consider is that this duplicates list of images where both libvirt and harvester uses the same cloud-init images (except SLE Micro). So having this list unified should be something to look at. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just some small comments, overall look really good.
thanks for the great contribution
@@ -0,0 +1,9 @@ | |||
terraform { | |||
required_version = ">= 1.1.5" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we really need this terraform version? Currently, sumaform is tested to work with terraform version "1.0.10"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not really. I just use what is in tumbleweed. I will change it to 1.0.10
but can we at least leave >=
? My modus operandi with newly cloned sumaform is to switch all versions to the most recent one. I can't say about testsuite, but regular non-testsuite use works fine with the latest.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good for me, and it should work fine in there.
Also we should update terraform version again :)
@@ -0,0 +1,13 @@ | |||
terraform { | |||
required_version = ">= 1.1.5" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same comment about terraform version as before.
slemicro51o-ign = "${var.use_mirror_images ? "http://${var.mirror}" : "http://download.suse.de"}/install/SLE-Micro-5.1-GM/SUSE-MicroOS.x86_64-5.1.0-Default-GM.raw.xz" | ||
} | ||
network_name = lookup(var.provider_settings, "network_name", "default") | ||
bridge = lookup(var.provider_settings, "bridge", null) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this variable doesn't look to be used.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right! I will remove it.
That is something we can think of, on how to share this list. |
I no longer have access to harvester instance and cannot continue on this. Let's close it then. |
What does this PR change?
Inspired by libvirt provider adapted to harvester resources.
Until harvester/harvester#2255 is fixed does not work in one go and after initial provisioning terraform needs to reapply to provision salt part.Fixed and released as v0.5.0