Skip to content

fix: isoPath as absolute path#247

Merged
lbajolet-hashicorp merged 1 commit intomainfrom
fix/relative-paths
Sep 9, 2024
Merged

fix: isoPath as absolute path#247
lbajolet-hashicorp merged 1 commit intomainfrom
fix/relative-paths

Conversation

@tenthirtyam
Copy link
Copy Markdown
Collaborator

@tenthirtyam tenthirtyam commented Sep 6, 2024 •
edited
Loading

Summary

This modification ensures that both isoPath is an absolute path.

Testing

Basic:

packer-plugin-vmware on  fix/relative-paths via 🐹 v1.23.0 
⇣8% ➜ go fmt ./...

packer-plugin-vmware on  fix/relative-paths via 🐹 v1.23.0 
⇣8% ➜ make build

packer-plugin-vmware on  fix/relative-paths via 🐹 v1.23.0 
⇣8% ➜ make test
?       github.com/hashicorp/packer-plugin-vmware       [no test files]
?       github.com/hashicorp/packer-plugin-vmware/version       [no test files]
ok      github.com/hashicorp/packer-plugin-vmware/builder/vmware/common 6.632s
ok      github.com/hashicorp/packer-plugin-vmware/builder/vmware/iso    1.766s
ok      github.com/hashicorp/packer-plugin-vmware/builder/vmware/vmx    2.182s

E2E:

  iso_checksum     = var.iso_checksum
  iso_url          = var.iso_url
  iso_target_path  = "./Downloads/" <--- using relative path.
✦6 ➜ packer build -var-file=ubuntu-24.04-aarch64.pkrvars.hcl .
vmware-iso.vm: output will be in this color.

==> vmware-iso.vm: Retrieving ISO
==> vmware-iso.vm: Trying https://cdimage.ubuntu.com/releases/noble/release/ubuntu-24.04.1-live-server-arm64.iso
==> vmware-iso.vm: Trying https://cdimage.ubuntu.com/releases/noble/release/ubuntu-24.04.1-live-server-arm64.iso?checksum=sha256%3A5ceecb7ef5f976e8ab3fffee7871518c8e9927ec221a3bb548ee1193989e1773
    vmware-iso.vm: ubuntu-24.04.1-live-server-arm64.iso 2.32 GiB / 2.32 GiB [=================] 100.00% 1m20s
==> vmware-iso.vm: https://cdimage.ubuntu.com/releases/noble/release/ubuntu-24.04.1-live-server-arm64.iso?checksum=sha256%3A5ceecb7ef5f976e8ab3fffee7871518c8e9927ec221a3bb548ee1193989e1773 => ./Downloads/a26d6549df299d1b04dcda5352d588f8b034ee1a.iso
==> vmware-iso.vm: Configuring output and export directories...
==> vmware-iso.vm: Creating temporary RSA SSH key for instance...
==> vmware-iso.vm: Creating virtual machine disks...
==> vmware-iso.vm: Generating the .vmx file...
==> vmware-iso.vm: Starting HTTP server on port 8361
==> vmware-iso.vm: Powering on virtual machine...
==> vmware-iso.vm: Connecting to VNC...
==> vmware-iso.vm: Waiting 5s for boot...
==> vmware-iso.vm: Typing the boot command over VNC...
==> vmware-iso.vm: Waiting for SSH to become available...
==> vmware-iso.vm: Connected to SSH!
==> vmware-iso.vm: Uploading VMware Tools (linux)...
==> vmware-iso.vm: Gracefully halting virtual machine...
    vmware-iso.vm: Waiting for clean up...
==> vmware-iso.vm: Deleting unnecessary files...
    vmware-iso.vm: Deleting: ../builds/build_files/packer-ubuntu-24.04-aarch64-vmware/startMenu.plist
    vmware-iso.vm: Deleting: ../builds/build_files/packer-ubuntu-24.04-aarch64-vmware/ubuntu-24.04-aarch64.plist
    vmware-iso.vm: Deleting: ../builds/build_files/packer-ubuntu-24.04-aarch64-vmware/ubuntu-24.04-aarch64.scoreboard
    vmware-iso.vm: Deleting: ../builds/build_files/packer-ubuntu-24.04-aarch64-vmware/vmware.log
==> vmware-iso.vm: Compacting all attached virtual disks...
    vmware-iso.vm: Compacting virtual disk 1
==> vmware-iso.vm: Cleaning VMX prior to finishing up...
    vmware-iso.vm: Detaching ISO from CD-ROM device sata0:1...
    vmware-iso.vm: Disabling VNC server...
    vmware-iso.vm: Removing Ethernet Interfaces...
==> vmware-iso.vm: Skipping export of virtual machine...
Build 'vmware-iso.vm' finished after 4 minutes 34 seconds.

==> Wait completed after 4 minutes 34 seconds

==> Builds finished. The artifacts of successful builds are:
--> vmware-iso.vm: VM files in directory: ./../builds/build_files/packer-ubuntu-24.04-aarch64-vmware

Notice:

==> vmware-iso.vm: https://cdimage.ubuntu.com/releases/noble/release/ubuntu-24.04.1-live-server-arm64.iso?checksum=sha256%3A5ceecb7ef5f976e8ab3fffee7871518c8e9927ec221a3bb548ee1193989e1773 => ./Downloads/a26d6549df299d1b04dcda5352d588f8b034ee1a.iso

And the VMX includes the following during the build:

sata0.present = "TRUE"
sata0:0.filename = "disk.vmdk"
sata0:0.present = "TRUE"
sata0:1.devicetype = "cdrom-image"
sata0:1.filename = "/Users/ryan/Library/Mobile Documents/com~apple~CloudDocs/Code/Personal/test/packer_templates/Downloads/a26d6549df299d1b04dcda5352d588f8b034ee1a.iso"
sata0:1.present = "TRUE"

Reference

Closes #25

@tenthirtyam tenthirtyam added this to the v1.1.1 milestone Sep 6, 2024
@tenthirtyam tenthirtyam self-assigned this Sep 6, 2024
@tenthirtyam tenthirtyam changed the title fix: relative paths fix: isoPath as absolute path Sep 7, 2024
@tenthirtyam
fix: isoPath as absolute path
ea0000a 
This modification ensures that both `isoPath` is an absolute path.

Ref: #25

Signed-off-by: Ryan Johnson <ryan@tenthirtyam.org>
@tenthirtyam tenthirtyam marked this pull request as ready for review September 7, 2024 04:36
@tenthirtyam tenthirtyam requested a review from a team as a code owner September 7, 2024 04:36
@tenthirtyam tenthirtyam mentioned this pull request Sep 7, 2024
Closed
lbajolet-hashicorp
lbajolet-hashicorp approved these changes Sep 9, 2024
View reviewed changes
Copy link
Copy Markdown
Contributor

@lbajolet-hashicorp lbajolet-hashicorp left a comment

Choose a reason for hiding this comment

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

Thanks for the PR @tenthirtyam, looks straightforward enough to me, I'll merge this ASAP.

I do have one question/suggestion: I wonder if we can do this upstream, when registering iso_path. If we register the iso_path as an absolute path in the first place, we can fail fast (I'm assuming this is done during Prepare?), so the path can be corrected quickly and we don't need to call filepath.Abs at this step.
But fundamentally this doesn't need to be done in this PR, it can also be rolled later, let me know how you want to proceed with this suggestion, and if you think it's irrelevant or you want to address it later, we can merge this today.

@tenthirtyam
Copy link
Copy Markdown
Collaborator Author

tenthirtyam commented Sep 9, 2024

@lbajolet-hashicorp - Yes, indeed! I'll raise this with the desktop hypervisor engineering team and reference this pull request. Likely something that should be considered for out-of-the-box support.

@lbajolet-hashicorp
Copy link
Copy Markdown
Contributor

lbajolet-hashicorp commented Sep 9, 2024

I realise my intent with upstream is misleading; while it would make sense for the upstream dependency to be updated, my suggestion was mostly to change how iso_path is registered in the builder's state, looking at the code it seems that the iso_path is registered as-is from the configuration, my suggestion is that we do the conversion to an absolute path before we register it as part of the state.
Let me know if that's not clear, we can push that discussion off-band if that's simpler too.

@tenthirtyam
Copy link
Copy Markdown
Collaborator Author

tenthirtyam commented Sep 9, 2024

Roger that, Lucas. I got you now and will make a note in my backlog list as the tech-debt is addressed.

If we can move on with merging this one here to resolve the issue and address how it in the state next.

@lbajolet-hashicorp
Copy link
Copy Markdown
Contributor

lbajolet-hashicorp commented Sep 9, 2024

Yeah that makes sense, we can proceed with the current fix and revisit later, no objection on my part!

Merging now!

@lbajolet-hashicorp lbajolet-hashicorp merged commit 448c054 into main Sep 9, 2024
@lbajolet-hashicorp lbajolet-hashicorp deleted the fix/relative-paths branch September 9, 2024 15:20
@tenthirtyam tenthirtyam mentioned this pull request Feb 21, 2025
Closed
@tenthirtyam tenthirtyam linked an issue Feb 21, 2025 that may be closed by this pull request
iso_target_path option fails to connect cd device #276
Closed
@github-actions
Copy link
Copy Markdown

github-actions bot commented Jan 31, 2026

I'm going to lock this pull request because it has been closed for 30 days. This helps our maintainers find and focus on the active issues.

If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jan 31, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

1 more reviewer

@lbajolet-hashicorp lbajolet-hashicorp lbajolet-hashicorp approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@tenthirtyam tenthirtyam

Labels

None yet

Projects

None yet

Milestone

v1.2.0

Development

Successfully merging this pull request may close these issues.

iso_target_path option fails to connect cd device Packer understands relative ISO paths, VMware doesn't

2 participants

@tenthirtyam @lbajolet-hashicorp