hashicorp / packer-plugin-vsphere

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

feat: add support for resetting cd-rom #326

Closed powertim closed 9 months ago

powertim commented 9 months ago

I added a parameter named _keep_one_cdrom_ as boolean to permit the add of 1 SATA controller with 1 SATA CD-ROM drive after all drives are deleted.

Modifications are included in the step_remove_cdrom.go file:

if s.Config.KeepOneCdrom == true {
        if _, err := vm.FindSATAController(); err == driver.ErrNoSataController {
            ui.Say("Adding SATA controller...")
            if err := vm.AddSATAController(); err != nil {
                state.Put("error", err)
                return multistep.ActionHalt
            }
        }

        ui.Say("Adding SATA CD-ROM drive...")
        err2 := vm.AddCdrom("sata", "[] /usr/lib/vmware/isoimages/windows.iso")
        if err2 != nil {
            state.Put("error", err2)
            return multistep.ActionHalt
        }

        ui.Say("Ejecting ISO on SATA CD-ROM drive...")
        err3 := vm.EjectCdroms()
        if err3 != nil {
            state.Put("error", err3)
            return multistep.ActionHalt
        }
    }

Closes #24

hashicorp-cla commented 9 months ago

CLA assistant check
All committers have signed the CLA.

powertim commented 9 months ago

Hi, I've already signed the CLA... It's still blocked on this page. Feel free to contact me.

lbajolet-hashicorp commented 9 months ago

Hi @powertim,

Regarding the CLA problem, I believe this is because your commits have been pushed with an email that is not the one used for your Github account.

Can you change this in your commit chain? For the bot to be happy, all your commit should be authored with the email address linked to your Github account, then that should work out fine.

I'll review the code once you've had a chance to fix that, please let me know if you need more help.

Thanks for the contribution!

tenthirtyam commented 9 months ago

@powertim - could you rebase this from main, please.

Marking as draft.

tenthirtyam commented 9 months ago

@powertim I've got some code that's similar but:

  1. uses the same cdrom_type base on what's provided or the default if not
  2. removes the hardcoded vmtools path (not all host will have this)
  3. performs a configuration prior to running the build to fail early
  4. improved logging

I'm testing it tom make sure it works as expected based on the above.

If you can rebase this PR, I can cherry-pick your commits and then merge in mine some you still get credit.

powertim commented 9 months ago

@powertim I've got some code that's similar but:

  1. uses the same cdrom_type base on what's provided or the default if not
  2. removes the hardcoded vmtools path (not all host will have this)
  3. performs a configuration prior to running the build to fail early
  4. improved logging

I'm testing it tom make sure it works as expected based on the above.

If you can rebase this PR, I can cherry-pick your commits and then merge in mine some you still get credit.

Hi @tenthirtyam I've tried to make all things right. Could you confirm please ? Thanks and sorry for the "strange code" submitted on this PR, I was trying to make this feature quickly available on mainstream. Have a nice day,

Tim.

tenthirtyam commented 9 months ago

Yes, looks good I can cherry-pick this and merge my changes. i just want to run some negative test cases first.

tenthirtyam commented 9 months ago

Cherry-picked into #327.