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

Oracle Support - Fix Oracle OSImage populate command. #3854

Merged
merged 1 commit into from
Nov 9, 2022

Conversation

HanFa
Copy link
Contributor

@HanFa HanFa commented Nov 7, 2022

What this PR does / why we need it

This is a follow-up PR for #3376, cherry-picked from #3557

It enables concurrent import for multiple public images using waitgroup. Behavioral-wise, it infers image information such as imageURL from the OSImage CR, instead of user passing-in using the flag. After the import finishes, it will put the private image OCID, compartment and region as the fields of osimage.spec.image.ref.

Which issue(s) this PR fixes

Fixes #

Describe testing done for PR

manual build and verification.

Release note

 Oracle Support - Fix Oracle OSImage populate command.

Additional information

Special notes for your reviewer

@HanFa HanFa requested a review from a team as a code owner November 7, 2022 22:53
@HanFa HanFa changed the title Fix Oracle OSImage populate command. Oracle Support - Fix Oracle OSImage populate command. Nov 7, 2022
@HanFa HanFa self-assigned this Nov 7, 2022
@codecov
Copy link

codecov bot commented Nov 8, 2022

Codecov Report

Merging #3854 (252bbe2) into main (c57f9ac) will decrease coverage by 0.89%.
The diff coverage is 0.00%.

@@            Coverage Diff             @@
##             main    #3854      +/-   ##
==========================================
- Coverage   46.56%   45.66%   -0.90%     
==========================================
  Files         400      425      +25     
  Lines       39830    41381    +1551     
==========================================
+ Hits        18548    18898     +350     
- Misses      19584    20768    +1184     
- Partials     1698     1715      +17     
Impacted Files Coverage Δ
cmd/cli/plugin/cluster/osimage.go 100.00% <ø> (ø)
cmd/cli/plugin/cluster/osimage_oracle.go 3.20% <0.00%> (ø)
addons/controllers/machine_controller.go 65.65% <0.00%> (-3.04%) ⬇️
cmd/cli/plugin/cluster/set_node_pool.go 14.63% <0.00%> (ø)
cmd/cli/plugin/cluster/create.go 42.22% <0.00%> (ø)
cmd/cli/plugin/cluster/delete.go 12.50% <0.00%> (ø)
cmd/cli/plugin/cluster/get_machinehealthcheck.go 11.42% <0.00%> (ø)
cmd/cli/plugin/cluster/main.go 0.00% <0.00%> (ø)
.../cli/plugin/cluster/set_machinehealthcheck_node.go 23.33% <0.00%> (ø)
cmd/cli/plugin/cluster/set_machinehealthcheck.go 23.33% <0.00%> (ø)
... and 19 more

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

Copy link
Member

@XudongLiuHarold XudongLiuHarold left a comment

Choose a reason for hiding this comment

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

LGTM

@DanielXiao
Copy link
Contributor

Please also add test cases to meet minimal code coverage rate

@DanielXiao DanielXiao added the ok-to-merge PRs should be labelled with this before merging label Nov 9, 2022
@HanFa HanFa merged commit 21e4437 into main Nov 9, 2022
@HanFa HanFa deleted the topic/fhan/fix-osimage-populate-cmd branch November 9, 2022 19:34
xiujuanx pushed a commit that referenced this pull request Nov 14, 2022
# for free to subscribe to this conversation on GitHub. Already have an account? #.
Labels
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