hashicorp / packer-plugin-vsphere

Packer plugin for VMware vSphere Builder
https://www.packer.io/docs/builders/vsphere
Mozilla Public License 2.0
93 stars 91 forks source link

error removing cdrom prior to reattaching: invalid number: n must be between 0 and 0 #393

Closed StephenSo closed 3 months ago

StephenSo commented 4 months ago

Overview of the Issue

Packer build for a given Windows server build no longer works. I believe the 1.2.5 refactoring release created this issue, it only became apparent once #386 was corrected. Now the build finishes, with the error

==> windows2022.vsphere-iso.windows2022: Reattaching CD-ROM devices...
==> windows2022.vsphere-iso.windows2022: error removing cdrom prior to reattaching: invalid number: n must be between 0 and 0
==> windows2022.vsphere-iso.windows2022: Step "StepReattachCDRom" failed
==> windows2022.vsphere-iso.windows2022: [c] Clean up and exit, [a] abort without cleanup, or [r] retry step (build may fail even if retry succeeds)? a
==> windows2022.vsphere-iso.windows2022: error removing cdrom prior to reattaching: invalid number: n must be between 0 and 0
==> windows2022.vsphere-iso.windows2022: Step "StepReattachCDRom" failed, aborting...

Version 1.2.4 and older: D: = Windows Install media ISO E: = VMware Tools ISO

Version 1.2.6: D: = VMware Tools ISO E: = Windows Install media ISO

Reproduction Steps

Works as per previous version: KO Packer build where

packer {
  required_plugins {
    ansible = {
      version = ">= 1.1.1"
      source  = "github.com/hashicorp/ansible"
    }
    vsphere = { # added due to https://www.hashicorp.com/blog/announcing-the-removal-of-bundled-plugins-in-hashicorp-packer
      version = "= 1.2.4"
      source  = "github.com/hashicorp/vsphere"
    }
  }
}

Change to 1.2.6 - Build will timeout waiting for VMTools to install

packer {
  required_plugins {
    ansible = {
      version = ">= 1.1.1"
      source  = "github.com/hashicorp/ansible"
    }
    vsphere = { # added due to https://www.hashicorp.com/blog/announcing-the-removal-of-bundled-plugins-in-hashicorp-packer
      version >= "= 1.2.6"
      source  = "github.com/hashicorp/vsphere"
    }
  }
}

Packer Version

1.8.3

Plugin Version and Builders

Please provide the plugin version.

1.2.6

Please select the builder.

VMware vSphere Version

Please provide the VMware vSphere version.

vCenter Server 7.0 Update 3o VMware ESXi 7.0 Update 3g

Guest Operating System

Windows 2022 & 2019

Simplified Packer Buildfile

source "vsphere-iso" "windows2022" {
  // https://github.com/vmware-samples/packer-examples-for-vsphere/blob/main/builds/windows/server/2016/windows-server.pkr.hcl
....
  storage {
      disk_size       = var.windows2022_vm_disk_sizeMB
      disk_controller_index = 0      
      disk_thin_provisioned = true
  }
  network_adapters {
      network         = var.vcenter_network
      network_card    = "vmxnet3"
  }
  cdrom_type          = var.windows2022_vm_cdrom_type
  remove_cdrom        = true
  reattach_cdroms     = 1

  // vSphere template settings
  convert_to_template = true
  export {
    force = true
    output_directory = var.ovfFile_location
  }
  create_snapshot     = false
  // OS Configuration
  guest_os_type       = "${var.windows2022_vm_guest_os_type}"
  notes               = "Default User: ${var.winrm_username}\nBuilt by Packer @ {{isotime \"2006-01-02 03:04\"}}."

  // Removable Media Settings
  iso_url             = "${var.windows_artifcatory}${var.windows2022_iso}"
  iso_target_path     = "${var.PACKER_CACHE_DIR}/${var.windows2022_iso}"
  iso_checksum        = var.windows2022_checksum
  iso_paths    = ["[] /vmimages/tools-isoimages/${var.windows2022_os_family}.iso"]
  floppy_files = [
    "${var.os_config_root}/${var.windows2022_os_family}/${var.windows2022_scripts_folder}/"
  ]
  floppy_content = {
    "autounattend.xml" = templatefile("${var.os_config_root}/${var.windows2022_os_family}/${var.windows2022_scripts_folder}/${var.windows2022_vm_firmware}_autounattend.pkrtpl.hcl", {
      vm_guest_os_language = var.windows2022_vm_guest_os_language
      vm_guest_os_keyboard = var.windows2022_vm_guest_os_keyboard
      vm_guest_os_timezone = var.windows2022_vm_guest_os_timezone
    })
  }

  // Boot and Provisioning Settings
  // http_port_min    = var.common_http_port_min
  // http_port_max    = var.common_http_port_max
  boot_order       = var.windows2022_vm_boot_order
  boot_wait        = var.windows2022_vm_boot_wait
  boot_command     = var.windows2022_vm_boot_command

Set the env var PACKER_LOG=1 for maximum log detail.

tenthirtyam commented 4 months ago

@Hi-Angel to triage.

Hi-Angel commented 4 months ago

Oh, I see, thank you. I'll have to dig into it, so I'll probably take a look into it tomorrow or on Friday at worst.

Hi-Angel commented 4 months ago

Took a quick look into it. It's a funny bug: the reason the problem happens is because in step_reattach_cdrom.go we expect the number of cdroms to be equal to s.CDRomConfig.ISOPaths. However, it does not take into account the existence of remove_cdrom = true HCL option which would remove remove cdroms, thus making this assumption invalid.

Possible solutions I see:

  1. Get the actual number of CDroms from the API
  2. In step_reattach_cdrom.go take into account remove_cdrom
  3. In remove_cdrom processing take into account reattach_cdroms

1 seems like the best one, except Idk offhand what API should I use and wouldn't it make request over the network (which isn't good).

2 I tried and there does not seem to be access to remove_cdrom value in step_reattach_cdrom.go file.

3 I didn't try yet, but I presume the problem would be similar to 2 (i.e. having to access to reattach_cdroms in wherever the remove_cdrom is processed).

Hi-Angel commented 4 months ago

Okay, got a fix locally by adding a function enlisting cdrom devices, which is the best way to handle that I think. But it doesn't quite work + I guess I need to add tests for that case as well, so it will take some more time.

Hi-Angel commented 4 months ago

@StephenSo thank you for finding the bug! Its fix is being reviewed at https://github.com/hashicorp/packer-plugin-vsphere/pull/394

That said, I just wanted to mention that you can make it work without waiting for the fix to get merged by removing the remove_cdrom line from the config. I don't see the point of removing the cdroms if you want them immediately to be readded. Reattached cdroms should have no flies attached to them in case that's what you were afraid of. As it stands, the option is just excess code for you to read and for your server to execute 😊

StephenSo commented 4 months ago

Thanks for the update. During the build there's 2 cdroms in use (1 for the OS and the other for VM tools), where only 1 is required in the VM template. In the docs, maybe I misunderstood the process. Remove cdroms and then keep only 1 for final config. So the correct process is to not remove cdroms and just use reattach_cdroms? It is confusing. Perhaps the variable should simply be retain_cdroms = # (to specify how many to be left in the template) ?

On Fri, 22 Mar 2024, 12:27 Konstantin K, @.***> wrote:

@StephenSo https://github.com/StephenSo thank you for finding the bug! Its fix is being reviewed at #394 https://github.com/hashicorp/packer-plugin-vsphere/pull/394

That said, I just wanted to mention that you can make it work without waiting for the fix to get merged by removing the remove_cdrom line from the config. I don't see the point of removing the cdroms if you want them immediately to be readded. Reattached cdroms should have no flies attached to them in case that's what you were afraid of. As it stands, the option is just excess code for you to read and for your server to execute 😊

β€” Reply to this email directly, view it on GitHub https://github.com/hashicorp/packer-plugin-vsphere/issues/393#issuecomment-2014972022, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABIKYKZPJUN2HH35CWFAYZTYZQPUFAVCNFSM6AAAAABE7GC42SVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDAMJUHE3TEMBSGI . You are receiving this because you were mentioned.Message ID: @.***>

Hi-Angel commented 4 months ago

Thanks for the update. During the build there's 2 cdroms in use (1 for the OS and the other for VM tools), where only 1 is required in the VM template. In the docs, maybe I misunderstood the process. Remove cdroms and then keep only 1 for final config. So the correct process is to not remove cdroms and just use reattach_cdroms? It is confusing.

Well, I do not seem to see at all reattach_cdrom being documented on the hashicorp site. I think they haven't uploaded the docs last few releases. For now there's documentation in the repo and it says, quoting:

When set to a value in the range (1..4, remark is mine), remove_cdrom is ignored and the CD-ROM devices are kept without any attached media.

So the interaction with remove_cdrom is documented and it says the option is just ignored (although in the code it actually isn't, which resulted in this bug, but as far as a user concerned there's no visible difference in the option behavior when reattach_cdrom is set as well).

Perhaps the variable should simply be retain_cdroms = # (to specify how many to be left in the template) ?

I think retain would be misleading, because it implies that some amount of cdroms would see no changes. Which is not true, because the code does some change: specifically, it deattaches ISO files from them.

StephenSo commented 4 months ago

Thanks @Hi-Angel, I will remove the use of remove_cdrom from my code. I will test and revert if it breaks the build.