Skip to content

Conversation

@michaelmdresser
Copy link
Contributor

@michaelmdresser michaelmdresser commented Feb 18, 2023

DO NOT RELEASE UNTIL v1.101 IS RELEASED

What does this PR change?

See release note.

I apologize to reviewers for the intense refactoring. Display options are now substantially clearer as a nice side effect, e.g. Allocation options no longer pollute Asset commands.

How does this PR impact users? (This is the kind of thing that goes in release notes!)

How was this PR tested?

→ go run cmd/kubectl-cost/kubectl-cost.go predict -r michaelkc -f ./test/multi.yaml              
+--------------------------------------------+------------+------------+-------------+-------------+
| WORKLOAD                                   | Δ CPU/MO   | Δ MEM/MO   | Δ GPU/MO    | Δ TOTAL/MO  |
+--------------------------------------------+------------+------------+-------------+-------------+
| michaelkc/deployment/nginx-deployment      | 209.47 USD |  18.72 USD |    0.00 USD |  228.18 USD |
| kubecost/deployment/kubecost-cost-analyzer |   0.35 USD |   0.03 USD |    0.00 USD |    0.38 USD |
| michaelkc/deployment/nginx-deployment-2    | 232.74 USD | 109.17 USD | 3467.50 USD | 3809.41 USD |
| michaelkc/pod/nginx-pod                    |   8.15 USD |   0.61 USD |    0.00 USD |    8.76 USD |
| michaelkc/pod/nginx-pod                    |   8.15 USD |   0.61 USD | 2080.50 USD | 2089.26 USD |
+--------------------------------------------+------------+------------+-------------+-------------+
|                                            | 458.85 USD | 129.13 USD | 5548.00 USD | 6135.98 USD |
+--------------------------------------------+------------+------------+-------------+-------------+
→ go run cmd/kubectl-cost/kubectl-cost.go predict -r michaelkc -f ./test/multi.yaml --only-after --only-diff=false
+--------------------------------------------+------------+------------+-------------+-------------+
| WORKLOAD                                   | CPU/MO     | MEM/MO     | GPU/MO      | TOTAL/MO    |
+--------------------------------------------+------------+------------+-------------+-------------+
| michaelkc/deployment/nginx-deployment      | 209.47 USD |  18.72 USD |    0.00 USD |  228.18 USD |
| kubecost/deployment/kubecost-cost-analyzer |   0.35 USD |   0.03 USD |    0.00 USD |    0.38 USD |
| michaelkc/deployment/nginx-deployment-2    | 232.74 USD | 109.17 USD | 3467.50 USD | 3809.41 USD |
| michaelkc/pod/nginx-pod                    |   8.15 USD |   0.61 USD |    0.00 USD |    8.76 USD |
| michaelkc/pod/nginx-pod                    |   8.15 USD |   0.61 USD | 2080.50 USD | 2089.26 USD |
+--------------------------------------------+------------+------------+-------------+-------------+
|                                            | 458.85 USD | 129.13 USD | 5548.00 USD | 6135.98 USD |
+--------------------------------------------+------------+------------+-------------+-------------+
→ go run cmd/kubectl-cost/kubectl-cost.go predict -r michaelkc -f ./test/multi.yaml --only-diff=false 
+--------------------------------------------+------------+------------+-------------+-------------+------------+------------+-------------+-------------+
| WORKLOAD                                   | CPU/MO     | MEM/MO     | GPU/MO      | TOTAL/MO    | Δ CPU/MO   | Δ MEM/MO   | Δ GPU/MO    | Δ TOTAL/MO  |
+--------------------------------------------+------------+------------+-------------+-------------+------------+------------+-------------+-------------+
| michaelkc/deployment/nginx-deployment      | 209.47 USD |  18.72 USD |    0.00 USD |  228.18 USD | 209.47 USD |  18.72 USD |    0.00 USD |  228.18 USD |
| kubecost/deployment/kubecost-cost-analyzer |   0.35 USD |   0.03 USD |    0.00 USD |    0.38 USD |   0.35 USD |   0.03 USD |    0.00 USD |    0.38 USD |
| michaelkc/deployment/nginx-deployment-2    | 232.74 USD | 109.17 USD | 3467.50 USD | 3809.41 USD | 232.74 USD | 109.17 USD | 3467.50 USD | 3809.41 USD |
| michaelkc/pod/nginx-pod                    |   8.15 USD |   0.61 USD |    0.00 USD |    8.76 USD |   8.15 USD |   0.61 USD |    0.00 USD |    8.76 USD |
| michaelkc/pod/nginx-pod                    |   8.15 USD |   0.61 USD | 2080.50 USD | 2089.26 USD |   8.15 USD |   0.61 USD | 2080.50 USD | 2089.26 USD |
+--------------------------------------------+------------+------------+-------------+-------------+------------+------------+-------------+-------------+
|                                            | 458.85 USD | 129.13 USD | 5548.00 USD | 6135.98 USD | 458.85 USD | 129.13 USD | 5548.00 USD | 6135.98 USD |
+--------------------------------------------+------------+------------+-------------+-------------+------------+------------+-------------+-------------+

Have you made an update to documentation?

@michaelmdresser michaelmdresser marked this pull request as ready for review February 27, 2023 23:13
@michaelmdresser michaelmdresser changed the title Switch 'kubectl cost predict' to use /prediction/speccost; keep old logic under 'oldpredict' Switch 'kubectl cost predict' to use /prediction/speccost Feb 27, 2023
Copy link

@nikovacevic nikovacevic left a comment

Choose a reason for hiding this comment

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

LGTM! (Still support others reviewing, but if you need to merge go for it.)

@michaelmdresser michaelmdresser merged commit f54a3b2 into main Mar 1, 2023
@michaelmdresser michaelmdresser deleted the mmd/use-new-spec-predict branch March 1, 2023 21:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants