Closed logyball closed 2 months ago
I don't love the naming of wrapper, I'd call tha
Currently staged, observing behavior. Nothing stands out at the moment as concerning but I'll keep an eye on it.
EDIT: no regressions observed after 24 hours. Merging!
When attempting to write out larger-reaching integration tests for the Azure Module of
cloudcost-exporter
, I was running into a bit of a wall with how the mocks currently work. This PR attempts to address that.What
Move our internal use of the Azure SDK into its own package. The client(s) used will live in a Wrapper object that will be attached to the Azure object, which can be passed to individual Collectors for when they need to reach the Azure API.
Why
By decoupling the logic of reaching out to the Azure API, we can much more easily test the actual logic of the collectors.
Notes for reviewers
The git diff is a bit much. One way to look at this is through several components
Component One: Azure Client Wrapper
These changes are isolated to
pkg/azure/azureClientWrapper/azureClientWrapper.go
. They are fairly straightforward and are lifting the logic from the individual components (e.g. MachineStore-> getVmInfoFromVmss becomes AzureClient->ListVirtualMachineScaleSetsOwnedVms).Component Two: Machine Store
Machine store has taken the logic that reaches out to the Azure API and deferred that responsibility to AzureClient. Notable places:
getVmInfoFromVmss
now usesazClientWrapper.ListVirtualMachineScaleSetsOwnedVms
getVMSSInfoFromResourceGroup
now usesazClientWrapper.ListVirtualMachineScaleSetsFromResourceGroup
getClustersInSubscription
now usesazClientWrapper.ListClustersInSubscription
Component Three: Price Store
PopulatePriceStore
now usesp.azureClientWrapper.ListPrices
Component Four: Testing
Testing can be seen fully implemented for a single function (as a PoC) in
price_store_test.go -> TestPopulatePriceStore
.Downsides (and counterpoints)