(vsphere-iso) add ability to define multiple disks by jhawk28 · Pull Request #8787 · hashicorp/packer

jhawk28 · 2020-02-21T15:48:55Z

Left the old fields so that this is a backwards compatible change. Will create a separate pull request to remove deprecated fields.

Closes #8749

jhawk28 · 2020-02-21T16:47:30Z

@sylviamoss how do I kick off the build again? it looks like the one just took too long.

nywilken

Hi @jhawk28 this is off to a good start. Code wise it looks solid to me. I left a few comments around the documentation and the possibility of adding a new test case to the acceptance tests. I don't have much experience working with vsphere so I will defer to the approval from another maintainer who can test the changes.

builder/vsphere/iso/step_create.go

website/source/docs/builders/vsphere-iso.html.md.erb

builder/vsphere/driver/vm.go

jhawk28 · 2020-03-03T17:17:37Z

@azr @sylviamoss @SwampDragons Should I merge or rebase off master to get the latest? It looks like the check-generate is failing.

SwampDragons · 2020-03-03T20:59:50Z

I normally rebase.

builder/proxmox/config.go

Closes hashicorp#8749

SwampDragons · 2020-03-04T23:17:41Z

I was able to stand up a vsphere cluster to test today. This looks good and works as expected. Thanks for this contribution, and thanks for your patience!!

…atching Before change ``` 413e19b Merge pull request #8942 from desolatorxxl/google-fix-ssh-keys-metadata b81800d Merge pull request #8935 from zaventh/feature/start-on-boot 9486316 Merge pull request #8922 from hashicorp/f-vsphere_iso-export-ovf-options 56aebbe Merge pull request #8920 from rhencke/patch-1 d068430 make sure locals are evaluated only once variables are + test this (#8918) 3dae5df Merge pull request #8905 from hashicorp/fix_8493 811a730 Merge pull request #8907 from hashicorp/fix_8428 fa49d21 Merge pull request #8906 from hashicorp/fix_8904 23f5603 Merge pull request #8889 from hashicorp/hcl2_singular_blocks dc9259f Merge pull request #8892 from zaventh/feature/vga-adapter fc35f02 Merge pull request #8890 from hashicorp/fix_8880 7972ab7 Merge pull request #8735 from hashicorp/fix_plugin_loading 890d7b2 Merge pull request #8875 from hashicorp/fix_8812 e94ff70 Merge pull request #8883 from hashicorp/fix_8835 9075b80 Merge pull request #8891 from rhencke/patch-1 6477d8a Merge pull request #8882 from hashicorp/fix-var-file-hcl 6008f91 Merge pull request #8847 from takaishi/support-keyboard-interactive 5604561 Merge pull request #8877 from paulcichonski/remote-esxi-bastion 698f744 Merge pull request #8887 from hashicorp/untangle_ssh_docs_from_aws aeedc9a Merge pull request #8879 from mbrancato/specify_keyvault_sku 5365fda Merge pull request #8884 from hashicorp/fix_codecov_config 4bd7b14 Merge pull request #8732 from jhawk28/reorder_cdrom_drive 072a71b Merge pull request #8863 from hashicorp/update_go-cty_regex 8a1caaa Merge pull request #8837 from hashicorp/fix_8730 7873cab Merge pull request #8858 from hashicorp/fix_8791 7e382d0 Merge pull request #8828 from mvitaly/fix_8816 8832b3e Merge pull request #8787 from jhawk28/vsphere_iso_multiple_disks 5281740 Merge pull request #8831 from rjhornsby/master e35a872 Merge pull request #8830 from hashicorp/d-var-file-hcl2-not-yet ``` After change ``` ⇶ git log v1.5.4...v1.5.5 --first-parent --oneline --grep="Merge pull request #[0-9]\+" --grep="(#[0-9]\+)$" 413e19b Merge pull request #8942 from desolatorxxl/google-fix-ssh-keys-metadata c387dc2 builder/vsphere-clone: Find the vm within the folder (#8938) b17b211 Add cleanup_remote_cache config option to vmware-iso (#8917) e6368b9 Fix azure winrm_password attribution and allow to set winrm_username (#8928) fcf10e9 Replace Amazon with Outscale for OSC BSU doc (#8944) 9240fb7 Fix typo in title (#8943) 2c6f096 Allow accepting image for the members in OpenStack builder (#8931) b81800d Merge pull request #8935 from zaventh/feature/start-on-boot daffd9c CONTRIBUTING: Update documentation for linting on Travis (#8933) 3a9d356 golangci-lint: Update --new-from-rev option to check only newly added commits (#8923) 97d797d Fix small typos in osc-bsuvolume.html.md (#8926) 9486316 Merge pull request #8922 from hashicorp/f-vsphere_iso-export-ovf-options 56aebbe Merge pull request #8920 from rhencke/patch-1 99b0b98 Add ovf export capability to vsphere builders (#8764) d068430 make sure locals are evaluated only once variables are + test this (#8918) ad8dafa HCL: add tests and fixes around var-file and var args (#8914) 7979ab0 Add after_n_builds to codecov.yml (#8913) 3dae5df Merge pull request #8905 from hashicorp/fix_8493 811a730 Merge pull request #8907 from hashicorp/fix_8428 fa49d21 Merge pull request #8906 from hashicorp/fix_8904 b94937c Update provisioner_test.go (#8900) 2319521 Add iso config test for checksum from file specific case (#8897) 23f5603 Merge pull request #8889 from hashicorp/hcl2_singular_blocks dc9259f Merge pull request #8892 from zaventh/feature/vga-adapter 690bf71 Add Codecov badge and remove report style (#8896) fc35f02 Merge pull request #8890 from hashicorp/fix_8880 7972ab7 Merge pull request #8735 from hashicorp/fix_plugin_loading 890d7b2 Merge pull request #8875 from hashicorp/fix_8812 e94ff70 Merge pull request #8883 from hashicorp/fix_8835 ```

jetersen · 2020-03-31T14:59:30Z

How is this intended to be written in HCL?

storage = [] is unsupported

packer -v
1.5.5

jetersen · 2020-03-31T15:31:37Z

Seems like a bug that disk_size is still required

So this how it would look

  disk_size = 32768
  disk_thin_provisioned = true

  storage {
    disk_size = 16384
    disk_thin_provisioned = true
  }
  storage {
    disk_size = 8192
    disk_thin_provisioned = true
  }

jhawk28 · 2020-03-31T15:37:16Z

I'm assuming that your example parses without the disk_size. There is a disk_size check in prepare that should be updated:

	if c.DiskSize == 0 {
		errs = append(errs, fmt.Errorf("'disk_size' is required"))
	}

jetersen · 2020-03-31T15:48:23Z

the example parses but if I wanted to use only storage blocks than it would currently fail.

So this would fail because of the above code you pasted.

  storage {
    disk_size = 32768
    disk_thin_provisioned = true
  }
  storage {
    disk_size = 16384
    disk_thin_provisioned = true
  }
  storage {
    disk_size = 8192
    disk_thin_provisioned = true
  }

jetersen · 2020-03-31T15:49:49Z

The check should be updated to check if storage length is zero and DiskSize is zero :)

jhawk28 · 2020-03-31T15:59:53Z

maybe like this: #8975

ghost · 2020-04-04T01:50:25Z

I'm going to lock this issue 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 similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

jhawk28 requested a review from a team as a code owner February 21, 2020 15:48

jhawk28 force-pushed the vsphere_iso_multiple_disks branch from 6452aff to 7aa6cb1 Compare February 21, 2020 15:52

nywilken reviewed Mar 2, 2020

View reviewed changes

builder/vsphere/iso/step_create.go Outdated Show resolved Hide resolved

website/source/docs/builders/vsphere-iso.html.md.erb Show resolved Hide resolved

builder/vsphere/driver/vm.go Show resolved Hide resolved

jhawk28 force-pushed the vsphere_iso_multiple_disks branch from e23f135 to 823887f Compare March 3, 2020 16:58

jhawk28 requested review from carlpett and taliesins as code owners March 3, 2020 21:25

carlpett reviewed Mar 4, 2020

View reviewed changes

builder/proxmox/config.go Show resolved Hide resolved

Joshua Foster added 2 commits March 4, 2020 09:48

(vsphere-iso) add ability to define multiple disks

d723082

Closes hashicorp#8749

update documentation based on feedback

1dbe5ab

jhawk28 force-pushed the vsphere_iso_multiple_disks branch from 130a4a6 to 1dbe5ab Compare March 4, 2020 14:49

nywilken added builder/vsphere enhancement labels Mar 4, 2020

SwampDragons approved these changes Mar 4, 2020

View reviewed changes

SwampDragons merged commit 8832b3e into hashicorp:master Mar 4, 2020

jhawk28 deleted the vsphere_iso_multiple_disks branch March 12, 2020 16:42

SwampDragons added this to the 1.5.5 milestone Mar 12, 2020

ghost locked and limited conversation to collaborators Apr 4, 2020

Conversation

jhawk28 commented Feb 21, 2020

Uh oh!

jhawk28 commented Feb 21, 2020

Uh oh!

nywilken left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jhawk28 commented Mar 3, 2020

Uh oh!

SwampDragons commented Mar 3, 2020

Uh oh!

Uh oh!

SwampDragons commented Mar 4, 2020

Uh oh!

jetersen commented Mar 31, 2020

Uh oh!

jetersen commented Mar 31, 2020

Uh oh!

jhawk28 commented Mar 31, 2020

Uh oh!

jetersen commented Mar 31, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jetersen commented Mar 31, 2020

Uh oh!

jhawk28 commented Mar 31, 2020

Uh oh!

ghost commented Apr 4, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

jetersen commented Mar 31, 2020 •

edited

Loading