POL-1405 Replace deprecated Kubecost endpoints for Kubecost Cluster Policy #2833

midir99 · 2024-11-14T17:31:14Z

Description

The endpoint we are currently using has been deprecated “/model/savings/clusterSizing"
It still works but only returns one cluster rather than all clusters.

The new endpoint is returns a different response “/model/savings/clusterSizingETL" and simply adding ETL onto the end of our current end point in the policy returns null values in ds_cluster_sizing

This endpoint returns all cluster recommendations in one response.

The ds_clusters function has been created to include two new parameters in the Kubecost API call:

param_allow_shared_core: This boolean parameter indicates whether shared cores should be allowed in the cluster sizing calculation. By default, Kubecost may not consider the shared cores between pods as part of the cost savings calculations. With this new parameter, the explicit inclusion of shared cores in the calculations can be enabled. This is useful for environments where resources are shared between multiple pods, and a more accurate estimate of potential savings is desired.
param_include_overhead: This boolean parameter specifies whether overhead should be included in the calculation. In many cases, it is necessary to consider additional costs, such as overhead costs associated with resource management, when calculating savings. With this parameter, the option to include these additional costs is provided, allowing for a more accurate representation of the total savings.

Changes Made:

Updated the function responsible for the API call to /model/savings/clusterSizing
Added the param_allow_shared_core and param_include_overhead parameters to the ds_clusters function.
Updated the HTTP call to send these parameters in the Request.
Set the default values of these parameters to False, meaning shared cores will not be allowed, and overhead will not be included by default.

How the response for the new endpoint differs from what we initially wanted: we modified it and added new functions to handle the response and convert it to the expected struct

Issues Resolved

Deprecated endpoint from kubecost was replaced.
https://flexera.atlassian.net/browse/POL-1405

Link to Example Applied Policy

https://app.flexeratest.com/orgs/1105/automation/applied-policies/projects/60073?policyId=673520d66168ce351f89f548

Contribution Check List

New functionality includes testing.
New functionality has been documented in the README if applicable
New functionality has been documented in CHANGELOG.MD

github-actions · 2024-11-14T17:32:47Z

	2 Errors
🚫	cost/kubecost/cluster/CHANGELOG.md Markdown syntax errors found: cost/kubecost/cluster/CHANGELOG.md:8: MD012 Multiple consecutive blank lines Further documentation is available for these failures: MD012: https://github.com/markdownlint/markdownlint/blob/main/docs/RULES.md#md012—multiple-consecutive-blank-lines
🚫	cost/kubecost/cluster/kubecost_cluster_rightsizing_recommendations.pt Version number in policy template does not match latest version number in `CHANGELOG.md`. Please review both files to make sure they are correct and aligned with each other. Orphaned `script` code blocks found. Blocks that are not used anywhere in the policy should be removed: js_get_cluster_list parameter code blocks found with out of order fields. Fields should be in the following order: type, category, label, description, allowed_values, allowed_pattern, min_value, max_value, default Line 39: parameter "param_allow_shared_core" Line 47: parameter "param_include_overhead"

	1 Warning
⚠️	cost/kubecost/cluster/kubecost_cluster_rightsizing_recommendations.pt Policy template updated but associated README.md file has not been. Please verify that any necessary changes have been made to the README. parameter code blocks with missing `default` field found. Please add the `default` field to these blocks: Line 39 Line 47 Optionally, you can add a comment within the parameter code blocks to indicate that the parameter requires user input and avoid this message. `# No default value, user input required` While not required, it is recommended that every parameter have a default value unless user input for that parameter is required and too specific for any default value to make sense Datasources and scripts with mismatched names found. These names should match; for example, a datasource named ds_currency should be paired with a script named js_currency. This convention should only be ignored when the same script is called by multiple datasources. The following datasource/script pairs have mismatched names: Line 211: ds_cluster_sizing / js_cluster_parse Recommendation policy has export that is missing recommended fields. These fields are scraped by the Flexera platform for dashboards: Line 357: tags

Generated by 🚫 Danger

cost/kubecost/cluster/kubecost_cluster_rightsizing_recommendations.pt

XOmniverse

I requested some specific changes, but more broadly, there appear to be a lot more changes here than just updating deprecated API endpoints.

Please fully document all changes in the changelog.

XOmniverse · 2024-11-22T13:36:14Z

cost/kubecost/cluster/CHANGELOG.md

@@ -1,5 +1,9 @@
 # Changelog

+## v0.3.2
+
+- Replace usage of Kubecost deprecated endpoints for newest versions


This is awkwardly worded. Recommended change:

Updated API endpoints for Kubecost. Functionality unchanged.

cost/kubecost/cluster/kubecost_cluster_rightsizing_recommendations.pt

added new params for use of the new endpoint

…red_core and changelog

Rajasundar · 2024-11-29T16:53:48Z

cost/kubecost/cluster/CHANGELOG.md

+## v0.3.3
+
+- Updated API endpoints for Kubecost.
+- Added two new params param_allow_shared_core,param_include_overhead.


we can use backticks or quotes here around the parameter names to indicate they are code or field names.

Rajasundar

@joseluiszuflores, I've added few comments with questions for clarification.

Rajasundar · 2024-11-29T17:06:32Z

cost/kubecost/cluster/kubecost_cluster_rightsizing_recommendations.pt

+  category "Include overhead"
+  label "Include overhead"
+  allowed_values "true", "false"
+  description " By enabling this option, Kubecost takes into account not only the resources used by applications and containers, but also those needed to operate and maintain the Kubernetes environment"


Additional leading space at the beginning can be removed.

Rajasundar · 2024-11-29T17:09:21Z

cost/kubecost/cluster/kubecost_cluster_rightsizing_recommendations.pt

+  result "result"
+  code <<-'EOS'
+  result = []
+  // Loop over each accountKey in data.accountID


comment can be removed, as the code already shows that data.accountID is being iterated.

Rajasundar · 2024-11-29T17:10:04Z

cost/kubecost/cluster/kubecost_cluster_rightsizing_recommendations.pt

+  // Loop over each accountKey in data.accountID
+  for (var accountKey in data.accountID) {
+    var accountData = data.accountID[accountKey];
+    // Extract required fields and push them as a flat object to extractedData


may be, the comment can be removed here as well as the code clearly indicates what it is doing.

Rajasundar · 2024-11-29T17:12:09Z

cost/kubecost/cluster/kubecost_cluster_rightsizing_recommendations.pt

@@ -141,10 +158,10 @@ script "js_currency", type:"javascript" do
  result "result"
  code <<-EOS
  code = ds_kubecost_currency_code['code'].toUpperCase().trim()
+  // using default code.


comment can be removed.

Rajasundar · 2024-11-29T17:58:51Z

cost/kubecost/cluster/kubecost_cluster_rightsizing_recommendations.pt

+        totalNodeCount: crs['totalNodeCount'],
+        totalRAMGB: crs['totalRAMGB'],
+        totalVCPUs: crs['totalVCPUs'],
+        monthlyRate: (Math.round(crs['monthlyRate'] * 100) / 100 ).toFixed(2),


Looks like the precision and return type varies here. Is this intended?

Initial code will return 3 decimals (number) and the new code will return 2 decimals (string).

Rajasundar · 2024-11-29T17:59:23Z

cost/kubecost/cluster/kubecost_cluster_rightsizing_recommendations.pt

+        totalRAMGB: crs['totalRAMGB'],
+        totalVCPUs: crs['totalVCPUs'],
+        monthlyRate: (Math.round(crs['monthlyRate'] * 100) / 100 ).toFixed(2),
+        totalMonthlyCost: (Math.round(recommendation['totalMonthlyCost'] * 100) / 100 ).toFixed(2),


Loooks like the precision and return type varies here. Is this intended?

Initial code will return 3 decimals (number) and the new code will return 2 decimals (string).

Rajasundar · 2024-11-29T17:59:54Z

cost/kubecost/cluster/kubecost_cluster_rightsizing_recommendations.pt

+        monthlyRate: (Math.round(crs['monthlyRate'] * 100) / 100 ).toFixed(2),
+        totalMonthlyCost: (Math.round(recommendation['totalMonthlyCost'] * 100) / 100 ).toFixed(2),
+        // Note: Savings is presented as a negative value. Hence why we multiply by a negative number to invert it
+        savings: (Math.round(recommendation['monthlySavings'] * 100) / 100 ).toFixed(2),


Loooks like the precision and return type varies here. Is this intended and not inline with the comment in line no. 324

Initial code will return 3 decimals (number) and the new code will return 2 decimals (string).

Rajasundar · 2024-11-29T18:14:23Z

cost/kubecost/cluster/kubecost_cluster_rightsizing_recommendations.pt

 end

 script "js_recommendations", type: "javascript" do
-  parameters "ds_cluster_sizing", "ds_currency", "ds_applied_policy", "param_strategy", "param_min_nodes", "param_lookback", "param_target_util"
+  parameters "ds_cluster_sizing", "ds_currency","ds_applied_policy", "param_strategy", "param_min_nodes", "param_lookback", "param_target_util"


Inconsistency in spacing.

Suggested change

parameters "ds_cluster_sizing", "ds_currency","ds_applied_policy", "param_strategy", "param_min_nodes", "param_lookback", "param_target_util"

parameters "ds_cluster_sizing", "ds_currency", "ds_applied_policy", "param_strategy", "param_min_nodes", "param_lookback", "param_target_util"

Rajasundar · 2024-11-29T18:15:10Z

cost/kubecost/cluster/kubecost_cluster_rightsizing_recommendations.pt

 datasource "ds_recommendations" do
-  run_script $js_recommendations, $ds_cluster_sizing, $ds_currency, $ds_applied_policy, $param_strategy, $param_min_nodes, $param_lookback, $param_target_util
+  run_script $js_recommendations, $ds_cluster_sizing, $ds_currency, $ds_applied_policy,$param_strategy, $param_min_nodes, $param_lookback, $param_target_util


Inconsistency in spacing.

Suggested change

run_script $js_recommendations, $ds_cluster_sizing, $ds_currency, $ds_applied_policy,$param_strategy, $param_min_nodes, $param_lookback, $param_target_util

run_script $js_recommendations, $ds_cluster_sizing, $ds_currency, $ds_applied_policy, $param_strategy, $param_min_nodes, $param_lookback, $param_target_util

nia-vf1 · 2024-12-02T09:52:33Z

cost/kubecost/cluster/kubecost_cluster_rightsizing_recommendations.pt

+  description "By enabling this option, Kubecost takes into account not only the resources used by applications and containers, but also those needed to operate and maintain the Kubernetes environment"
+end
+
+


Suggested change

Rajasundar

LGTM

POL-1405 Replace deprecated Kubecost endpoints

f777b41

midir99 added READY-FOR-REVIEW READY FOR APPROVAL labels Nov 14, 2024

midir99 self-assigned this Nov 14, 2024

midir99 requested a review from a team as a code owner November 14, 2024 17:31

XOmniverse requested changes Nov 14, 2024

View reviewed changes

cost/kubecost/cluster/kubecost_cluster_rightsizing_recommendations.pt Outdated Show resolved Hide resolved

Fix review comments

0e64f97

midir99 requested review from XOmniverse and joseluiszuflores November 19, 2024 19:38

Merge branch 'master' into POL-1405_fix_kubecost_policy

7480f2c

XOmniverse requested changes Nov 22, 2024

View reviewed changes

nia-vf1 reviewed Nov 25, 2024

View reviewed changes

cost/kubecost/cluster/kubecost_cluster_rightsizing_recommendations.pt Outdated Show resolved Hide resolved

midir99 requested review from Rajasundar and removed request for joseluiszuflores November 25, 2024 15:50

Jose Zuniga added 6 commits November 27, 2024 15:58

solved comments about fuctions doesn't use and add params

4de6f23

added new params for use of the new endpoint

squash! Fix review comments

041185e

squash! Fix review comments

d4942b0

reverted formatNumber

abc7bbe

update identation with js fn

54ce420

updated description of params param_include_overhead, param_allow_sha…

d9e9b9e

…red_core and changelog

Rajasundar reviewed Nov 29, 2024

View reviewed changes

Rajasundar requested changes Nov 29, 2024

View reviewed changes

joseluiszuflores requested review from XOmniverse and nia-vf1 November 29, 2024 18:47

Jose Zuniga added 2 commits November 29, 2024 15:23

fix incositency space and changedlog file

c4aba19

fixed monthlyRate,totalMonthlyCost, savings with 3 decimals

36a12eb

nia-vf1 reviewed Dec 2, 2024

View reviewed changes

fix template

913b886

Rajasundar previously approved these changes Dec 2, 2024

View reviewed changes

fix correct fields on totalMonthlyCost and Savings

29f836d

joseluiszuflores dismissed Rajasundar’s stale review via 29f836d December 4, 2024 12:51

removed line in blank

ae4dee3

joseluiszuflores requested review from nia-vf1 and Rajasundar December 4, 2024 19:11

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

POL-1405 Replace deprecated Kubecost endpoints for Kubecost Cluster Policy #2833

POL-1405 Replace deprecated Kubecost endpoints for Kubecost Cluster Policy #2833

midir99 commented Nov 14, 2024 •

edited by joseluiszuflores

Loading

github-actions bot commented Nov 14, 2024 •

edited

Loading

cost/kubecost/cluster/CHANGELOG.md

cost/kubecost/cluster/kubecost_cluster_rightsizing_recommendations.pt

cost/kubecost/cluster/kubecost_cluster_rightsizing_recommendations.pt

XOmniverse left a comment

XOmniverse Nov 22, 2024

Rajasundar Nov 29, 2024

Rajasundar left a comment

Rajasundar Nov 29, 2024

Rajasundar Nov 29, 2024

Rajasundar Nov 29, 2024

Rajasundar Nov 29, 2024

Rajasundar Nov 29, 2024

Rajasundar Nov 29, 2024

Rajasundar Nov 29, 2024

Rajasundar Nov 29, 2024

Rajasundar Nov 29, 2024

nia-vf1 Dec 2, 2024

Rajasundar left a comment

	parameters "ds_cluster_sizing", "ds_currency","ds_applied_policy", "param_strategy", "param_min_nodes", "param_lookback", "param_target_util"
	parameters "ds_cluster_sizing", "ds_currency", "ds_applied_policy", "param_strategy", "param_min_nodes", "param_lookback", "param_target_util"

	run_script $js_recommendations, $ds_cluster_sizing, $ds_currency, $ds_applied_policy,$param_strategy, $param_min_nodes, $param_lookback, $param_target_util
	run_script $js_recommendations, $ds_cluster_sizing, $ds_currency, $ds_applied_policy, $param_strategy, $param_min_nodes, $param_lookback, $param_target_util

		description "By enabling this option, Kubecost takes into account not only the resources used by applications and containers, but also those needed to operate and maintain the Kubernetes environment"
		end

POL-1405 Replace deprecated Kubecost endpoints for Kubecost Cluster Policy #2833

Are you sure you want to change the base?

POL-1405 Replace deprecated Kubecost endpoints for Kubecost Cluster Policy #2833

Conversation

midir99 commented Nov 14, 2024 • edited by joseluiszuflores Loading

Description

Changes Made:

Issues Resolved

Link to Example Applied Policy

Contribution Check List

github-actions bot commented Nov 14, 2024 • edited Loading

cost/kubecost/cluster/CHANGELOG.md

cost/kubecost/cluster/kubecost_cluster_rightsizing_recommendations.pt

cost/kubecost/cluster/kubecost_cluster_rightsizing_recommendations.pt

XOmniverse left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Rajasundar left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Rajasundar left a comment

Choose a reason for hiding this comment

midir99 commented Nov 14, 2024 •

edited by joseluiszuflores

Loading

github-actions bot commented Nov 14, 2024 •

edited

Loading