From 98bdab8fca735201efe2a785b6c20003e1d9058f Mon Sep 17 00:00:00 2001 From: Ward Vandewege Date: Tue, 29 Jun 2021 17:36:37 -0400 Subject: [PATCH] 17841: address review comments. Arvados-DCO-1.1-Signed-off-by: Ward Vandewege --- lib/costanalyzer/costanalyzer.go | 64 ++++++++++++++------------- lib/costanalyzer/costanalyzer_test.go | 20 ++++----- 2 files changed, 44 insertions(+), 40 deletions(-) diff --git a/lib/costanalyzer/costanalyzer.go b/lib/costanalyzer/costanalyzer.go index 3fd8097f82..4a48db1a8b 100644 --- a/lib/costanalyzer/costanalyzer.go +++ b/lib/costanalyzer/costanalyzer.go @@ -39,11 +39,16 @@ type nodeInfo struct { Preemptible bool } -type containerInfo struct { +type consumption struct { cost float64 duration float64 } +func (c *consumption) Add(n consumption) { + c.cost += n.cost + c.duration += n.duration +} + type arrayFlags []string func (i *arrayFlags) String() string { @@ -194,7 +199,9 @@ func ensureDirectory(logger *logrus.Logger, dir string) (err error) { return } -func addContainerLine(logger *logrus.Logger, node nodeInfo, cr arvados.ContainerRequest, container arvados.Container) (csv string, cost float64, duration float64) { +func addContainerLine(logger *logrus.Logger, node nodeInfo, cr arvados.ContainerRequest, container arvados.Container) (string, consumption) { + var csv string + var containerConsumption consumption csv = cr.UUID + "," csv += cr.Name + "," csv += container.UUID + "," @@ -222,10 +229,10 @@ func addContainerLine(logger *logrus.Logger, node nodeInfo, cr arvados.Container price = node.Price size = node.ProviderType } - cost = delta.Seconds() / 3600 * price - duration = delta.Seconds() - csv += size + "," + fmt.Sprintf("%+v", node.Preemptible) + "," + strconv.FormatFloat(price, 'f', 8, 64) + "," + strconv.FormatFloat(cost, 'f', 8, 64) + "\n" - return + containerConsumption.cost = delta.Seconds() / 3600 * price + containerConsumption.duration = delta.Seconds() + csv += size + "," + fmt.Sprintf("%+v", node.Preemptible) + "," + strconv.FormatFloat(price, 'f', 8, 64) + "," + strconv.FormatFloat(containerConsumption.cost, 'f', 8, 64) + "\n" + return csv, containerConsumption } func loadCachedObject(logger *logrus.Logger, file string, uuid string, object interface{}) (reload bool) { @@ -360,8 +367,8 @@ func getNode(arv *arvadosclient.ArvadosClient, ac *arvados.Client, kc *keepclien return } -func handleProject(logger *logrus.Logger, uuid string, arv *arvadosclient.ArvadosClient, ac *arvados.Client, kc *keepclient.KeepClient, resultsDir string, cache bool) (cost map[string]containerInfo, err error) { - cost = make(map[string]containerInfo) +func handleProject(logger *logrus.Logger, uuid string, arv *arvadosclient.ArvadosClient, ac *arvados.Client, kc *keepclient.KeepClient, resultsDir string, cache bool) (cost map[string]consumption, err error) { + cost = make(map[string]consumption) var project arvados.Group err = loadObject(logger, ac, uuid, uuid, cache, &project) @@ -394,11 +401,11 @@ func handleProject(logger *logrus.Logger, uuid string, arv *arvadosclient.Arvado items := value.([]interface{}) for _, item := range items { itemMap := item.(map[string]interface{}) - crCsv, err := generateCrCsv(logger, itemMap["uuid"].(string), arv, ac, kc, resultsDir, cache) + crInfo, err := generateCrInfo(logger, itemMap["uuid"].(string), arv, ac, kc, resultsDir, cache) if err != nil { return nil, fmt.Errorf("error generating container_request CSV: %s", err.Error()) } - for k, v := range crCsv { + for k, v := range crInfo { cost[k] = v } } @@ -408,14 +415,13 @@ func handleProject(logger *logrus.Logger, uuid string, arv *arvadosclient.Arvado return } -func generateCrCsv(logger *logrus.Logger, uuid string, arv *arvadosclient.ArvadosClient, ac *arvados.Client, kc *keepclient.KeepClient, resultsDir string, cache bool) (cost map[string]containerInfo, err error) { +func generateCrInfo(logger *logrus.Logger, uuid string, arv *arvadosclient.ArvadosClient, ac *arvados.Client, kc *keepclient.KeepClient, resultsDir string, cache bool) (cost map[string]consumption, err error) { - cost = make(map[string]containerInfo) + cost = make(map[string]consumption) csv := "CR UUID,CR name,Container UUID,State,Started At,Finished At,Duration in seconds,Compute node type,Preemptible,Hourly node cost,Total cost\n" var tmpCsv string - var tmpTotalCost, tmpTotalDuration float64 - var totalCost, totalDuration float64 + var total, tmpTotal consumption logger.Debugf("Processing %s", uuid) var crUUID = uuid @@ -458,10 +464,9 @@ func generateCrCsv(logger *logrus.Logger, uuid string, arv *arvadosclient.Arvado logger.Errorf("Skipping container request %s: error getting node %s: %s", cr.UUID, cr.UUID, err) return nil, nil } - tmpCsv, totalCost, totalDuration = addContainerLine(logger, topNode, cr, container) + tmpCsv, total = addContainerLine(logger, topNode, cr, container) csv += tmpCsv - totalCost += tmpTotalCost - cost[container.UUID] = containerInfo{cost: totalCost, duration: totalDuration} + cost[container.UUID] = total // Find all container requests that have the container we found above as requesting_container_uuid var childCrs arvados.ContainerRequestList @@ -498,14 +503,14 @@ func generateCrCsv(logger *logrus.Logger, uuid string, arv *arvadosclient.Arvado if err != nil { return nil, fmt.Errorf("error loading object %s: %s", cr2.ContainerUUID, err) } - tmpCsv, tmpTotalCost, tmpTotalDuration = addContainerLine(logger, node, cr2, c2) - cost[cr2.ContainerUUID] = containerInfo{cost: tmpTotalCost, duration: tmpTotalDuration} + tmpCsv, tmpTotal = addContainerLine(logger, node, cr2, c2) + cost[cr2.ContainerUUID] = tmpTotal csv += tmpCsv - totalCost += tmpTotalCost + total.Add(tmpTotal) } logger.Debug("Done collecting child containers") - csv += "TOTAL,,,,," + strconv.FormatFloat(totalDuration, 'f', 3, 64) + ",,,," + strconv.FormatFloat(totalCost, 'f', 2, 64) + "\n" + csv += "TOTAL,,,,,," + strconv.FormatFloat(total.duration, 'f', 3, 64) + ",,,," + strconv.FormatFloat(total.cost, 'f', 2, 64) + "\n" if resultsDir != "" { // Write the resulting CSV file @@ -590,7 +595,7 @@ func (c *command) costAnalyzer(prog string, args []string, logger *logrus.Logger } }() - cost := make(map[string]containerInfo) + cost := make(map[string]consumption) for uuid := range uuidChannel { logger.Debugf("Considering %s", uuid) @@ -606,14 +611,14 @@ func (c *command) costAnalyzer(prog string, args []string, logger *logrus.Logger } } else if strings.Contains(uuid, "-xvhdp-") || strings.Contains(uuid, "-4zz18-") { // This is a container request - var crCsv map[string]containerInfo - crCsv, err = generateCrCsv(logger, uuid, arv, ac, kc, c.resultsDir, c.cache) + var crInfo map[string]consumption + crInfo, err = generateCrInfo(logger, uuid, arv, ac, kc, c.resultsDir, c.cache) if err != nil { err = fmt.Errorf("error generating CSV for uuid %s: %s", uuid, err.Error()) exitcode = 2 return } - for k, v := range crCsv { + for k, v := range crInfo { cost[k] = v } } else if strings.Contains(uuid, "-tpzed-") { @@ -641,14 +646,13 @@ func (c *command) costAnalyzer(prog string, args []string, logger *logrus.Logger csv += "# " + uuid + "\n" } - var totalCost, totalDuration float64 + var total consumption for k, v := range cost { csv += k + "," + strconv.FormatFloat(v.duration, 'f', 3, 64) + "," + strconv.FormatFloat(v.cost, 'f', 8, 64) + "\n" - totalCost += v.cost - totalDuration += v.duration + total.Add(v) } - csv += "TOTAL," + strconv.FormatFloat(totalDuration, 'f', 3, 64) + "," + strconv.FormatFloat(totalCost, 'f', 2, 64) + "\n" + csv += "TOTAL," + strconv.FormatFloat(total.duration, 'f', 3, 64) + "," + strconv.FormatFloat(total.cost, 'f', 2, 64) + "\n" if c.resultsDir != "" { // Write the resulting CSV file @@ -663,7 +667,7 @@ func (c *command) costAnalyzer(prog string, args []string, logger *logrus.Logger } // Output the total dollar amount on stdout - fmt.Fprintf(stdout, "%s\n", strconv.FormatFloat(totalCost, 'f', 2, 64)) + fmt.Fprintf(stdout, "%s\n", strconv.FormatFloat(total.cost, 'f', 2, 64)) return } diff --git a/lib/costanalyzer/costanalyzer_test.go b/lib/costanalyzer/costanalyzer_test.go index 512c33c6ed..9fee66e1dd 100644 --- a/lib/costanalyzer/costanalyzer_test.go +++ b/lib/costanalyzer/costanalyzer_test.go @@ -171,8 +171,8 @@ func (*Suite) TestTimestampRange(c *check.C) { uuid2Report, err := ioutil.ReadFile(resultsDir + "/" + arvadostest.CompletedDiagnosticsContainerRequest2UUID + ".csv") c.Assert(err, check.IsNil) - c.Check(string(uuidReport), check.Matches, "(?ms).*TOTAL,,,,,756.789,,,,0.01") - c.Check(string(uuid2Report), check.Matches, "(?ms).*TOTAL,,,,,482.097,,,,0.01") + c.Check(string(uuidReport), check.Matches, "(?ms).*TOTAL,,,,,,763.467,,,,0.01") + c.Check(string(uuid2Report), check.Matches, "(?ms).*TOTAL,,,,,,488.775,,,,0.01") re := regexp.MustCompile(`(?ms).*supplied uuids in (.*?)\n`) matches := re.FindStringSubmatch(stderr.String()) // matches[1] contains a string like 'results/2020-11-02-18-57-45-aggregate-costaccounting.csv' @@ -194,7 +194,7 @@ func (*Suite) TestContainerRequestUUID(c *check.C) { c.Assert(err, check.IsNil) // Make sure the 'preemptible' flag was picked up c.Check(string(uuidReport), check.Matches, "(?ms).*,Standard_E4s_v3,true,.*") - c.Check(string(uuidReport), check.Matches, "(?ms).*TOTAL,,,,,86462.000,,,,7.01") + c.Check(string(uuidReport), check.Matches, "(?ms).*TOTAL,,,,,,86462.000,,,,7.01") re := regexp.MustCompile(`(?ms).*supplied uuids in (.*?)\n`) matches := re.FindStringSubmatch(stderr.String()) // matches[1] contains a string like 'results/2020-11-02-18-57-45-aggregate-costaccounting.csv' @@ -238,7 +238,7 @@ func (*Suite) TestCollectionUUID(c *check.C) { uuidReport, err := ioutil.ReadFile(resultsDir + "/" + arvadostest.CompletedContainerRequestUUID + ".csv") c.Assert(err, check.IsNil) - c.Check(string(uuidReport), check.Matches, "(?ms).*TOTAL,,,,,86462.000,,,,7.01") + c.Check(string(uuidReport), check.Matches, "(?ms).*TOTAL,,,,,,86462.000,,,,7.01") re := regexp.MustCompile(`(?ms).*supplied uuids in (.*?)\n`) matches := re.FindStringSubmatch(stderr.String()) // matches[1] contains a string like 'results/2020-11-02-18-57-45-aggregate-costaccounting.csv' @@ -258,11 +258,11 @@ func (*Suite) TestDoubleContainerRequestUUID(c *check.C) { uuidReport, err := ioutil.ReadFile(resultsDir + "/" + arvadostest.CompletedContainerRequestUUID + ".csv") c.Assert(err, check.IsNil) - c.Check(string(uuidReport), check.Matches, "(?ms).*TOTAL,,,,,86462.000,,,,7.01") + c.Check(string(uuidReport), check.Matches, "(?ms).*TOTAL,,,,,,86462.000,,,,7.01") uuidReport2, err := ioutil.ReadFile(resultsDir + "/" + arvadostest.CompletedContainerRequestUUID2 + ".csv") c.Assert(err, check.IsNil) - c.Check(string(uuidReport2), check.Matches, "(?ms).*TOTAL,,,,,86462.000,,,,42.27") + c.Check(string(uuidReport2), check.Matches, "(?ms).*TOTAL,,,,,,86462.000,,,,42.27") re := regexp.MustCompile(`(?ms).*supplied uuids in (.*?)\n`) matches := re.FindStringSubmatch(stderr.String()) // matches[1] contains a string like 'results/2020-11-02-18-57-45-aggregate-costaccounting.csv' @@ -299,11 +299,11 @@ func (*Suite) TestDoubleContainerRequestUUID(c *check.C) { uuidReport, err = ioutil.ReadFile(resultsDir + "/" + arvadostest.CompletedContainerRequestUUID + ".csv") c.Assert(err, check.IsNil) - c.Check(string(uuidReport), check.Matches, "(?ms).*TOTAL,,,,,86462.000,,,,7.01") + c.Check(string(uuidReport), check.Matches, "(?ms).*TOTAL,,,,,,86462.000,,,,7.01") uuidReport2, err = ioutil.ReadFile(resultsDir + "/" + arvadostest.CompletedContainerRequestUUID2 + ".csv") c.Assert(err, check.IsNil) - c.Check(string(uuidReport2), check.Matches, "(?ms).*TOTAL,,,,,86462.000,,,,42.27") + c.Check(string(uuidReport2), check.Matches, "(?ms).*TOTAL,,,,,,86462.000,,,,42.27") re = regexp.MustCompile(`(?ms).*supplied uuids in (.*?)\n`) matches = re.FindStringSubmatch(stderr.String()) // matches[1] contains a string like 'results/2020-11-02-18-57-45-aggregate-costaccounting.csv' @@ -347,11 +347,11 @@ func (*Suite) TestMultipleContainerRequestUUIDWithReuse(c *check.C) { uuidReport, err := ioutil.ReadFile(resultsDir + "/" + arvadostest.CompletedDiagnosticsContainerRequest1UUID + ".csv") c.Assert(err, check.IsNil) - c.Check(string(uuidReport), check.Matches, "(?ms).*TOTAL,,,,,756.789,,,,0.01") + c.Check(string(uuidReport), check.Matches, "(?ms).*TOTAL,,,,,,763.467,,,,0.01") uuidReport2, err := ioutil.ReadFile(resultsDir + "/" + arvadostest.CompletedDiagnosticsContainerRequest2UUID + ".csv") c.Assert(err, check.IsNil) - c.Check(string(uuidReport2), check.Matches, "(?ms).*TOTAL,,,,,482.097,,,,0.01") + c.Check(string(uuidReport2), check.Matches, "(?ms).*TOTAL,,,,,,488.775,,,,0.01") re := regexp.MustCompile(`(?ms).*supplied uuids in (.*?)\n`) matches := re.FindStringSubmatch(stderr.String()) // matches[1] contains a string like 'results/2020-11-02-18-57-45-aggregate-costaccounting.csv' -- 2.30.2