Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

chore(gke): Remove deprecated method #350

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

chore(gke): Remove deprecated method #350

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
94 changes: 11 additions & 83 deletions pkg/google/gke/pricing_map.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -196,6 +196,8 @@ var (
}
)

// Populate is responsible for collecting skus related to Compute Engine, parsing out the response, and then populating the pricing map
// with relevant skus.
func (pm *PricingMap) Populate(ctx context.Context, billingService *billingv1.CloudCatalogClient) error {
serviceName, err := billing.GetServiceName(ctx, billingService, "Compute Engine")
if err != nil {
Expand All @@ -207,6 +209,11 @@ func (pm *PricingMap) Populate(ctx context.Context, billingService *billingv1.Cl
return ErrSkuNotFound
}

return pm.ParseSkus(skus)
}

// ParseSkus accepts a list of skus, parses their content, and updates the pricing map with the appropriate costs.
func (pm *PricingMap) ParseSkus(skus []*billingpb.Sku) error {
for _, sku := range skus {
rawData, err := getDataFromSku(sku)
if errors.Is(err, ErrSkuNotRelevant) {
Expand Down Expand Up @@ -265,14 +272,14 @@ func (pm *PricingMap) Populate(ctx context.Context, billingService *billingv1.Cl
break
}
}
if strings.Contains(data.Description, "Confidential") {
log.Printf("Storage class contains Confidential: %s\n%s\n", storageClass, data.Description)
continue
}
if storageClass == "" {
log.Printf("Storage class not found for %s. Skipping", data.Description)
continue
}
if strings.Contains(data.Description, "Confidential") {
log.Printf("Storage class contains Confidential: %s\n%s\n", storageClass, data.Description)
continue
}
Comment on lines +279 to +282
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm moving this after the first storageClass check since that one should take priority.

if pm.Storage[data.Region].Storage[storageClass] != 0 {
log.Printf("Storage class %s already exists in region %s", storageClass, data.Region)
continue
Expand All @@ -293,85 +300,6 @@ func (pm *PricingMap) Populate(ctx context.Context, billingService *billingv1.Cl
return nil
}

// Paula: deprecate this function in favour of func (pm *PricingMap) Populate(skus []*billingpb.Sku) (*PricingMap, error)
func GeneratePricingMap(skus []*billingpb.Sku) (*PricingMap, error) {
if len(skus) == 0 {
return &PricingMap{}, ErrSkuNotFound
}
pricingMap := NewComputePricingMap()
for _, sku := range skus {
rawData, err := getDataFromSku(sku)
if errors.Is(err, ErrSkuNotRelevant) {
continue
}
if errors.Is(err, ErrPricingDataIsOff) {
continue
}
if errors.Is(err, ErrSkuNotParsable) {
continue
}
if err != nil {
return nil, err
}
for _, data := range rawData {
switch data.ComputeResource {
case Ram, Cpu:
if _, ok := pricingMap.Compute[data.Region]; !ok {
pricingMap.Compute[data.Region] = NewMachineTypePricing()
}
if _, ok := pricingMap.Compute[data.Region].Family[data.Description]; !ok {
pricingMap.Compute[data.Region].Family[data.Description] = NewPriceTiers()
}
floatPrice := float64(data.Price) * 1e-9
priceTier := pricingMap.Compute[data.Region].Family[data.Description]
if data.PriceTier == Spot {
if data.ComputeResource == Ram {
priceTier.Spot.Ram = floatPrice
continue
}
priceTier.Spot.Cpu = floatPrice
continue
}
if data.ComputeResource == Ram {
priceTier.OnDemand.Ram = floatPrice
continue
}
priceTier.OnDemand.Cpu = floatPrice
case Storage:
// Right now this is somewhat tightly coupled to GKE persistent volumes.
// In GKE you can only provision the following classes: https://cloud.google.com/kubernetes-engine/docs/how-to/persistent-volumes/gce-pd-csi-driver#create_a_storageclass
// For extreme disks, we are ignoring the cost of IOPs, which would be a significant cost(could double cost of disk)
// TODO(pokom): Add support for other storage classes
// TODO(pokom): Add support for IOps operations
if _, ok := pricingMap.Storage[data.Region]; !ok {
pricingMap.Storage[data.Region] = NewStoragePricing()
}
storageClass := ""
for description, sc := range storageClasses {
// We check to see if the description starts with the storage class name
// This is primarily because this could return a false positive in cases of Regional storage which
// has a similar description.
if strings.Index(data.Description, description) == 0 {
storageClass = sc
// Break to prevent overwritting the storage class
break
}
}
if storageClass == "" {
log.Printf("Storage class not found for %s. Skipping", data.Description)
continue
}
if pricingMap.Storage[data.Region].Storage[storageClass] != 0 {
log.Printf("Storage class %s already exists in region %s", storageClass, data.Region)
continue
}
pricingMap.Storage[data.Region].Storage[storageClass] = float64(data.Price) * 1e-9 / utils.HoursInMonth
}
}
}
return pricingMap, nil
}

var ignoreList = []string{
"Network",
"Nvidia",
Expand Down
14 changes: 7 additions & 7 deletions pkg/google/gke/pricing_map_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,17 +107,13 @@ func TestStructuredPricingMap_GetCostOfInstance(t *testing.T) {
}
}

func TestGeneratePricingMap(t *testing.T) {
func TestPricingMapParseSkus(t *testing.T) {
for _, tc := range []struct {
name string
skus []*billingpb.Sku
expectedPricingMap *PricingMap
expectedError error
}{
{
name: "no skus",
expectedError: ErrSkuNotFound,
},
{
name: "empty sku, empty pricing map",
skus: []*billingpb.Sku{{}},
Expand Down Expand Up @@ -644,13 +640,17 @@ func TestGeneratePricingMap(t *testing.T) {
},
} {
t.Run(tc.name, func(t *testing.T) {
pm, err := GeneratePricingMap(tc.skus)
pricingMap := &PricingMap{
Compute: make(map[string]*FamilyPricing),
Storage: make(map[string]*StoragePricing),
}
Comment on lines +643 to +646
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Specifically not calling NewPricingMap(...) because that method will also call populate. I'm specifically trying to avoid having to mock out the billing client.

err := pricingMap.ParseSkus(tc.skus)
if tc.expectedError != nil {
require.EqualError(t, err, tc.expectedError.Error())
return
}
require.NoError(t, err)
require.Equal(t, tc.expectedPricingMap, pm)
require.Equal(t, tc.expectedPricingMap, pricingMap)
})
}
}
Expand Down
Loading