Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: support asterisk in url during import #1813

Merged
merged 1 commit into from
Oct 20, 2023

Conversation

pawelprazak
Copy link
Contributor

@pawelprazak pawelprazak commented Jan 11, 2023

Description

Fixes #1806 to ensure use of the correct URL parsing function from vmware/govmomi.

Testing

See #1813 (comment),

References

Closes #1806

@hashicorp-cla
Copy link

hashicorp-cla commented Jan 11, 2023

CLA assistant check
All committers have signed the CLA.

@tenthirtyam
Copy link
Collaborator

Could we ask that you please use conventional commits structure for the pull request title and commit description?

@tenthirtyam tenthirtyam added the area/ovf Area: OVA/OVF label Jan 11, 2023
@tenthirtyam tenthirtyam added the bug Type: Bug label Jan 11, 2023
@tenthirtyam tenthirtyam added this to the Backlog milestone Jan 11, 2023
@pawelprazak pawelprazak force-pushed the pawelprazak/issue1806 branch 2 times, most recently from 8ee9fd4 to 65a80e2 Compare January 12, 2023 08:34
@github-actions github-actions bot added provider Type: Provider size/s Relative Sizing: Small labels Jan 12, 2023
@pawelprazak pawelprazak changed the title #1806 - support asterisk in URL during import fix: support asterisk in URL during import Jan 12, 2023
@pawelprazak
Copy link
Contributor Author

Could we ask that you please use conventional commits structure for the pull request title and commit description?

I've updated the title and commit, and signed the CLA.

@pawelprazak pawelprazak marked this pull request as ready for review January 12, 2023 08:35
@pawelprazak
Copy link
Contributor Author

@tenthirtyam could you provide any pointers regarding adding a test for this PR? :)

@tenthirtyam
Copy link
Collaborator

At the very least, provide the results of your ad hoc testing. cc @appilon for a e2e scenario test.

@pawelprazak
Copy link
Contributor Author

pawelprazak commented Jan 20, 2023

The VM resource used for manual test:

resource "vsphere_virtual_machine" "pprazak-test-1813" {
  name             = "pprazak-test-1813"
  host_system_id   = data.vsphere_host.host.id
  resource_pool_id = data.vsphere_resource_pool.pool.id
  datastore_id     = data.vsphere_datastore.datastore.id
  datacenter_id    = data.vsphere_datacenter.dc.id

  num_cpus = 2
  memory   = 1024

  ovf_deploy {
    remote_ovf_url    = "https://cloud-images.ubuntu.com/focal/current/focal-server-cloudimg-amd64.ova"
    disk_provisioning = "thin"
  }
  network_interface {
    network_id = data.vsphere_network.network.id
  }
}

repro before the fix:

vsphere_virtual_machine.pprazak-test-1813: Creating...
╷
│ Error: error while importing ovf/ova template, error while uploading the disk ubuntu-focal-20.04-cloudimg.vmdk error while uploading the file ubuntu-focal-20.04-cloudimg.vmdk Post "https://*/ha-nfc/52e7b604-da64-267c-7dd3-bedd34f06fac/disk-0.vmdk": dial tcp: lookup *: no such host
│
│   with vsphere_virtual_machine.pprazak-test-1813,
│   on main.tf line 43, in resource "vsphere_virtual_machine" "pprazak-test-1813":
│   43: resource "vsphere_virtual_machine" "pprazak-test-1813" {

And after the fix:

vsphere_virtual_machine.pprazak-test-1813: Creating...
vsphere_virtual_machine.pprazak-test-1813: Still creating... [10s elapsed]
vsphere_virtual_machine.pprazak-test-1813: Still creating... [20s elapsed]
vsphere_virtual_machine.pprazak-test-1813: Still creating... [30s elapsed]

It made it's way to the ESXi host:
Screenshot 2023-01-20 at 14 53 09

@tenthirtyam
Copy link
Collaborator

HI @pawelprazak - thank you for the update.

When I have some free time, I'll look over and test this pull request when I have some spare cycles..

@tenthirtyam tenthirtyam self-requested a review January 20, 2023 14:03
@Djodane
Copy link

Djodane commented Feb 9, 2023

Hi ! We would love to see this PR merged to be able to deploy and manage vm from templates on standalone esxi hosts.

@tenthirtyam
Copy link
Collaborator

tenthirtyam commented Feb 9, 2023

Hi ! We would love to see this PR merged to be able to deploy and manage vm from templates on standalone esxi hosts.

It will be merged once prioritized for a milestone and reviewed / approved by the maintainers.

@Djodane
Copy link

Djodane commented Feb 9, 2023

Hi ! We would love to see this PR merged to be able to deploy and manage vm from templates on standalone esxi hosts.

It will be merged once prioritized for a milestone and reviewed / approved by the maintainers.

thx! hope it'll be soon :)

@xamyp
Copy link

xamyp commented Feb 9, 2023

Hello, I would like to see these changes as well. I hope it will merge soon.

@tenthirtyam
Copy link
Collaborator

Hello, I would like to see these changes as well. I hope it will merge soon.

Community Note

@tenthirtyam tenthirtyam modified the milestones: Backlog, v2.5.0 May 4, 2023
@tenthirtyam tenthirtyam modified the milestones: v2.5.0, Backlog Jul 25, 2023
@tenthirtyam tenthirtyam modified the milestones: Backlog, v2.5.0 Aug 7, 2023
@tenthirtyam tenthirtyam removed this from the v2.5.0 milestone Oct 9, 2023
@tenthirtyam tenthirtyam added this to the v2.6.0 milestone Oct 9, 2023
@tenthirtyam
Copy link
Collaborator

I've completed an initial verification of the change and the results are positive.

Once I perform some negative testing I'll add my review.

Ryan Johnson
Senior Staff Solutions Architect | Product Engineering @ VMware, Inc.

@tenthirtyam tenthirtyam changed the title fix: support asterisk in URL during import fix: support asterisk in url during import Oct 19, 2023
Copy link
Collaborator

@tenthirtyam tenthirtyam left a comment

Choose a reason for hiding this comment

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

LGTM! 🚀

I've completed testing for this change and the results are all positive.

No regressions observed.

Ryan Johnson
Senior Staff Solutions Architect | Product Engineering @ VMware, Inc.

@tenthirtyam tenthirtyam force-pushed the pawelprazak/issue1806 branch from 65a80e2 to da8abe1 Compare October 19, 2023 20:37
@tenthirtyam tenthirtyam requested a review from a team as a code owner October 19, 2023 20:37
@tenthirtyam tenthirtyam force-pushed the pawelprazak/issue1806 branch from da8abe1 to 7e1e0a9 Compare October 19, 2023 20:43
@github-actions github-actions bot added the documentation Type: Documentation label Oct 19, 2023
- use the correct URL parsing function from govmomi library to replace asterisk with a host when needed

Refs: hashicorp#1806
@tenthirtyam tenthirtyam force-pushed the pawelprazak/issue1806 branch from 7e1e0a9 to 10ab688 Compare October 19, 2023 20:44
@tenthirtyam
Copy link
Collaborator

@iBrandyJackson I've updated the CHANGELOG.md in this pull request to reflect the changes.

@iBrandyJackson iBrandyJackson merged commit 5e66f7b into hashicorp:main Oct 20, 2023
4 checks passed
Copy link

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 Nov 20, 2023
@pawelprazak pawelprazak deleted the pawelprazak/issue1806 branch November 20, 2023 08:19
@hashicorp hashicorp unlocked this conversation Nov 30, 2023
Copy link

This functionality has been released in v2.6.0 of the Terraform Provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading.

For further feature requests or bug reports with this functionality, please create a new GitHub issue following the template. Thank you!

Copy link

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 Dec 31, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area/ovf Area: OVA/OVF bug Type: Bug documentation Type: Documentation provider Type: Provider size/s Relative Sizing: Small
Projects
None yet
6 participants