Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Masami created an issue. See original summary.

Masami’s picture

Issue summary: View changes
Masami’s picture

Status: Active » Needs review
FileSize
72.11 KB
Masami’s picture

FileSize
72.71 KB
yas’s picture

@masami

Thank you for adding the block. I tested Basically it looks good to me.

@masatotakada

What do you think?

Masami’s picture

FileSize
72.71 KB
Masami’s picture

FileSize
78.39 KB
Masami’s picture

yas’s picture

Version: 3.0.0-alpha1 » 3.x-dev
MasatoTakada’s picture

@Masami

Thank you for the patches. It looks good to me except for one question that
is it okay if Cost_type field with string format has any integer number at the line 138 and add it at the line 148??
In my understanding, this field has the type of cost ,like 'on_demand_yearly' and so on, from resource entity.
So, I am worried about zero mismatches it and then it might causes the error.

diff --git a/modules/cloud_service_providers/k8s/src/Plugin/cloud_cost_storage/K8sCloudCostStoragePlugin.php b/modules/cloud_service_providers/k8s/src/Plugin/cloud_cost_storage/K8sCloudCostStoragePlugin.php
index fd0e0a4c..8eca5f62 100644
--- a/modules/cloud_service_providers/k8s/src/Plugin/cloud_cost_storage/K8sCloudCostStoragePlugin.php
+++ b/modules/cloud_service_providers/k8s/src/Plugin/cloud_cost_storage/K8sCloudCostStoragePlugin.php
@@ -135,6 +135,7 @@ class K8sCloudCostStoragePlugin extends CloudCostStorageBase {
           if (!isset($result[$label][$time_range])) {
             $result[$label][$time_range] = [
               'total_cost' => 0,
+              'cost_type' => 0,
               'count' => 0,
               'resources' => [
                 'cpu_usage' => 0,
@@ -144,6 +145,7 @@ class K8sCloudCostStoragePlugin extends CloudCostStorageBase {
             ];
           }
           $result[$label][$time_range]['total_cost'] += $resource_entity->get('cost')->value;
+          $result[$label][$time_range]['cost_type'] += $resource_entity->get('cost_type')->value;
           $result[$label][$time_range]['count'] += 1;
           $resources = Yaml::decode($resource_entity->getResources());
           $result[$label][$time_range]['resources']['cpu_usage'] += $resources['cpu_usage'];
@@ -175,7 +177,7 @@ class K8sCloudCostStoragePlugin extends CloudCostStorageBase {
           'memory_usage_avg' => $memory_usage_avg,
           'pod_usage_avg' => $pod_usage_avg,
         ];
-        $this->save('cloud_cost_storage', 'k8s', $label, $avg_cost, Yaml::encode($resources), $key);
+        $this->save('cloud_cost_storage', 'k8s', $label, $value['cost_type'], $avg_cost, Yaml::encode($resources), $key);
       }
     }
   }
Masami’s picture

@yas
@masatotakada
Thank you for your review. I update the patches, so please review them again.

MasatoTakada’s picture

@masami

Thank you for work. It looks good to me.

yas’s picture

Status: Needs review » Reviewed & tested by the community

@masatotakada

Thank you for you review.

@masami

Thank you for the update. I'll merge the patch to 8.x-2.x and 3.x and close this issue as Fixed.

  • yas committed ca1c238 on 8.x-2.x authored by Masami
    Issue #3168861 by Masami, yas, MasatoTakada: Add a block to display the...

  • yas committed 9242de5 on 3.x authored by Masami
    Issue #3168861 by Masami, yas, MasatoTakada: Add a block to display the...
yas’s picture

Status: Reviewed & tested by the community » Fixed

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.