🐛 Use a cheaper method for storage policy

[rikatz]

What this PR does / why we need it:
During the debug of #2453 (comment) we have figured out this call is too expensive for environments with a lot of resources, like more than 10k resources.

This may turn into a huge slow query that can take up to 20 minutes.

Instead, we should be using the "QueryAssociatedProfiles" call that, given an object (like a disk), check if it has associated storage policies

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes #

Note vmware/govmomi#3268 implements the call directly on pbmClient, but as this is not released yet I have added it on the new utils file.

[rikatz]

/hold
I will compile it locally and run on our environment just to make sure the problem is gone

[codecov]

Codecov Report

Attention: 31 lines in your changes are missing coverage. Please review.

Comparison is base (c23a946) 63.45% compared to head (40b2c61) 63.43%.
Report is 12 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2467      +/-   ##
==========================================
- Coverage   63.45%   63.43%   -0.03%     
==========================================
  Files         122      123       +1     
  Lines        8754     8773      +19     
==========================================
+ Hits         5555     5565      +10     
- Misses       2785     2788       +3     
- Partials      414      420       +6     

Files	Coverage Δ
pkg/services/govmomi/storageprofile_util.go	0.00% <0.00%> (ø)
pkg/services/govmomi/service.go	3.45% <0.00%> (+2.20%)	⬆️

... and 3 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[rikatz]

/hold cancel
Tested on our environment, creation is being really fast now with Storage Policies and respecting the associated storage policy specified

[sbueringer]

Thx! Just nits.

Should we backport this change/fix?

[sbueringer]

/assign @chrischdi

[rikatz]

/retest
weird

[rikatz]

Should we backport this change/fix?

Yes. On environments that, as an example don't reclaim PVs, there are going to be a lot of objects. Querying all of this objects is an expensive operation and may get cluster reconciliation stuck for a long time. IMHO at least for 1.7 and 1.8 this should be cherry picked.

Thanks!!

[sbueringer]

Thank you!

/lgtm

/assign @chrischdi

[k8s-ci-robot]

LGTM label has been added.

Git tree hash: 2d507e25aa8439bb38a58223eea99981a95e68ff

[sbueringer]

/cherry-pick release-1.8

[k8s-infra-cherrypick-robot]

@sbueringer: once the present PR merges, I will cherry-pick it on top of release-1.8 in a new PR and assign it to you.

In response to this:

/cherry-pick release-1.8

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[sbueringer]

/cherry-pick release-1.7

[k8s-infra-cherrypick-robot]

@sbueringer: once the present PR merges, I will cherry-pick it on top of release-1.7 in a new PR and assign it to you.

In response to this:

/cherry-pick release-1.7

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[rikatz]

/hold
I will just update govmomi for 0.33.1 and drop the copied code (thanks @dougm !!)

[rikatz]

/hold cancel
@sbueringer @chrischdi last changes after Stefan's review:

• bumped govmomi to v0.33.1 (it contains the required methods on pbmclient)
• squashed commits
  Thanks!!

[sbueringer]

/lgtm

@chrischdi PTAL :)

[k8s-ci-robot]

LGTM label has been added.

Git tree hash: 0b28b37b746682e78686634550de1c03054f5598

[chrischdi]

Only nits about comments and a optional question/thing for the test code, lgtm to it in general!

Thanks for also adding test cases!

[chrischdi]

/approve

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: chrischdi

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [chrischdi]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[sbueringer]

/lgtm

[k8s-ci-robot]

LGTM label has been added.

Git tree hash: e64c8031680e6e298591358973b06b37a64ed8a5

[k8s-infra-cherrypick-robot]

@sbueringer: #2467 failed to apply on top of branch "release-1.8":

Applying: Use a less expensive method for storage policy
Using index info to reconstruct a base tree...
M	go.mod
M	go.sum
M	pkg/services/govmomi/service.go
M	pkg/services/govmomi/service_test.go
Falling back to patching base and 3-way merge...
Auto-merging pkg/services/govmomi/service_test.go
CONFLICT (content): Merge conflict in pkg/services/govmomi/service_test.go
Auto-merging pkg/services/govmomi/service.go
CONFLICT (content): Merge conflict in pkg/services/govmomi/service.go
Auto-merging go.sum
CONFLICT (content): Merge conflict in go.sum
Auto-merging go.mod
CONFLICT (content): Merge conflict in go.mod
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0001 Use a less expensive method for storage policy
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

In response to this:

/cherry-pick release-1.8

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[k8s-infra-cherrypick-robot]

@sbueringer: #2467 failed to apply on top of branch "release-1.7":

Applying: Use a less expensive method for storage policy
Using index info to reconstruct a base tree...
M	go.mod
M	go.sum
M	pkg/services/govmomi/service.go
M	pkg/services/govmomi/service_test.go
Falling back to patching base and 3-way merge...
Auto-merging pkg/services/govmomi/service_test.go
CONFLICT (content): Merge conflict in pkg/services/govmomi/service_test.go
Auto-merging pkg/services/govmomi/service.go
CONFLICT (content): Merge conflict in pkg/services/govmomi/service.go
Auto-merging go.sum
CONFLICT (content): Merge conflict in go.sum
Auto-merging go.mod
CONFLICT (content): Merge conflict in go.mod
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0001 Use a less expensive method for storage policy
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

In response to this:

/cherry-pick release-1.7

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.