Skip to content

fix: simultaneous attempts to create parent directories #2366

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

Merged

Conversation

skevir
Copy link
Contributor

@skevir skevir commented Mar 28, 2025

Description

This PR fixes the functionality that attempts to cope with a directory already existing when creating the parent directories for a vsphere_virtual_disk resource. For VSAN datastores it has been observed that multiple simultaneous attempts to create the same parent directories can result in vSphere reporting errors.

A previous attempt to fix this (under #639) appears to be flawed, in that the error message the functionality is triggered off is actually slightly different to what the code is written to expect. Additionally, there is different error that is sometimes hit in the same scenario, and the original fix didn't handle that case at all.

To capture both of these cases, as well as any other errors that might be seen in the future, I have changed the implementation to ignore the error hit when attempting to create the parent directories and use the existing functionality that performs a post-check that the directory now exists to determine if the creation was successful.

Testing

I have installed a fixed version of the provider and tested the following. In all case I have performed the test both in the case when the directory doesn't already exist, as well as when it does:

  • creating multiple vsphere_virtual_disk resources in the same directory using separate definitions in the Terraform config file.
  • creating multiple vsphere_virtual_disk resources using the count field on a single resource definition in the Terraform config file.

In the cases that would have previously failed (that is, when the directory didn't already exist) I have confirmed, by looking at the debug logs, that errors were reported when trying to create the directories for some of the vsphere_virtual_disk resources to verify that the bug fix was being invoked.

Acceptance tests

  • Have you added an acceptance test for the functionality being added? I do not believe any new acceptance tests are required due to the nature of the bug being fixed.
  • Have you run the acceptance tests on this branch? No - the code change seems contained enough that the live testing I have performed feels adequate.

Output from acceptance testing:

Not run

Release Note

Release note for CHANGELOG:

BUG FIX:
`resource/vsphere_virtual_disk`: Fix handling of the creation of parent directories hitting "already exists" errors.

References

Closes #2309
PR for previous fix attempt: #639

@github-actions github-actions bot added provider Provider needs-review Needs Review labels Mar 28, 2025
@tenthirtyam tenthirtyam added this to the Backlog milestone Mar 28, 2025
@tenthirtyam tenthirtyam added the bug Bug label Mar 28, 2025
@skevir skevir changed the title Fix handling simultaneous attempts to create parent directories when creating a vsphere_virtual_disk resource fix: handle simultaneous attempts to create parent directories when creating a vsphere_virtual_disk resource Mar 28, 2025
@skevir skevir force-pushed the ras/virtual-disk-create-directories branch from 2c061b1 to 3a03e78 Compare March 28, 2025 17:11
Copy link
Contributor

@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.

Hi, @skevir! 👋🏻 The HashiCorp CLA needs to be signed before this pull request can proceed.

@tenthirtyam tenthirtyam force-pushed the ras/virtual-disk-create-directories branch from 3a03e78 to 7a28861 Compare March 31, 2025 15:26
@skevir
Copy link
Contributor Author

skevir commented Mar 31, 2025

Hi, @skevir! 👋🏻 The HashiCorp CLA needs to be signed before this pull request can proceed.

Thanks - I'm just confirming with my company's legal department that they are happy for me to sign. Once I have that, I will remove the "Draft" status for this PR so that it can proceed.

@tenthirtyam
Copy link
Contributor

I'm just confirming with my company's legal department that they are happy for me to sign.

Understood! Note that right now the commit is signed-off with your no-reply that does not designate an employer. So be mindful if you need to agree to the CLA as an individual contrinutor or through your employer.

Merge: 7a288618 9ad0f32c
Author: skevir <[email protected]>
Date:   Mon Mar 31 17:03:54 2025 +0100

@tenthirtyam tenthirtyam force-pushed the ras/virtual-disk-create-directories branch from a09bfcd to 564536d Compare April 15, 2025 13:38
@tenthirtyam tenthirtyam added do-not-merge Do Not Merge and removed provider Provider labels Apr 16, 2025
@tenthirtyam tenthirtyam removed the needs-review Needs Review label Apr 26, 2025
@tenthirtyam tenthirtyam force-pushed the main branch 2 times, most recently from 2d0d0cd to 3ebb159 Compare May 6, 2025 00:35
@tenthirtyam tenthirtyam force-pushed the ras/virtual-disk-create-directories branch from 564536d to 76e98e9 Compare May 7, 2025 01:06
@tenthirtyam tenthirtyam changed the title fix: handle simultaneous attempts to create parent directories when creating a vsphere_virtual_disk resource [wip] fix: simultaneous attempts to create parent directories May 7, 2025
@github-actions github-actions bot added provider Provider needs-review Needs Review labels May 7, 2025
@tenthirtyam tenthirtyam added awaiting-response Awaiting Response and removed needs-review Needs Review do-not-merge Do Not Merge labels May 7, 2025
@github-actions github-actions bot added the needs-review Needs Review label May 13, 2025
Copy link
Contributor

@spacegospod spacegospod left a comment

Choose a reason for hiding this comment

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

@skevir the diff looks good!
Can you please sign your last commit and mark the PR as ready (non-draft)

@tenthirtyam tenthirtyam removed needs-review Needs Review provider Provider labels May 30, 2025
@tenthirtyam tenthirtyam force-pushed the ras/virtual-disk-create-directories branch from eb6b166 to 8c55aee Compare May 30, 2025 17:30
@github-actions github-actions bot added provider Provider needs-review Needs Review size/xs Relative Sizing: Extra-Small labels May 30, 2025
@tenthirtyam tenthirtyam force-pushed the ras/virtual-disk-create-directories branch 3 times, most recently from 9984ae7 to d770398 Compare May 30, 2025 17:37
Handles `fm.MakeDirectory` failing when creating parent directories for a virtual disk.

Signed-off-by: skevir <[email protected]>
@tenthirtyam tenthirtyam force-pushed the ras/virtual-disk-create-directories branch from d770398 to 02a05f0 Compare May 30, 2025 17:38
@tenthirtyam tenthirtyam modified the milestones: Backlog, v2.14.0 May 30, 2025
@vmware vmware deleted a comment from hashicorp-cla-app bot May 30, 2025
@tenthirtyam tenthirtyam dismissed their stale review May 30, 2025 17:39

Commit updated.

@tenthirtyam tenthirtyam marked this pull request as ready for review May 30, 2025 17:40
@tenthirtyam tenthirtyam requested a review from a team as a code owner May 30, 2025 17:40
@tenthirtyam tenthirtyam changed the title [wip] fix: simultaneous attempts to create parent directories fix: simultaneous attempts to create parent directories May 30, 2025
Copy link
Contributor

@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

@vmware vmware deleted a comment from hashicorp-cla-app bot May 30, 2025
@tenthirtyam tenthirtyam removed awaiting-response Awaiting Response needs-review Needs Review labels May 30, 2025
@tenthirtyam tenthirtyam merged commit da04da6 into vmware:main Jun 2, 2025
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Bug provider Provider size/xs Relative Sizing: Extra-Small
Projects
None yet
Development

Successfully merging this pull request may close these issues.

r/virtual_disk: create_directories fails if creating multiple disks in parallel in the same directory
3 participants