Skip to content

add ovf export capability to vsphere builders#8764

Merged
nywilken merged 18 commits intohashicorp:masterfrom
jhawk28:vsphere_export_ovf
Mar 19, 2020
Merged

add ovf export capability to vsphere builders#8764
nywilken merged 18 commits intohashicorp:masterfrom
jhawk28:vsphere_export_ovf

Conversation

@jhawk28
Copy link
Copy Markdown
Contributor

@jhawk28 jhawk28 commented Feb 18, 2020

Ported the export ovf from govc. It compiles, but still needs to be tested. I made it common so that it can be used to export for both iso and clone builders

Closes #8752

@jhawk28 jhawk28 requested a review from a team as a code owner February 18, 2020 05:18
@jhawk28 jhawk28 force-pushed the vsphere_export_ovf branch 3 times, most recently from 15c4ae5 to ccfd946 Compare February 18, 2020 05:36
@jhawk28
Copy link
Copy Markdown
Contributor Author

jhawk28 commented Feb 18, 2020

not sure why the check-generate is failing...

Copy link
Copy Markdown
Contributor

@sylviamoss sylviamoss left a comment

Choose a reason for hiding this comment

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

The step_export from vmware seems to be more simple and uses ovftool. Do you think is a good idea to give it a try?
https://github.com/hashicorp/packer/blob/master/builder/vmware/common/step_export.go
it's less code and the necessary configuration is also more simple 🤔

@jhawk28
Copy link
Copy Markdown
Contributor Author

jhawk28 commented Feb 18, 2020

using the StepExport from the VMware builder would add the requirement that the user install the ovftool. This approach would just work without any additional requirements and use the existing connections.

@jhawk28 jhawk28 changed the title add ovf export capability to vsphere builders [WIP] add ovf export capability to vsphere builders Feb 18, 2020
SwampDragons
SwampDragons previously approved these changes Feb 18, 2020
@SwampDragons
Copy link
Copy Markdown
Contributor

What if someone does not want to export to an OVF? I'm not seeing an option that actually says whether to do it.

@SwampDragons SwampDragons dismissed their stale review February 18, 2020 23:46

I didn't think closely enough about usage; hasn't been tested yet.

@jhawk28
Copy link
Copy Markdown
Contributor Author

jhawk28 commented Feb 19, 2020

I think it would be cleanest to make the config do an export field. If the export field is populated, it would do the export. If it is not, it does not do the export. The alternative is to do a "skip_export" flag, but I'm leaning towards just doing the export field since that would also let me clean up the "export_" prefixes.

@SwampDragons
Copy link
Copy Markdown
Contributor

I agree -- explicitly asking for the export is better than asking whether not to export. Then we can add a test at the very beginning of the "run" and return multistep.ActionContinue if export is not set.

@sylviamoss
Copy link
Copy Markdown
Contributor

sylviamoss commented Feb 19, 2020

I'm still concern that the configuration is different from the vmware one. The user here was expecting it to be the same. I think it's worth trying to make the config similar to https://github.com/hashicorp/packer/blob/master/builder/vmware/common/step_export.go#L21
what do you think?

@sylviamoss
Copy link
Copy Markdown
Contributor

After thinking more about this, I suggest only naming the config options that are the same with the same name as it is for vmware. Overall, the PR looks good to me! 👍

@jhawk28
Copy link
Copy Markdown
Contributor Author

jhawk28 commented Feb 20, 2020

still doing some testing. trying to figure out how to fix an error.

@jhawk28
Copy link
Copy Markdown
Contributor Author

jhawk28 commented Feb 20, 2020

After thinking more about this, I suggest only naming the config options that are the same with the same name as it is for vmware. Overall, the PR looks good to me! 👍

I chose name instead of vm_name because vm_name is not differentiating the name of the export and the name of the vm in vsphere.

The OutputDir approach is the same as other builders (hyperv etc). We can probably make that object common since they are all the same. We should probably also update the vmware one to use it.

The tool options don't make sense because the API currently doesn't support all of them.

Would you like me to add the skip_export flag back?

@jhawk28 jhawk28 changed the title [WIP] add ovf export capability to vsphere builders add ovf export capability to vsphere builders Feb 20, 2020
@jhawk28
Copy link
Copy Markdown
Contributor Author

jhawk28 commented Feb 20, 2020

export is working now. rebased/merged from master so no more conflicts...

Copy link
Copy Markdown
Contributor

@sylviamoss sylviamoss left a comment

Choose a reason for hiding this comment

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

I think this looks good!

@jhawk28
Copy link
Copy Markdown
Contributor Author

jhawk28 commented Mar 19, 2020

looks like #8910 needs to be fixed before I can test my changes

@jhawk28 jhawk28 force-pushed the vsphere_export_ovf branch from 9f60a43 to 3d5ec70 Compare March 19, 2020 13:30
@jhawk28
Copy link
Copy Markdown
Contributor Author

jhawk28 commented Mar 19, 2020

ok, was able to verify the changes on my infrastructure.

Copy link
Copy Markdown
Contributor

@nywilken nywilken left a comment

Choose a reason for hiding this comment

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

@jhawk28 thanks for the quick turnaround. The changes look great! I agree Manifest is clearer.

I went ahead and tested the new code and everything works as expected 🎉

Test Results (using defaults):

Using the following export configuration

"export": {
}

⇶  cd output-vsphere-iso
⇶  tree
.
├── example-ubuntu-disk-0.vmdk
├── example-ubuntu.mf
└── example-ubuntu.ovf

0 directories, 3 files

⇶  sha256sum -c example-ubuntu.mf 
example-ubuntu-disk-0.vmdk: OK
example-ubuntu.ovf: OK

Test Results (name, manifest options):

Using the following export configuration

"export": {
  "name": "ubuntu",
  "manifest": "none",
  "output_directory": "output_vsphere_nomanifest"
}

⇶  cd output_vsphere_nomanifest 
⇶  tree
.
├── ubuntu-disk-0.vmdk
└── ubuntu.ovf

@nywilken nywilken merged commit 99b0b98 into hashicorp:master Mar 19, 2020
@nywilken nywilken added this to the 1.5.5 milestone Mar 19, 2020
@jhawk28 jhawk28 deleted the vsphere_export_ovf branch March 19, 2020 18:15
nywilken added a commit that referenced this pull request Mar 27, 2020
…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
```
@ghost
Copy link
Copy Markdown

ghost commented Apr 19, 2020

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.

@ghost ghost locked and limited conversation to collaborators Apr 19, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

vsphere-iso format ovf no longer works?

5 participants