Skip to content
This repository has been archived by the owner on Oct 10, 2023. It is now read-only.

Fix vSphere OSImage labeling #4282

Merged
merged 1 commit into from
Jan 14, 2023

Conversation

imikushin
Copy link
Contributor

What this PR does / why we need it

For vsphere-nonparavirt provider, OSImage objects have version field their .spec.image.ref field, e.g. like this:

  image:
    type: ova
    ref:
      version: v1.24.9+vmware.1-tkg.1-226b7a84930e5368c38aa867f998ce33

When being added to the TKR Resolver cache (for example, when being updated by the tkr-status-controller), the OSImage .spec.image.ref contents are set as the OSImage labels to enable resolution of the OSImage by this data using a label query.

The problem is: "+" cannot be used in label values. This change fixes it by replacing "+" with "---" (same way we derive TKR names from their versions).

Which issue(s) this PR fixes

N/A

Describe testing done for PR

Updated unit tests to expose the problem fixed.

Release note

OSImage objects for vsphere-nonparavirt provider will now have `ova-version` label set to
the value of the OSImage `.spec.image.ref.version` with "+" replaced by "---".

Additional information

Special notes for your reviewer

For vsphere-nonparavirt provider, OSImage objects have version field
their `.spec.image.ref` field, e.g. like this:
```
  image:
    type: ova
    ref:
      version: v1.24.9+vmware.1-tkg.1-226b7a84930e5368c38aa867f998ce33
```

When being added to the TKR Resolver cache (for example, when being
updated by the tkr-status-controller), the OSImage `.spec.image.ref`
contents are set as the OSImage labels to enable resolution of the
OSImage by this data using a label query.

The problem is: "+" cannot be used in label values. This change fixes it
by replacing "+" with "---" (same way we derive TKR names from their
versions).

Signed-off-by: Ivan Mikushin <imikushin@vmware.com>
@imikushin imikushin requested a review from a team as a code owner January 14, 2023 00:55
@imikushin
Copy link
Contributor Author

/test install-vc7

@codecov
Copy link

codecov bot commented Jan 14, 2023

Codecov Report

Merging #4282 (11d9074) into main (b0cc3b8) will decrease coverage by 0.90%.
The diff coverage is 65.21%.

@@            Coverage Diff             @@
##             main    #4282      +/-   ##
==========================================
- Coverage   49.41%   48.51%   -0.91%     
==========================================
  Files         451      481      +30     
  Lines       44718    46869    +2151     
==========================================
+ Hits        22098    22739     +641     
- Misses      20541    21994    +1453     
- Partials     2079     2136      +57     
Impacted Files Coverage Δ
tkg/clusterclient/clusterclient.go 49.14% <ø> (ø)
tkg/clusterclient/credentials.go 71.48% <63.15%> (-1.45%) ⬇️
tkg/client/update_credentials.go 35.45% <75.00%> (ø)
...ons/controllers/packageinstallstatus_controller.go 77.99% <0.00%> (-1.16%) ⬇️
cmd/cli/plugin/isolated-cluster/main.go 0.00% <0.00%> (ø)
cmd/cli/plugin/cluster/kubeconfig_get.go 8.82% <0.00%> (ø)
cmd/cli/plugin/cluster/delete_node_pool.go 16.66% <0.00%> (ø)
...cluster/delete_machinehealthcheck_control_plane.go 16.66% <0.00%> (ø)
cmd/cli/plugin/cluster/machinehealthcheck.go 100.00% <0.00%> (ø)
... and 27 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@jayunit100
Copy link
Contributor

/lgtm
/approve

@imikushin imikushin added the ok-to-merge PRs should be labelled with this before merging label Jan 14, 2023
@imikushin imikushin merged commit f810ab3 into main Jan 14, 2023
@imikushin imikushin deleted the topic/imikushin/vsphere-osimage-ref-version-label branch January 14, 2023 03:59
# for free to subscribe to this conversation on GitHub. Already have an account? #.
Labels
area/tkr cla-not-required ok-to-merge PRs should be labelled with this before merging
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants