Rename Downloader to PriceSheetDownloader and remove type parameter

Now they're not in a package named azurepricesheet, the names for
Downloader and NewClient were too vague. Also the type parameter on
Downloader was a trick to avoid a circular dependency on the type in
pkg/cloud, but since everything's in pkg/cloud/azure we don't need it
anymore.

Signed-off-by: Christian Muirhead <christian.muirhead@microsoft.com>

pkg/cloud/azure/azureprovider.go

@@ -878,7 +878,7 @@ func (az *Azure) DownloadPricingData() error {
 
 	// If we've got a billing account set, kick off downloading the custom pricing data.
 	if config.AzureBillingAccount != "" {
-		downloader := Downloader[AzurePricing]{
+		downloader := PriceSheetDownloader{
 			TenantID:       config.AzureTenantID,
 			ClientID:       config.AzureClientID,
 			ClientSecret:   config.AzureClientSecret,

pkg/cloud/azure/client.go

@@ -34,11 +34,11 @@ type PriceSheetClient struct {
 	pl               runtime.Pipeline
 }
 
-// NewClient creates a new instance of PriceSheetClient with the specified values.
+// NewPriceSheetClient creates a new instance of PriceSheetClient with the specified values.
 // billingAccountId - Azure Billing Account ID.
 // credential - used to authorize requests. Usually a credential from azidentity.
 // options - pass nil to accept the default values.
-func NewClient(billingAccountID string, credential azcore.TokenCredential, options *arm.ClientOptions) (*PriceSheetClient, error) {
+func NewPriceSheetClient(billingAccountID string, credential azcore.TokenCredential, options *arm.ClientOptions) (*PriceSheetClient, error) {
 	if options == nil {
 		options = &arm.ClientOptions{}
 	}

pkg/cloud/azure/downloader.go

@@ -20,18 +20,18 @@ import (
 	"github.com/opencost/opencost/pkg/log"
 )
 
-type Downloader[T any] struct {
+type PriceSheetDownloader struct {
 	TenantID         string
 	ClientID         string
 	ClientSecret     string
 	BillingAccount   string
 	OfferID          string
-	ConvertMeterInfo func(info commerce.MeterInfo) (map[string]*T, error)
+	ConvertMeterInfo func(info commerce.MeterInfo) (map[string]*AzurePricing, error)
 }
 
-func (d *Downloader[T]) GetPricing(ctx context.Context) (map[string]*T, error) {
+func (d *PriceSheetDownloader) GetPricing(ctx context.Context) (map[string]*AzurePricing, error) {
 	log.Infof("requesting pricesheet download link")
-	url, err := d.getPricesheetDownloadURL(ctx)
+	url, err := d.getDownloadURL(ctx)
 	if err != nil {
 		return nil, fmt.Errorf("getting download URL: %w", err)
 	}
@@ -50,12 +50,12 @@ func (d *Downloader[T]) GetPricing(ctx context.Context) (map[string]*T, error) {
 	return prices, nil
 }
 
-func (d *Downloader[T]) getPricesheetDownloadURL(ctx context.Context) (string, error) {
+func (d *PriceSheetDownloader) getDownloadURL(ctx context.Context) (string, error) {
 	cred, err := azidentity.NewClientSecretCredential(d.TenantID, d.ClientID, d.ClientSecret, nil)
 	if err != nil {
 		return "", fmt.Errorf("creating credential: %w", err)
 	}
-	client, err := NewClient(d.BillingAccount, cred, nil)
+	client, err := NewPriceSheetClient(d.BillingAccount, cred, nil)
 	if err != nil {
 		return "", fmt.Errorf("creating pricesheet client: %w", err)
 	}
@@ -72,7 +72,7 @@ func (d *Downloader[T]) getPricesheetDownloadURL(ctx context.Context) (string, e
 	return resp.Properties.DownloadURL, nil
 }
 
-func (d Downloader[T]) saveData(ctx context.Context, url, tempName string) (io.ReadCloser, error) {
+func (d PriceSheetDownloader) saveData(ctx context.Context, url, tempName string) (io.ReadCloser, error) {
 	// Download file from URL in response.
 	out, err := os.CreateTemp("", tempName)
 	if err != nil {
@@ -113,7 +113,7 @@ func (r *removeOnClose) Close() error {
 	return os.Remove(r.Name())
 }
 
-func (d *Downloader[T]) readPricesheet(ctx context.Context, data io.Reader) (map[string]*T, error) {
+func (d *PriceSheetDownloader) readPricesheet(ctx context.Context, data io.Reader) (map[string]*AzurePricing, error) {
 	// Avoid double-buffering.
 	buf, ok := (data).(*bufio.Reader)
 	if !ok {
@@ -144,7 +144,7 @@ func (d *Downloader[T]) readPricesheet(ctx context.Context, data io.Reader) (map
 
 	units := make(map[string]bool)
 
-	results := make(map[string]*T)
+	results := make(map[string]*AzurePricing)
 	lines := 2
 	for {
 		row, err := reader.Read()

pkg/cloud/azure/downloader_test.go

@@ -7,11 +7,12 @@ import (
 	"testing"
 
 	"github.com/Azure/azure-sdk-for-go/profiles/2020-09-01/commerce/mgmt/commerce"
+	"github.com/opencost/opencost/pkg/cloud/types"
 	"github.com/stretchr/testify/require"
 )
 
 func TestDownloader(t *testing.T) {
-	d := Downloader[fakePricing]{
+	d := PriceSheetDownloader{
 		TenantID:         "test-tenant-id",
 		ClientID:         "test-client-id",
 		ClientSecret:     "test-client-secret",
@@ -26,11 +27,11 @@ func TestDownloader(t *testing.T) {
 
 		// Units and prices are normalised.
 		// Info for saving plans and other offers is skipped.
-		expected := map[string]*fakePricing{
-			"DC96as_v4": {price: "10.505", unit: "1 Hour"},
-			"DC2as_v4":  {price: "0.219", unit: "1 Hour"},
-			"VM1":       {price: "1.0", unit: "1 Hour"},
-			"VM2":       {price: "2.0", unit: "1 Hour"},
+		expected := map[string]*AzurePricing{
+			"DC96as_v4 1 Hour": {Node: &types.Node{Cost: "10.505"}},
+			"DC2as_v4 1 Hour":  {Node: &types.Node{Cost: "0.219"}},
+			"VM1 1 Hour":       {Node: &types.Node{Cost: "1.0"}},
+			"VM2 1 Hour":       {Node: &types.Node{Cost: "2.0"}},
 		}
 		require.Equal(t, expected, results)
 	})
@@ -48,13 +49,13 @@ func TestDownloader(t *testing.T) {
 	})
 
 	t.Run("no matching prices", func(t *testing.T) {
-		d := Downloader[fakePricing]{
+		d := PriceSheetDownloader{
 			TenantID:       "test-tenant-id",
 			ClientID:       "test-client-id",
 			ClientSecret:   "test-client-secret",
 			BillingAccount: "test-billing-account",
 			OfferID:        "my-offer-id",
-			ConvertMeterInfo: func(commerce.MeterInfo) (map[string]*fakePricing, error) {
+			ConvertMeterInfo: func(commerce.MeterInfo) (map[string]*AzurePricing, error) {
 				return nil, nil
 			},
 		}
@@ -63,29 +64,26 @@ func TestDownloader(t *testing.T) {
 	})
 }
 
-func convertMeter(info commerce.MeterInfo) (map[string]*fakePricing, error) {
+func convertMeter(info commerce.MeterInfo) (map[string]*AzurePricing, error) {
 	switch *info.MeterName {
 	case "skip-this":
 		return nil, nil
 	case "multiple-prices":
-		return map[string]*fakePricing{
-			"VM1": {price: "1.0", unit: "1 Hour"},
-			"VM2": {price: "2.0", unit: "1 Hour"},
+		return map[string]*AzurePricing{
+			"VM1 1 Hour": {Node: &types.Node{Cost: "1.0"}},
+			"VM2 1 Hour": {Node: &types.Node{Cost: "2.0"}},
 		}, nil
 	case "error":
 		return nil, fmt.Errorf("there was an error handling this row!")
 	default:
-		return map[string]*fakePricing{
-			*info.MeterName: {price: fmt.Sprintf("%0.3f", *info.MeterRates["0"]), unit: *info.Unit},
+		return map[string]*AzurePricing{
+			*info.MeterName + " " + *info.Unit: {
+				Node: &types.Node{Cost: fmt.Sprintf("%0.3f", *info.MeterRates["0"])},
+			},
 		}, nil
 	}
 }
 
-type fakePricing struct {
-	price string
-	unit  string
-}
-
 const pricesheetData = `Price Sheet Report for billing period - 202304
 
 Meter ID,Meter name,Meter category,Meter sub-category,Meter region,Unit,Unit of measure,Part number,Unit price,Currency code,Included quantity,Offer Id,Term,Price type