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

feat(vm): add support for "args" flag for VM #205

Merged
merged 3 commits into from
Jan 15, 2023
Merged

feat(vm): add support for "args" flag for VM #205

merged 3 commits into from
Jan 15, 2023

Conversation

kaje783
Copy link
Contributor

@kaje783 kaje783 commented Jan 12, 2023

Community Note

  • Please vote on this pull request by adding a 👍 reaction to the original pull request comment to help the community and maintainers prioritize this request
  • Please do not leave "+1" or other comments that do not add relevant new information or questions, they generate extra noise for pull request followers and do not help prioritize the request

Relates OR Closes #0000

Example to deploy OpenSuse MicroOS and setup it with Ingition

  1. You need first a template and a qcow2 image:
mkdir -p /root/download/
cd /root/download/
wget https://download.opensuse.org/tumbleweed/appliances/openSUSE-MicroOS.x86_64-ContainerHost-kvm-and-xen.qcow2

qm create 1001 --name opensuse-microos --memory 4098 --cores 4
qm importdisk 1001 /root/download/openSUSE-MicroOS.x86_64-ContainerHost-kvm-and-xen.qcow2 local
qm set 1001 --scsihw virtio-scsi-pci --virtio0 local:vm-1001-disk-0 --serial0 socket --boot c --bootdisk virtio0 --agent 1 --vga qxl

  1. enable snippets for local storage

  2. terraform template:

terraform {
	required_providers {
        proxmox = {
          source = "bpg/proxmox"
          version = "0.9.1-1"
        }
        ignition = {
          source = "community-terraform-providers/ignition"
          version = "2.1.3"
        }
	}
}

provider "proxmox" {
  virtual_environment {
    endpoint = var.pve.address
    username = var.pve.username
    password = var.pve.password
    insecure = var.pve.insecure
  }
}


data "ignition_systemd_unit" "hello_world" {
  name = "hello_world.service"
  content = "[Service]\nType=oneshot\nExecStart=/usr/bin/echo Hello World\n\n[Install]\nWantedBy=multi-user.target"
}

data "ignition_user" "root" {
    name = "root"
    password_hash = "$6$rounds=4096$yWKn7iI7kTgxrXia$jHovsXMe1pYWwJKhzg.VheV6/RtVBFGVjbj/Q6Q5.Ck8fs14jaVwid7NJIe7n5caZFsITnb6EFbxDU.cK.ahE/"
}

data "ignition_config" "hello_world" {
    systemd = [
        data.ignition_systemd_unit.hello_world.rendered,
    ]

    users = [
        data.ignition_user.root.rendered,
    ]
}

resource "proxmox_virtual_environment_file" "ignition_hello_world" {
    content_type = "snippets"
    datastore_id = "local"
    node_name    = "pve01"

    source_raw {
        data = data.ignition_config.hello_world.rendered
        file_name = "ignition_hello_world.ign"
    }
}

resource "proxmox_virtual_environment_vm" "vms" {
    name                = "test-ignition"
    node_name           = "pve01"
    started             = true
    timeout_shutdown_vm = 10

    args = "-fw_cfg name=opt/com.coreos/config,file=/mnt/pve/local/snippets/ignition_hello_world.ign"

    clone  {
        vm_id = 1001
        retries = 20
    }    
}

Login with root and testtest

@product-auto-label product-auto-label bot added size: m 📖 documentation Improvements or additions to documentation labels Jan 12, 2023
@bpg bpg changed the title feat: add support for "args" flag for VM feat(vm): add support for "args" flag for VM Jan 13, 2023
Copy link
Owner

@bpg bpg left a comment

Choose a reason for hiding this comment

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

Hey @kaje783 👋🏻 Thanks for the PR!

Overall it looks great, but could you pls. address that linter warning about the duplicated struct field? I don't have a strong opinion about the name, either existing KVMArguments or new Args, but there should be only one :)

You can ignore other linter errors for now, I'm going to address most of them in #204

Thanks!

@@ -229,6 +229,7 @@ type VirtualEnvironmentVMCreateRequestBody struct {
ACPI *CustomBool `json:"acpi,omitempty" url:"acpi,omitempty,int"`
Agent *CustomAgent `json:"agent,omitempty" url:"agent,omitempty"`
AllowReboot *CustomBool `json:"reboot,omitempty" url:"reboot,omitempty,int"`
Args *string `json:"args,omitempty" url:"args,omitempty"`
Copy link
Owner

Choose a reason for hiding this comment

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

See the linter error below about KVMArguments -- that's actually a good catch, i wouldn't notice that 😅
I think we can totally reuse this field instead of the new one, but I leave it to you which one to keep.

@kaje783
Copy link
Contributor Author

kaje783 commented Jan 13, 2023

Hey @bpg , i move from args to kvmarguments.

I had to change the type of kvmarguments because the proxmox api expects a string.

As an alternative I created a second branch which takes a list as input. But there is a little problem there. For splitting I have to take " -" and add at the end a "-", for this problem there is surely a better solution:
Code Part

also update doc to match description from the official PVE documentation.
@bpg
Copy link
Owner

bpg commented Jan 15, 2023

Hey @bpg , i move from args to kvmarguments.

I had to change the type of kvmarguments because the proxmox api expects a string.

As an alternative I created a second branch which takes a list as input. But there is a little problem there. For splitting I have to take " -" and add at the end a "-", for this problem there is surely a better solution: Code Part

Great, thanks!
I think we're OK with having this just as a string, I don't see any need for parsing them for now.
I've also took the liberty to update the param name to kvm_arguments, just to align with other multi-word params.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
📖 documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants