[ARVADOS] updated: 1.3.0-3261-g8100fb55f

Git user git at public.arvados.org
Sat Nov 14 01:47:25 UTC 2020


Summary of changes:
 lib/costanalyzer/{command.go => cmd.go} |   5 +-
 lib/costanalyzer/costanalyzer.go        | 451 ++++++++++++++------------------
 lib/costanalyzer/costanalyzer_test.go   |  68 +++--
 3 files changed, 251 insertions(+), 273 deletions(-)
 rename lib/costanalyzer/{command.go => cmd.go} (85%)

       via  8100fb55fd4f328870625993caf91884df11d2d6 (commit)
       via  b215c29447ea3b7c0c066cc4262308f26f6629fa (commit)
      from  9adb8ea50fcf555c7ec73cb0924c869af2345f86 (commit)

Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.


commit 8100fb55fd4f328870625993caf91884df11d2d6
Author: Ward Vandewege <ward at curii.com>
Date:   Fri Nov 13 16:22:57 2020 -0500

    16950: more changes after review.
    
    Arvados-DCO-1.1-Signed-off-by: Ward Vandewege <ward at curii.com>

diff --git a/lib/costanalyzer/cmd.go b/lib/costanalyzer/cmd.go
index 6829be7d3..800860ddf 100644
--- a/lib/costanalyzer/cmd.go
+++ b/lib/costanalyzer/cmd.go
@@ -28,7 +28,7 @@ func (command) RunCommand(prog string, args []string, stdin io.Reader, stdout, s
 	logger := ctxlog.New(stderr, "text", "info")
 	defer func() {
 		if err != nil {
-			logger.WithError(err).Error("fatal")
+			logger.Error("\n" + err.Error() + "\n")
 		}
 	}()
 
@@ -38,7 +38,7 @@ func (command) RunCommand(prog string, args []string, stdin io.Reader, stdout, s
 	loader := config.NewLoader(stdin, logger)
 	loader.SkipLegacy = true
 
-	exitcode := costanalyzer(prog, args, loader, logger, stdout, stderr)
+	exitcode, err := costanalyzer(prog, args, loader, logger, stdout, stderr)
 
 	return exitcode
 }
diff --git a/lib/costanalyzer/costanalyzer.go b/lib/costanalyzer/costanalyzer.go
index ccd9520bc..8ebf1972f 100644
--- a/lib/costanalyzer/costanalyzer.go
+++ b/lib/costanalyzer/costanalyzer.go
@@ -5,8 +5,6 @@
 package costanalyzer
 
 import (
-	"bytes"
-	"context"
 	"encoding/json"
 	"errors"
 	"flag"
@@ -26,52 +24,17 @@ import (
 	"github.com/sirupsen/logrus"
 )
 
-// LegacyNodeInfo is a struct for records created by Arvados Node Manager (Arvados <= 1.4.3)
-// Example:
-// {
-//    "total_cpu_cores":2,
-//    "total_scratch_mb":33770,
-//    "cloud_node":
-//      {
-//        "price":0.1,
-//        "size":"m4.large"
-//      },
-//     "total_ram_mb":7986
-// }
-type LegacyNodeInfo struct {
-	CPUCores  int64           `json:"total_cpu_cores"`
-	ScratchMb int64           `json:"total_scratch_mb"`
-	RAMMb     int64           `json:"total_ram_mb"`
-	CloudNode LegacyCloudNode `json:"cloud_node"`
-}
-
-// LegacyCloudNode is a struct for records created by Arvados Node Manager (Arvados <= 1.4.3)
-type LegacyCloudNode struct {
-	Price float64 `json:"price"`
-	Size  string  `json:"size"`
-}
-
-// Node is a struct for records created by Arvados Dispatch Cloud (Arvados >= 2.0.0)
-// Example:
-// {
-//    "Name": "Standard_D1_v2",
-//    "ProviderType": "Standard_D1_v2",
-//    "VCPUs": 1,
-//    "RAM": 3584000000,
-//    "Scratch": 50000000000,
-//    "IncludedScratch": 50000000000,
-//    "AddedScratch": 0,
-//    "Price": 0.057,
-//    "Preemptible": false
-//}
-type Node struct {
-	VCPUs        int64
-	Scratch      int64
-	RAM          int64
-	Price        float64
-	Name         string
+type nodeInfo struct {
+	// Legacy (records created by Arvados Node Manager with Arvados <= 1.4.3)
+	Properties struct {
+		CloudNode struct {
+			Price float64
+			Size  string
+		} `json:"cloud_node"`
+	}
+	// Modern
 	ProviderType string
-	Preemptible  bool
+	Price        float64
 }
 
 type arrayFlags []string
@@ -85,7 +48,7 @@ func (i *arrayFlags) Set(value string) error {
 	return nil
 }
 
-func parseFlags(prog string, args []string, loader *config.Loader, logger *logrus.Logger, stderr io.Writer) (exitCode int, uuids arrayFlags, resultsDir string, cache bool) {
+func parseFlags(prog string, args []string, loader *config.Loader, logger *logrus.Logger, stderr io.Writer) (exitCode int, uuids arrayFlags, resultsDir string, cache bool, err error) {
 	flags := flag.NewFlagSet("", flag.ContinueOnError)
 	flags.SetOutput(stderr)
 	flags.Usage = func() {
@@ -127,12 +90,13 @@ Options:
 `, prog)
 		flags.PrintDefaults()
 	}
-	loglevel := flags.String("log-level", "info", "logging level (debug, info, ...)")
-	resultsDir = *flags.String("output", "results", "output directory for the CSV reports")
-	flags.Var(&uuids, "uuid", "Toplevel project or container request uuid. May be specified more than once.")
+	loglevel := flags.String("log-level", "info", "logging `level` (debug, info, ...)")
+	resultsDir = *flags.String("output", "results", "output `directory` for the CSV reports")
+	flags.Var(&uuids, "uuid", "Toplevel `project or container request` uuid. May be specified more than once.")
 	flags.BoolVar(&cache, "cache", true, "create and use a local disk cache of Arvados objects")
-	err := flags.Parse(args)
+	err = flags.Parse(args)
 	if err == flag.ErrHelp {
+		err = nil
 		exitCode = 1
 		return
 	} else if err != nil {
@@ -141,8 +105,8 @@ Options:
 	}
 
 	if len(uuids) < 1 {
-		logger.Errorf("Error: no uuid(s) provided")
 		flags.Usage()
+		err = fmt.Errorf("Error: no uuid(s) provided")
 		exitCode = 2
 		return
 	}
@@ -161,90 +125,87 @@ func ensureDirectory(logger *logrus.Logger, dir string) (err error) {
 	if os.IsNotExist(err) {
 		err = os.MkdirAll(dir, 0700)
 		if err != nil {
-			return fmt.Errorf("Error creating directory %s: %s\n", dir, err.Error())
+			return fmt.Errorf("error creating directory %s: %s", dir, err.Error())
 		}
 	} else {
 		if !statData.IsDir() {
-			return fmt.Errorf("The path %s is not a directory\n", dir)
+			return fmt.Errorf("the path %s is not a directory", dir)
 		}
 	}
 	return
 }
 
-func addContainerLine(logger *logrus.Logger, node interface{}, cr, container map[string]interface{}) (csv string, cost float64) {
-	csv = cr["uuid"].(string) + ","
-	csv += cr["name"].(string) + ","
-	csv += container["uuid"].(string) + ","
-	csv += container["state"].(string) + ","
-	if container["started_at"] != nil {
-		csv += container["started_at"].(string) + ","
+func addContainerLine(logger *logrus.Logger, node nodeInfo, cr arvados.ContainerRequest, container arvados.Container) (csv string, cost float64) {
+	csv = cr.UUID + ","
+	csv += cr.Name + ","
+	csv += container.UUID + ","
+	csv += string(container.State) + ","
+	if container.StartedAt != nil {
+		csv += container.StartedAt.String() + ","
 	} else {
 		csv += ","
 	}
 
 	var delta time.Duration
-	if container["finished_at"] != nil {
-		csv += container["finished_at"].(string) + ","
-		finishedTimestamp, err := time.Parse("2006-01-02T15:04:05.000000000Z", container["finished_at"].(string))
-		if err != nil {
-			fmt.Println(err)
-		}
-		startedTimestamp, err := time.Parse("2006-01-02T15:04:05.000000000Z", container["started_at"].(string))
-		if err != nil {
-			fmt.Println(err)
-		}
-		delta = finishedTimestamp.Sub(startedTimestamp)
+	if container.FinishedAt != nil {
+		csv += container.FinishedAt.String() + ","
+		delta = container.FinishedAt.Sub(*container.StartedAt)
 		csv += strconv.FormatFloat(delta.Seconds(), 'f', 0, 64) + ","
 	} else {
 		csv += ",,"
 	}
 	var price float64
 	var size string
-	switch n := node.(type) {
-	case Node:
-		price = n.Price
-		size = n.ProviderType
-	case LegacyNodeInfo:
-		price = n.CloudNode.Price
-		size = n.CloudNode.Size
-	default:
-		logger.Warn("WARNING: unknown node type found!")
+	if node.Properties.CloudNode.Price != 0 {
+		price = node.Properties.CloudNode.Price
+		size = node.Properties.CloudNode.Size
+	} else {
+		price = node.Price
+		size = node.ProviderType
 	}
 	cost = delta.Seconds() / 3600 * price
 	csv += size + "," + strconv.FormatFloat(price, 'f', 8, 64) + "," + strconv.FormatFloat(cost, 'f', 8, 64) + "\n"
 	return
 }
 
-func loadCachedObject(logger *logrus.Logger, file string, uuid string) (reload bool, object map[string]interface{}) {
+func loadCachedObject(logger *logrus.Logger, file string, uuid string, object interface{}) (reload bool) {
 	reload = true
 	// See if we have a cached copy of this object
-	if _, err := os.Stat(file); err == nil {
-		data, err := ioutil.ReadFile(file)
-		if err != nil {
-			logger.Errorf("error reading %q: %s", file, err)
-			return
-		}
-		err = json.Unmarshal(data, &object)
-		if err != nil {
-			logger.Errorf("failed to unmarshal json: %s: %s", data, err)
-			return
-		}
+	_, err := os.Stat(file)
+	if err != nil {
+		return
+	}
+	data, err := ioutil.ReadFile(file)
+	if err != nil {
+		logger.Errorf("error reading %q: %s", file, err)
+		return
+	}
+	err = json.Unmarshal(data, &object)
+	if err != nil {
+		logger.Errorf("failed to unmarshal json: %s: %s", data, err)
+		return
+	}
 
-		// See if it is in a final state, if that makes sense
+	// See if it is in a final state, if that makes sense
+	switch v := object.(type) {
+	case arvados.Group:
 		// Projects (j7d0g) do not have state so they should always be reloaded
-		if !strings.Contains(uuid, "-j7d0g-") {
-			if object["state"].(string) == "Complete" || object["state"].(string) == "Failed" {
-				reload = false
-				logger.Debugf("Loaded object %s from local cache (%s)\n", uuid, file)
-				return
-			}
+	case arvados.Container:
+		if v.State == arvados.ContainerStateComplete || v.State == arvados.ContainerStateCancelled {
+			reload = false
+			logger.Debugf("Loaded object %s from local cache (%s)\n", uuid, file)
+		}
+	case arvados.ContainerRequest:
+		if v.State == arvados.ContainerRequestStateFinal {
+			reload = false
+			logger.Debugf("Loaded object %s from local cache (%s)\n", uuid, file)
 		}
 	}
 	return
 }
 
 // Load an Arvados object.
-func loadObject(logger *logrus.Logger, arv *arvadosclient.ArvadosClient, path string, uuid string, cache bool) (object map[string]interface{}, err error) {
+func loadObject(logger *logrus.Logger, arv *arvadosclient.ArvadosClient, path string, uuid string, cache bool, object interface{}) (err error) {
 	err = ensureDirectory(logger, path)
 	if err != nil {
 		return
@@ -256,7 +217,7 @@ func loadObject(logger *logrus.Logger, arv *arvadosclient.ArvadosClient, path st
 	if !cache {
 		reload = true
 	} else {
-		reload, object = loadCachedObject(logger, file, uuid)
+		reload = loadCachedObject(logger, file, uuid, &object)
 	}
 	if !reload {
 		return
@@ -272,89 +233,53 @@ func loadObject(logger *logrus.Logger, arv *arvadosclient.ArvadosClient, path st
 		err = arv.Get("jobs", uuid, nil, &object)
 	}
 	if err != nil {
-		err = fmt.Errorf("Error loading object with UUID %q:\n  %s\n", uuid, err)
+		err = fmt.Errorf("error loading object with UUID %q:\n  %s", uuid, err)
 		return
 	}
 	encoded, err := json.MarshalIndent(object, "", " ")
 	if err != nil {
-		err = fmt.Errorf("Error marshaling object with UUID %q:\n  %s\n", uuid, err)
+		err = fmt.Errorf("error marshaling object with UUID %q:\n  %s", uuid, err)
 		return
 	}
 	err = ioutil.WriteFile(file, encoded, 0644)
 	if err != nil {
-		err = fmt.Errorf("Error writing file %s:\n  %s\n", file, err)
+		err = fmt.Errorf("error writing file %s:\n  %s", file, err)
 		return
 	}
 	return
 }
 
-func getNode(arv *arvadosclient.ArvadosClient, ac *arvados.Client, kc *keepclient.KeepClient, itemMap map[string]interface{}) (node interface{}, err error) {
-	logUuid, ok := itemMap["log_uuid"]
-	if !ok {
+func getNode(arv *arvadosclient.ArvadosClient, ac *arvados.Client, kc *keepclient.KeepClient, cr arvados.ContainerRequest) (node nodeInfo, err error) {
+	if cr.LogUUID == "" {
 		err = errors.New("No log collection")
 		return
 	}
 
 	var collection arvados.Collection
-	err = arv.Get("collections", logUuid.(string), nil, &collection)
+	err = arv.Get("collections", cr.LogUUID, nil, &collection)
 	if err != nil {
-		err = fmt.Errorf("Error getting collection: %s", err)
+		err = fmt.Errorf("error getting collection: %s", err)
 		return
 	}
 
 	var fs arvados.CollectionFileSystem
 	fs, err = collection.FileSystem(ac, kc)
 	if err != nil {
-		err = fmt.Errorf("Error opening collection as filesystem: %s", err)
+		err = fmt.Errorf("error opening collection as filesystem: %s", err)
 		return
 	}
 	var f http.File
 	f, err = fs.Open("node.json")
 	if err != nil {
-		err = fmt.Errorf("Error opening file 'node.json' in collection %s: %s", logUuid.(string), err)
+		err = fmt.Errorf("error opening file 'node.json' in collection %s: %s", cr.LogUUID, err)
 		return
 	}
 
-	var nodeDict map[string]interface{}
-	buf := new(bytes.Buffer)
-	_, err = buf.ReadFrom(f)
+	err = json.NewDecoder(f).Decode(&node)
 	if err != nil {
-		err = fmt.Errorf("Error reading file 'node.json' in collection %s: %s", logUuid.(string), err)
+		err = fmt.Errorf("error reading file 'node.json' in collection %s: %s", cr.LogUUID, err)
 		return
 	}
-	contents := buf.String()
-	f.Close()
-
-	err = json.Unmarshal([]byte(contents), &nodeDict)
-	if err != nil {
-		err = fmt.Errorf("Error unmarshalling: %s", err)
-		return
-	}
-	if val, ok := nodeDict["properties"]; ok {
-		var encoded []byte
-		encoded, err = json.MarshalIndent(val, "", " ")
-		if err != nil {
-			err = fmt.Errorf("Error marshalling: %s", err)
-			return
-		}
-		// node is type LegacyNodeInfo
-		var newNode LegacyNodeInfo
-		err = json.Unmarshal(encoded, &newNode)
-		if err != nil {
-			err = fmt.Errorf("Error unmarshalling: %s", err)
-			return
-		}
-		node = newNode
-	} else {
-		// node is type Node
-		var newNode Node
-		err = json.Unmarshal([]byte(contents), &newNode)
-		if err != nil {
-			err = fmt.Errorf("Error unmarshalling: %s", err)
-			return
-		}
-		node = newNode
-	}
 	return
 }
 
@@ -362,9 +287,10 @@ func handleProject(logger *logrus.Logger, uuid string, arv *arvadosclient.Arvado
 
 	cost = make(map[string]float64)
 
-	project, err := loadObject(logger, arv, resultsDir+"/"+uuid, uuid, cache)
+	var project arvados.Group
+	err = loadObject(logger, arv, resultsDir+"/"+uuid, uuid, cache, &project)
 	if err != nil {
-		return nil, fmt.Errorf("Error loading object %s: %s\n", uuid, err.Error())
+		return nil, fmt.Errorf("error loading object %s: %s", uuid, err.Error())
 	}
 
 	// arv -f uuid container_request list --filters '[["owner_uuid","=","<someuuid>"],["requesting_container_uuid","=",null]]'
@@ -375,7 +301,7 @@ func handleProject(logger *logrus.Logger, uuid string, arv *arvadosclient.Arvado
 		{
 			Attr:     "owner_uuid",
 			Operator: "=",
-			Operand:  project["uuid"].(string),
+			Operand:  project.UUID,
 		},
 		{
 			Attr:     "requesting_container_uuid",
@@ -383,12 +309,12 @@ func handleProject(logger *logrus.Logger, uuid string, arv *arvadosclient.Arvado
 			Operand:  nil,
 		},
 	}
-	err = ac.RequestAndDecodeContext(context.Background(), &childCrs, "GET", "arvados/v1/container_requests", nil, map[string]interface{}{
+	err = ac.RequestAndDecode(&childCrs, "GET", "arvados/v1/container_requests", nil, map[string]interface{}{
 		"filters": filterset,
 		"limit":   10000,
 	})
 	if err != nil {
-		return nil, fmt.Errorf("Error querying container_requests: %s\n", err.Error())
+		return nil, fmt.Errorf("error querying container_requests: %s", err.Error())
 	}
 	if value, ok := childCrs["items"]; ok {
 		logger.Infof("Collecting top level container requests in project %s\n", uuid)
@@ -397,7 +323,7 @@ func handleProject(logger *logrus.Logger, uuid string, arv *arvadosclient.Arvado
 			itemMap := item.(map[string]interface{})
 			crCsv, err := generateCrCsv(logger, itemMap["uuid"].(string), arv, ac, kc, resultsDir, cache)
 			if err != nil {
-				return nil, fmt.Errorf("Error generating container_request CSV: %s\n", err.Error())
+				return nil, fmt.Errorf("error generating container_request CSV: %s", err.Error())
 			}
 			for k, v := range crCsv {
 				cost[k] = v
@@ -419,59 +345,59 @@ func generateCrCsv(logger *logrus.Logger, uuid string, arv *arvadosclient.Arvado
 	var totalCost float64
 
 	// This is a container request, find the container
-	cr, err := loadObject(logger, arv, resultsDir+"/"+uuid, uuid, cache)
+	var cr arvados.ContainerRequest
+	err = loadObject(logger, arv, resultsDir+"/"+uuid, uuid, cache, &cr)
 	if err != nil {
-		return nil, fmt.Errorf("Error loading object %s: %s", uuid, err)
+		return nil, fmt.Errorf("error loading cr object %s: %s", uuid, err)
 	}
-	container, err := loadObject(logger, arv, resultsDir+"/"+uuid, cr["container_uuid"].(string), cache)
+	fmt.Printf("cr: %+v\n", cr)
+	var container arvados.Container
+	err = loadObject(logger, arv, resultsDir+"/"+uuid, cr.ContainerUUID, cache, &container)
 	if err != nil {
-		return nil, fmt.Errorf("Error loading object %s: %s", cr["container_uuid"].(string), err)
+		return nil, fmt.Errorf("error loading container object %s: %s", cr.ContainerUUID, err)
 	}
 
 	topNode, err := getNode(arv, ac, kc, cr)
 	if err != nil {
-		return nil, fmt.Errorf("Error getting node %s: %s\n", cr["uuid"], err)
+		return nil, fmt.Errorf("error getting node %s: %s", cr.UUID, err)
 	}
 	tmpCsv, totalCost = addContainerLine(logger, topNode, cr, container)
 	csv += tmpCsv
 	totalCost += tmpTotalCost
-	cost[container["uuid"].(string)] = totalCost
+	cost[container.UUID] = totalCost
 
 	// Now find all container requests that have the container we found above as requesting_container_uuid
-	var childCrs map[string]interface{}
+	var childCrs arvados.ContainerRequestList
 	filterset := []arvados.Filter{
 		{
 			Attr:     "requesting_container_uuid",
 			Operator: "=",
-			Operand:  container["uuid"].(string),
+			Operand:  container.UUID,
 		}}
-	err = ac.RequestAndDecodeContext(context.Background(), &childCrs, "GET", "arvados/v1/container_requests", nil, map[string]interface{}{
+	err = ac.RequestAndDecode(&childCrs, "GET", "arvados/v1/container_requests", nil, map[string]interface{}{
 		"filters": filterset,
 		"limit":   10000,
 	})
 	if err != nil {
 		return nil, fmt.Errorf("error querying container_requests: %s", err.Error())
 	}
-	if value, ok := childCrs["items"]; ok {
-		logger.Infof("Collecting child containers for container request %s", uuid)
-		items := value.([]interface{})
-		for _, item := range items {
-			logger.Info(".")
-			itemMap := item.(map[string]interface{})
-			node, err := getNode(arv, ac, kc, itemMap)
-			if err != nil {
-				return nil, fmt.Errorf("Error getting node %s: %s\n", itemMap["uuid"], err)
-			}
-			logger.Debug("\nChild container: " + itemMap["container_uuid"].(string) + "\n")
-			c2, err := loadObject(logger, arv, resultsDir+"/"+uuid, itemMap["container_uuid"].(string), cache)
-			if err != nil {
-				return nil, fmt.Errorf("Error loading object %s: %s", cr["container_uuid"].(string), err)
-			}
-			tmpCsv, tmpTotalCost = addContainerLine(logger, node, itemMap, c2)
-			cost[itemMap["container_uuid"].(string)] = tmpTotalCost
-			csv += tmpCsv
-			totalCost += tmpTotalCost
+	logger.Infof("Collecting child containers for container request %s", uuid)
+	for _, cr2 := range childCrs.Items {
+		logger.Info(".")
+		node, err := getNode(arv, ac, kc, cr2)
+		if err != nil {
+			return nil, fmt.Errorf("error getting node %s: %s", cr2.UUID, err)
+		}
+		logger.Debug("\nChild container: " + cr2.ContainerUUID + "\n")
+		var c2 arvados.Container
+		err = loadObject(logger, arv, resultsDir+"/"+uuid, cr2.ContainerUUID, cache, &c2)
+		if err != nil {
+			return nil, fmt.Errorf("error loading object %s: %s", cr2.ContainerUUID, err)
 		}
+		tmpCsv, tmpTotalCost = addContainerLine(logger, node, cr2, c2)
+		cost[cr2.ContainerUUID] = tmpTotalCost
+		csv += tmpCsv
+		totalCost += tmpTotalCost
 	}
 	logger.Info(" done\n")
 
@@ -481,20 +407,19 @@ func generateCrCsv(logger *logrus.Logger, uuid string, arv *arvadosclient.Arvado
 	fName := resultsDir + "/" + uuid + ".csv"
 	err = ioutil.WriteFile(fName, []byte(csv), 0644)
 	if err != nil {
-		return nil, fmt.Errorf("Error writing file with path %s: %s\n", fName, err.Error())
+		return nil, fmt.Errorf("error writing file with path %s: %s", fName, err.Error())
 	}
 
 	return
 }
 
-func costanalyzer(prog string, args []string, loader *config.Loader, logger *logrus.Logger, stdout, stderr io.Writer) (exitcode int) {
-	exitcode, uuids, resultsDir, cache := parseFlags(prog, args, loader, logger, stderr)
+func costanalyzer(prog string, args []string, loader *config.Loader, logger *logrus.Logger, stdout, stderr io.Writer) (exitcode int, err error) {
+	exitcode, uuids, resultsDir, cache, err := parseFlags(prog, args, loader, logger, stderr)
 	if exitcode != 0 {
 		return
 	}
-	err := ensureDirectory(logger, resultsDir)
+	err = ensureDirectory(logger, resultsDir)
 	if err != nil {
-		logger.Errorf("%s", err)
 		exitcode = 3
 		return
 	}
@@ -502,13 +427,13 @@ func costanalyzer(prog string, args []string, loader *config.Loader, logger *log
 	// Arvados Client setup
 	arv, err := arvadosclient.MakeArvadosClient()
 	if err != nil {
-		logger.Errorf("error creating Arvados object: %s", err)
+		err = fmt.Errorf("error creating Arvados object: %s", err)
 		exitcode = 1
 		return
 	}
 	kc, err := keepclient.MakeKeepClient(arv)
 	if err != nil {
-		logger.Errorf("error creating Keep object: %s", err)
+		err = fmt.Errorf("error creating Keep object: %s", err)
 		exitcode = 1
 		return
 	}
@@ -521,8 +446,6 @@ func costanalyzer(prog string, args []string, loader *config.Loader, logger *log
 			// This is a project (group)
 			cost, err = handleProject(logger, uuid, arv, ac, kc, resultsDir, cache)
 			if err != nil {
-				// FIXME print error
-				logger.Info(err.Error())
 				exitcode = 1
 				return
 			}
@@ -531,9 +454,12 @@ func costanalyzer(prog string, args []string, loader *config.Loader, logger *log
 			}
 		} else if strings.Contains(uuid, "-xvhdp-") {
 			// This is a container request
-			crCsv, err := generateCrCsv(logger, uuid, arv, ac, kc, resultsDir, cache)
+			var crCsv map[string]float64
+			crCsv, err = generateCrCsv(logger, uuid, arv, ac, kc, resultsDir, cache)
 			if err != nil {
-				logger.Fatalf("Error generating container_request CSV: %s\n", err.Error())
+				err = fmt.Errorf("Error generating container_request CSV for uuid %s: %s", uuid, err.Error())
+				exitcode = 2
+				return
 			}
 			for k, v := range crCsv {
 				cost[k] = v
@@ -541,7 +467,8 @@ func costanalyzer(prog string, args []string, loader *config.Loader, logger *log
 		} else if strings.Contains(uuid, "-tpzed-") {
 			// This is a user. The "Home" project for a user is not a real project.
 			// It is identified by the user uuid. As such, cost analysis for the
-			// "Home" project is not supported by this program.
+			// "Home" project is not supported by this program. Skip this uuid, but
+			// keep going.
 			logger.Errorf("Cost analysis is not supported for the 'Home' project: %s", uuid)
 		}
 	}
@@ -575,11 +502,10 @@ func costanalyzer(prog string, args []string, loader *config.Loader, logger *log
 	aFile := resultsDir + "/" + time.Now().Format("2006-01-02-15-04-05") + "-aggregate-costaccounting.csv"
 	err = ioutil.WriteFile(aFile, []byte(csv), 0644)
 	if err != nil {
-		logger.Errorf("Error writing file with path %s: %s\n", aFile, err.Error())
+		err = fmt.Errorf("Error writing file with path %s: %s", aFile, err.Error())
 		exitcode = 1
 		return
-	} else {
-		logger.Infof("\nAggregate cost accounting for all supplied uuids in %s\n", aFile)
 	}
+	logger.Infof("\nAggregate cost accounting for all supplied uuids in %s\n", aFile)
 	return
 }
diff --git a/lib/costanalyzer/costanalyzer_test.go b/lib/costanalyzer/costanalyzer_test.go
index 0a44be8d8..e2fb9e99b 100644
--- a/lib/costanalyzer/costanalyzer_test.go
+++ b/lib/costanalyzer/costanalyzer_test.go
@@ -6,7 +6,6 @@ package costanalyzer
 
 import (
 	"bytes"
-	"context"
 	"io"
 	"io/ioutil"
 	"os"
@@ -92,6 +91,18 @@ func (s *Suite) SetUpSuite(c *check.C) {
     "Preemptible": false
 }`
 
+	legacyD1V2JSON := `{
+    "properties": {
+        "cloud_node": {
+            "price": 0.073001,
+            "size": "Standard_D1_v2"
+        },
+        "total_cpu_cores": 1,
+        "total_ram_mb": 3418,
+        "total_scratch_mb": 51170
+    }
+}`
+
 	// Our fixtures do not actually contain file contents. Populate the log collections we're going to use with the node.json file
 	createNodeJSON(c, arv, ac, kc, arvadostest.CompletedContainerRequestUUID, arvadostest.LogCollectionUUID, standardE4sV3JSON)
 	createNodeJSON(c, arv, ac, kc, arvadostest.CompletedContainerRequestUUID2, arvadostest.LogCollectionUUID2, standardD32sV3JSON)
@@ -100,19 +111,19 @@ func (s *Suite) SetUpSuite(c *check.C) {
 	createNodeJSON(c, arv, ac, kc, arvadostest.CompletedDiagnosticsContainerRequest2UUID, arvadostest.DiagnosticsContainerRequest2LogCollectionUUID, standardA1V2JSON)
 	createNodeJSON(c, arv, ac, kc, arvadostest.CompletedDiagnosticsHasher1ContainerRequestUUID, arvadostest.Hasher1LogCollectionUUID, standardA1V2JSON)
 	createNodeJSON(c, arv, ac, kc, arvadostest.CompletedDiagnosticsHasher2ContainerRequestUUID, arvadostest.Hasher2LogCollectionUUID, standardA2V2JSON)
-	createNodeJSON(c, arv, ac, kc, arvadostest.CompletedDiagnosticsHasher3ContainerRequestUUID, arvadostest.Hasher3LogCollectionUUID, standardA1V2JSON)
+	createNodeJSON(c, arv, ac, kc, arvadostest.CompletedDiagnosticsHasher3ContainerRequestUUID, arvadostest.Hasher3LogCollectionUUID, legacyD1V2JSON)
 }
 
 func createNodeJSON(c *check.C, arv *arvadosclient.ArvadosClient, ac *arvados.Client, kc *keepclient.KeepClient, crUUID string, logUUID string, nodeJSON string) {
 	// Get the CR
 	var cr arvados.ContainerRequest
-	err := ac.RequestAndDecodeContext(context.Background(), &cr, "GET", "arvados/v1/container_requests/"+crUUID, nil, nil)
+	err := ac.RequestAndDecode(&cr, "GET", "arvados/v1/container_requests/"+crUUID, nil, nil)
 	c.Assert(err, check.Equals, nil)
 	c.Assert(cr.LogUUID, check.Equals, logUUID)
 
 	// Get the log collection
 	var coll arvados.Collection
-	err = ac.RequestAndDecodeContext(context.Background(), &coll, "GET", "arvados/v1/collections/"+cr.LogUUID, nil, nil)
+	err = ac.RequestAndDecode(&coll, "GET", "arvados/v1/collections/"+cr.LogUUID, nil, nil)
 	c.Assert(err, check.IsNil)
 
 	// Create a node.json file -- the fixture doesn't actually contain the contents of the collection.
@@ -131,7 +142,7 @@ func createNodeJSON(c *check.C, arv *arvadosclient.ArvadosClient, ac *arvados.Cl
 	c.Assert(mtxt, check.NotNil)
 
 	// Update collection record
-	err = ac.RequestAndDecodeContext(context.Background(), &coll, "PUT", "arvados/v1/collections/"+cr.LogUUID, nil, map[string]interface{}{
+	err = ac.RequestAndDecode(&coll, "PUT", "arvados/v1/collections/"+cr.LogUUID, nil, map[string]interface{}{
 		"collection": map[string]interface{}{
 			"manifest_text": mtxt,
 		},
@@ -195,13 +206,13 @@ func (*Suite) TestDoubleContainerRequestUUID(c *check.C) {
 	// the analysis with the project uuid. The results should be identical.
 	ac := arvados.NewClientFromEnv()
 	var cr arvados.ContainerRequest
-	err = ac.RequestAndDecodeContext(context.Background(), &cr, "PUT", "arvados/v1/container_requests/"+arvadostest.CompletedContainerRequestUUID, nil, map[string]interface{}{
+	err = ac.RequestAndDecode(&cr, "PUT", "arvados/v1/container_requests/"+arvadostest.CompletedContainerRequestUUID, nil, map[string]interface{}{
 		"container_request": map[string]interface{}{
 			"owner_uuid": arvadostest.AProjectUUID,
 		},
 	})
 	c.Assert(err, check.IsNil)
-	err = ac.RequestAndDecodeContext(context.Background(), &cr, "PUT", "arvados/v1/container_requests/"+arvadostest.CompletedContainerRequestUUID2, nil, map[string]interface{}{
+	err = ac.RequestAndDecode(&cr, "PUT", "arvados/v1/container_requests/"+arvadostest.CompletedContainerRequestUUID2, nil, map[string]interface{}{
 		"container_request": map[string]interface{}{
 			"owner_uuid": arvadostest.AProjectUUID,
 		},
@@ -234,16 +245,18 @@ func (*Suite) TestMultipleContainerRequestUUIDWithReuse(c *check.C) {
 	var stdout, stderr bytes.Buffer
 	// Run costanalyzer with 2 container request uuids
 	exitcode := Command.RunCommand("costanalyzer.test", []string{"-uuid", arvadostest.CompletedDiagnosticsContainerRequest1UUID, "-uuid", arvadostest.CompletedDiagnosticsContainerRequest2UUID}, &bytes.Buffer{}, &stdout, &stderr)
+	c.Logf("%s", stderr.Bytes())
+	c.Logf("%s", stdout.Bytes())
 	c.Check(exitcode, check.Equals, 0)
 	c.Assert(stdout.String(), check.Matches, "(?ms).*supplied uuids in .*")
 
 	uuidReport, err := ioutil.ReadFile("results/" + arvadostest.CompletedDiagnosticsContainerRequest1UUID + ".csv")
 	c.Assert(err, check.IsNil)
-	c.Check(string(uuidReport), check.Matches, "(?ms).*TOTAL,,,,,,,,,0.00914539")
+	c.Check(string(uuidReport), check.Matches, "(?ms).*TOTAL,,,,,,,,,0.00916192")
 
 	uuidReport2, err := ioutil.ReadFile("results/" + arvadostest.CompletedDiagnosticsContainerRequest2UUID + ".csv")
 	c.Assert(err, check.IsNil)
-	c.Check(string(uuidReport2), check.Matches, "(?ms).*TOTAL,,,,,,,,,0.00586435")
+	c.Check(string(uuidReport2), check.Matches, "(?ms).*TOTAL,,,,,,,,,0.00588088")
 
 	re := regexp.MustCompile(`(?ms).*supplied uuids in (.*?)\n`)
 	matches := re.FindStringSubmatch(stdout.String()) // matches[1] contains a string like 'results/2020-11-02-18-57-45-aggregate-costaccounting.csv'
@@ -251,5 +264,5 @@ func (*Suite) TestMultipleContainerRequestUUIDWithReuse(c *check.C) {
 	aggregateCostReport, err := ioutil.ReadFile(matches[1])
 	c.Assert(err, check.IsNil)
 
-	c.Check(string(aggregateCostReport), check.Matches, "(?ms).*TOTAL,0.01490377")
+	c.Check(string(aggregateCostReport), check.Matches, "(?ms).*TOTAL,0.01492030")
 }

commit b215c29447ea3b7c0c066cc4262308f26f6629fa
Author: Ward Vandewege <ward at curii.com>
Date:   Wed Nov 11 22:07:48 2020 -0500

    16950: changes after review.
    
    Arvados-DCO-1.1-Signed-off-by: Ward Vandewege <ward at curii.com>

diff --git a/lib/costanalyzer/command.go b/lib/costanalyzer/cmd.go
similarity index 97%
rename from lib/costanalyzer/command.go
rename to lib/costanalyzer/cmd.go
index 0760b4fd2..6829be7d3 100644
--- a/lib/costanalyzer/command.go
+++ b/lib/costanalyzer/cmd.go
@@ -33,6 +33,7 @@ func (command) RunCommand(prog string, args []string, stdin io.Reader, stdout, s
 	}()
 
 	logger.SetFormatter(new(NoPrefixFormatter))
+	logger.SetOutput(stdout)
 
 	loader := config.NewLoader(stdin, logger)
 	loader.SkipLegacy = true
diff --git a/lib/costanalyzer/costanalyzer.go b/lib/costanalyzer/costanalyzer.go
index c86e26769..ccd9520bc 100644
--- a/lib/costanalyzer/costanalyzer.go
+++ b/lib/costanalyzer/costanalyzer.go
@@ -6,6 +6,7 @@ package costanalyzer
 
 import (
 	"bytes"
+	"context"
 	"encoding/json"
 	"errors"
 	"flag"
@@ -16,7 +17,6 @@ import (
 	"git.arvados.org/arvados.git/sdk/go/keepclient"
 	"io"
 	"io/ioutil"
-	"log"
 	"net/http"
 	"os"
 	"strconv"
@@ -26,9 +26,6 @@ import (
 	"github.com/sirupsen/logrus"
 )
 
-// Dict is a helper type so we don't have to write out 'map[string]interface{}' every time.
-type Dict map[string]interface{}
-
 // LegacyNodeInfo is a struct for records created by Arvados Node Manager (Arvados <= 1.4.3)
 // Example:
 // {
@@ -88,7 +85,7 @@ func (i *arrayFlags) Set(value string) error {
 	return nil
 }
 
-func parseFlags(prog string, args []string, loader *config.Loader, logger *logrus.Logger, stderr io.Writer) (exitCode int, uuids arrayFlags, resultsDir string) {
+func parseFlags(prog string, args []string, loader *config.Loader, logger *logrus.Logger, stderr io.Writer) (exitCode int, uuids arrayFlags, resultsDir string, cache bool) {
 	flags := flag.NewFlagSet("", flag.ContinueOnError)
 	flags.SetOutput(stderr)
 	flags.Usage = func() {
@@ -133,6 +130,7 @@ Options:
 	loglevel := flags.String("log-level", "info", "logging level (debug, info, ...)")
 	resultsDir = *flags.String("output", "results", "output directory for the CSV reports")
 	flags.Var(&uuids, "uuid", "Toplevel project or container request uuid. May be specified more than once.")
+	flags.BoolVar(&cache, "cache", true, "create and use a local disk cache of Arvados objects")
 	err := flags.Parse(args)
 	if err == flag.ErrHelp {
 		exitCode = 1
@@ -163,19 +161,17 @@ func ensureDirectory(logger *logrus.Logger, dir string) (err error) {
 	if os.IsNotExist(err) {
 		err = os.MkdirAll(dir, 0700)
 		if err != nil {
-			logger.Errorf("Error creating directory %s: %s\n", dir, err.Error())
-			return
+			return fmt.Errorf("Error creating directory %s: %s\n", dir, err.Error())
 		}
 	} else {
 		if !statData.IsDir() {
-			logger.Errorf("The path %s is not a directory\n", dir)
-			return
+			return fmt.Errorf("The path %s is not a directory\n", dir)
 		}
 	}
 	return
 }
 
-func addContainerLine(logger *logrus.Logger, node interface{}, cr Dict, container Dict) (csv string, cost float64) {
+func addContainerLine(logger *logrus.Logger, node interface{}, cr, container map[string]interface{}) (csv string, cost float64) {
 	csv = cr["uuid"].(string) + ","
 	csv += cr["name"].(string) + ","
 	csv += container["uuid"].(string) + ","
@@ -219,7 +215,7 @@ func addContainerLine(logger *logrus.Logger, node interface{}, cr Dict, containe
 	return
 }
 
-func loadCachedObject(logger *logrus.Logger, file string, uuid string) (reload bool, object Dict) {
+func loadCachedObject(logger *logrus.Logger, file string, uuid string) (reload bool, object map[string]interface{}) {
 	reload = true
 	// See if we have a cached copy of this object
 	if _, err := os.Stat(file); err == nil {
@@ -248,7 +244,7 @@ func loadCachedObject(logger *logrus.Logger, file string, uuid string) (reload b
 }
 
 // Load an Arvados object.
-func loadObject(logger *logrus.Logger, arv *arvadosclient.ArvadosClient, path string, uuid string) (object Dict, err error) {
+func loadObject(logger *logrus.Logger, arv *arvadosclient.ArvadosClient, path string, uuid string, cache bool) (object map[string]interface{}, err error) {
 	err = ensureDirectory(logger, path)
 	if err != nil {
 		return
@@ -257,112 +253,118 @@ func loadObject(logger *logrus.Logger, arv *arvadosclient.ArvadosClient, path st
 	file := path + "/" + uuid + ".json"
 
 	var reload bool
-	reload, object = loadCachedObject(logger, file, uuid)
+	if !cache {
+		reload = true
+	} else {
+		reload, object = loadCachedObject(logger, file, uuid)
+	}
+	if !reload {
+		return
+	}
 
-	if reload {
-		var err error
-		if strings.Contains(uuid, "-j7d0g-") {
-			err = arv.Get("groups", uuid, nil, &object)
-		} else if strings.Contains(uuid, "-xvhdp-") {
-			err = arv.Get("container_requests", uuid, nil, &object)
-		} else if strings.Contains(uuid, "-dz642-") {
-			err = arv.Get("containers", uuid, nil, &object)
-		} else {
-			err = arv.Get("jobs", uuid, nil, &object)
-		}
-		if err != nil {
-			logger.Fatalf("Error loading object with UUID %q:\n  %s\n", uuid, err)
-		}
-		encoded, err := json.MarshalIndent(object, "", " ")
-		if err != nil {
-			logger.Fatalf("Error marshaling object with UUID %q:\n  %s\n", uuid, err)
-		}
-		err = ioutil.WriteFile(file, encoded, 0644)
-		if err != nil {
-			logger.Fatalf("Error writing file %s:\n  %s\n", file, err)
-		}
+	if strings.Contains(uuid, "-j7d0g-") {
+		err = arv.Get("groups", uuid, nil, &object)
+	} else if strings.Contains(uuid, "-xvhdp-") {
+		err = arv.Get("container_requests", uuid, nil, &object)
+	} else if strings.Contains(uuid, "-dz642-") {
+		err = arv.Get("containers", uuid, nil, &object)
+	} else {
+		err = arv.Get("jobs", uuid, nil, &object)
+	}
+	if err != nil {
+		err = fmt.Errorf("Error loading object with UUID %q:\n  %s\n", uuid, err)
+		return
+	}
+	encoded, err := json.MarshalIndent(object, "", " ")
+	if err != nil {
+		err = fmt.Errorf("Error marshaling object with UUID %q:\n  %s\n", uuid, err)
+		return
+	}
+	err = ioutil.WriteFile(file, encoded, 0644)
+	if err != nil {
+		err = fmt.Errorf("Error writing file %s:\n  %s\n", file, err)
+		return
 	}
 	return
 }
 
-func getNode(arv *arvadosclient.ArvadosClient, arv2 *arvados.Client, kc *keepclient.KeepClient, itemMap Dict) (node interface{}, err error) {
-	if _, ok := itemMap["log_uuid"]; ok {
-		if itemMap["log_uuid"] == nil {
-			err = errors.New("No log collection")
-			return
-		}
+func getNode(arv *arvadosclient.ArvadosClient, ac *arvados.Client, kc *keepclient.KeepClient, itemMap map[string]interface{}) (node interface{}, err error) {
+	logUuid, ok := itemMap["log_uuid"]
+	if !ok {
+		err = errors.New("No log collection")
+		return
+	}
 
-		var collection arvados.Collection
-		err = arv.Get("collections", itemMap["log_uuid"].(string), nil, &collection)
-		if err != nil {
-			err = fmt.Errorf("Error getting collection: %s", err)
-			return
-		}
+	var collection arvados.Collection
+	err = arv.Get("collections", logUuid.(string), nil, &collection)
+	if err != nil {
+		err = fmt.Errorf("Error getting collection: %s", err)
+		return
+	}
 
-		var fs arvados.CollectionFileSystem
-		fs, err = collection.FileSystem(arv2, kc)
-		if err != nil {
-			err = fmt.Errorf("Error opening collection as filesystem: %s", err)
-			return
-		}
-		var f http.File
-		f, err = fs.Open("node.json")
+	var fs arvados.CollectionFileSystem
+	fs, err = collection.FileSystem(ac, kc)
+	if err != nil {
+		err = fmt.Errorf("Error opening collection as filesystem: %s", err)
+		return
+	}
+	var f http.File
+	f, err = fs.Open("node.json")
+	if err != nil {
+		err = fmt.Errorf("Error opening file 'node.json' in collection %s: %s", logUuid.(string), err)
+		return
+	}
+
+	var nodeDict map[string]interface{}
+	buf := new(bytes.Buffer)
+	_, err = buf.ReadFrom(f)
+	if err != nil {
+		err = fmt.Errorf("Error reading file 'node.json' in collection %s: %s", logUuid.(string), err)
+		return
+	}
+	contents := buf.String()
+	f.Close()
+
+	err = json.Unmarshal([]byte(contents), &nodeDict)
+	if err != nil {
+		err = fmt.Errorf("Error unmarshalling: %s", err)
+		return
+	}
+	if val, ok := nodeDict["properties"]; ok {
+		var encoded []byte
+		encoded, err = json.MarshalIndent(val, "", " ")
 		if err != nil {
-			err = fmt.Errorf("Error opening file 'node.json' in collection %s: %s", itemMap["log_uuid"].(string), err)
+			err = fmt.Errorf("Error marshalling: %s", err)
 			return
 		}
-
-		var nodeDict Dict
-		buf := new(bytes.Buffer)
-		_, err = buf.ReadFrom(f)
+		// node is type LegacyNodeInfo
+		var newNode LegacyNodeInfo
+		err = json.Unmarshal(encoded, &newNode)
 		if err != nil {
-			err = fmt.Errorf("Error reading file 'node.json' in collection %s: %s", itemMap["log_uuid"].(string), err)
+			err = fmt.Errorf("Error unmarshalling: %s", err)
 			return
 		}
-		contents := buf.String()
-		f.Close()
-
-		err = json.Unmarshal([]byte(contents), &nodeDict)
+		node = newNode
+	} else {
+		// node is type Node
+		var newNode Node
+		err = json.Unmarshal([]byte(contents), &newNode)
 		if err != nil {
 			err = fmt.Errorf("Error unmarshalling: %s", err)
 			return
 		}
-		if val, ok := nodeDict["properties"]; ok {
-			var encoded []byte
-			encoded, err = json.MarshalIndent(val, "", " ")
-			if err != nil {
-				err = fmt.Errorf("Error marshalling: %s", err)
-				return
-			}
-			// node is type LegacyNodeInfo
-			var newNode LegacyNodeInfo
-			err = json.Unmarshal(encoded, &newNode)
-			if err != nil {
-				err = fmt.Errorf("Error unmarshalling: %s", err)
-				return
-			}
-			node = newNode
-		} else {
-			// node is type Node
-			var newNode Node
-			err = json.Unmarshal([]byte(contents), &newNode)
-			if err != nil {
-				err = fmt.Errorf("Error unmarshalling: %s", err)
-				return
-			}
-			node = newNode
-		}
+		node = newNode
 	}
 	return
 }
 
-func handleProject(logger *logrus.Logger, uuid string, arv *arvadosclient.ArvadosClient, arv2 *arvados.Client, kc *keepclient.KeepClient, resultsDir string) (cost map[string]float64) {
+func handleProject(logger *logrus.Logger, uuid string, arv *arvadosclient.ArvadosClient, ac *arvados.Client, kc *keepclient.KeepClient, resultsDir string, cache bool) (cost map[string]float64, err error) {
 
 	cost = make(map[string]float64)
 
-	project, err := loadObject(logger, arv, resultsDir+"/"+uuid, uuid)
+	project, err := loadObject(logger, arv, resultsDir+"/"+uuid, uuid, cache)
 	if err != nil {
-		logger.Fatalf("Error loading object %s: %s\n", uuid, err.Error())
+		return nil, fmt.Errorf("Error loading object %s: %s\n", uuid, err.Error())
 	}
 
 	// arv -f uuid container_request list --filters '[["owner_uuid","=","<someuuid>"],["requesting_container_uuid","=",null]]'
@@ -381,16 +383,23 @@ func handleProject(logger *logrus.Logger, uuid string, arv *arvadosclient.Arvado
 			Operand:  nil,
 		},
 	}
-	err = arv.List("container_requests", arvadosclient.Dict{"filters": filterset, "limit": 10000}, &childCrs)
+	err = ac.RequestAndDecodeContext(context.Background(), &childCrs, "GET", "arvados/v1/container_requests", nil, map[string]interface{}{
+		"filters": filterset,
+		"limit":   10000,
+	})
 	if err != nil {
-		logger.Fatalf("Error querying container_requests: %s\n", err.Error())
+		return nil, fmt.Errorf("Error querying container_requests: %s\n", err.Error())
 	}
 	if value, ok := childCrs["items"]; ok {
 		logger.Infof("Collecting top level container requests in project %s\n", uuid)
 		items := value.([]interface{})
 		for _, item := range items {
 			itemMap := item.(map[string]interface{})
-			for k, v := range generateCrCsv(logger, itemMap["uuid"].(string), arv, arv2, kc, resultsDir) {
+			crCsv, err := generateCrCsv(logger, itemMap["uuid"].(string), arv, ac, kc, resultsDir, cache)
+			if err != nil {
+				return nil, fmt.Errorf("Error generating container_request CSV: %s\n", err.Error())
+			}
+			for k, v := range crCsv {
 				cost[k] = v
 			}
 		}
@@ -400,7 +409,7 @@ func handleProject(logger *logrus.Logger, uuid string, arv *arvadosclient.Arvado
 	return
 }
 
-func generateCrCsv(logger *logrus.Logger, uuid string, arv *arvadosclient.ArvadosClient, arv2 *arvados.Client, kc *keepclient.KeepClient, resultsDir string) (cost map[string]float64) {
+func generateCrCsv(logger *logrus.Logger, uuid string, arv *arvadosclient.ArvadosClient, ac *arvados.Client, kc *keepclient.KeepClient, resultsDir string, cache bool) (cost map[string]float64, err error) {
 
 	cost = make(map[string]float64)
 
@@ -410,23 +419,22 @@ func generateCrCsv(logger *logrus.Logger, uuid string, arv *arvadosclient.Arvado
 	var totalCost float64
 
 	// This is a container request, find the container
-	cr, err := loadObject(logger, arv, resultsDir+"/"+uuid, uuid)
+	cr, err := loadObject(logger, arv, resultsDir+"/"+uuid, uuid, cache)
 	if err != nil {
-		log.Fatalf("Error loading object %s: %s", uuid, err)
+		return nil, fmt.Errorf("Error loading object %s: %s", uuid, err)
 	}
-	container, err := loadObject(logger, arv, resultsDir+"/"+uuid, cr["container_uuid"].(string))
+	container, err := loadObject(logger, arv, resultsDir+"/"+uuid, cr["container_uuid"].(string), cache)
 	if err != nil {
-		log.Fatalf("Error loading object %s: %s", cr["container_uuid"].(string), err)
+		return nil, fmt.Errorf("Error loading object %s: %s", cr["container_uuid"].(string), err)
 	}
 
-	topNode, err := getNode(arv, arv2, kc, cr)
+	topNode, err := getNode(arv, ac, kc, cr)
 	if err != nil {
-		log.Fatalf("Error getting node %s: %s\n", cr["uuid"], err)
+		return nil, fmt.Errorf("Error getting node %s: %s\n", cr["uuid"], err)
 	}
 	tmpCsv, totalCost = addContainerLine(logger, topNode, cr, container)
 	csv += tmpCsv
 	totalCost += tmpTotalCost
-
 	cost[container["uuid"].(string)] = totalCost
 
 	// Now find all container requests that have the container we found above as requesting_container_uuid
@@ -437,9 +445,12 @@ func generateCrCsv(logger *logrus.Logger, uuid string, arv *arvadosclient.Arvado
 			Operator: "=",
 			Operand:  container["uuid"].(string),
 		}}
-	err = arv.List("container_requests", arvadosclient.Dict{"filters": filterset, "limit": 10000}, &childCrs)
+	err = ac.RequestAndDecodeContext(context.Background(), &childCrs, "GET", "arvados/v1/container_requests", nil, map[string]interface{}{
+		"filters": filterset,
+		"limit":   10000,
+	})
 	if err != nil {
-		log.Fatal("error querying container_requests", err.Error())
+		return nil, fmt.Errorf("error querying container_requests: %s", err.Error())
 	}
 	if value, ok := childCrs["items"]; ok {
 		logger.Infof("Collecting child containers for container request %s", uuid)
@@ -447,14 +458,14 @@ func generateCrCsv(logger *logrus.Logger, uuid string, arv *arvadosclient.Arvado
 		for _, item := range items {
 			logger.Info(".")
 			itemMap := item.(map[string]interface{})
-			node, err := getNode(arv, arv2, kc, itemMap)
+			node, err := getNode(arv, ac, kc, itemMap)
 			if err != nil {
-				log.Fatalf("Error getting node %s: %s\n", itemMap["uuid"], err)
+				return nil, fmt.Errorf("Error getting node %s: %s\n", itemMap["uuid"], err)
 			}
 			logger.Debug("\nChild container: " + itemMap["container_uuid"].(string) + "\n")
-			c2, err := loadObject(logger, arv, resultsDir+"/"+uuid, itemMap["container_uuid"].(string))
+			c2, err := loadObject(logger, arv, resultsDir+"/"+uuid, itemMap["container_uuid"].(string), cache)
 			if err != nil {
-				log.Fatalf("Error loading object %s: %s", cr["container_uuid"].(string), err)
+				return nil, fmt.Errorf("Error loading object %s: %s", cr["container_uuid"].(string), err)
 			}
 			tmpCsv, tmpTotalCost = addContainerLine(logger, node, itemMap, c2)
 			cost[itemMap["container_uuid"].(string)] = tmpTotalCost
@@ -470,20 +481,20 @@ func generateCrCsv(logger *logrus.Logger, uuid string, arv *arvadosclient.Arvado
 	fName := resultsDir + "/" + uuid + ".csv"
 	err = ioutil.WriteFile(fName, []byte(csv), 0644)
 	if err != nil {
-		logger.Errorf("Error writing file with path %s: %s\n", fName, err.Error())
-		os.Exit(1)
+		return nil, fmt.Errorf("Error writing file with path %s: %s\n", fName, err.Error())
 	}
 
 	return
 }
 
 func costanalyzer(prog string, args []string, loader *config.Loader, logger *logrus.Logger, stdout, stderr io.Writer) (exitcode int) {
-	exitcode, uuids, resultsDir := parseFlags(prog, args, loader, logger, stderr)
+	exitcode, uuids, resultsDir, cache := parseFlags(prog, args, loader, logger, stderr)
 	if exitcode != 0 {
 		return
 	}
 	err := ensureDirectory(logger, resultsDir)
 	if err != nil {
+		logger.Errorf("%s", err)
 		exitcode = 3
 		return
 	}
@@ -492,26 +503,39 @@ func costanalyzer(prog string, args []string, loader *config.Loader, logger *log
 	arv, err := arvadosclient.MakeArvadosClient()
 	if err != nil {
 		logger.Errorf("error creating Arvados object: %s", err)
-		os.Exit(1)
+		exitcode = 1
+		return
 	}
 	kc, err := keepclient.MakeKeepClient(arv)
 	if err != nil {
 		logger.Errorf("error creating Keep object: %s", err)
-		os.Exit(1)
+		exitcode = 1
+		return
 	}
 
-	arv2 := arvados.NewClientFromEnv()
+	ac := arvados.NewClientFromEnv()
 
 	cost := make(map[string]float64)
 	for _, uuid := range uuids {
 		if strings.Contains(uuid, "-j7d0g-") {
 			// This is a project (group)
-			for k, v := range handleProject(logger, uuid, arv, arv2, kc, resultsDir) {
+			cost, err = handleProject(logger, uuid, arv, ac, kc, resultsDir, cache)
+			if err != nil {
+				// FIXME print error
+				logger.Info(err.Error())
+				exitcode = 1
+				return
+			}
+			for k, v := range cost {
 				cost[k] = v
 			}
 		} else if strings.Contains(uuid, "-xvhdp-") {
 			// This is a container request
-			for k, v := range generateCrCsv(logger, uuid, arv, arv2, kc, resultsDir) {
+			crCsv, err := generateCrCsv(logger, uuid, arv, ac, kc, resultsDir, cache)
+			if err != nil {
+				logger.Fatalf("Error generating container_request CSV: %s\n", err.Error())
+			}
+			for k, v := range crCsv {
 				cost[k] = v
 			}
 		} else if strings.Contains(uuid, "-tpzed-") {
@@ -552,7 +576,8 @@ func costanalyzer(prog string, args []string, loader *config.Loader, logger *log
 	err = ioutil.WriteFile(aFile, []byte(csv), 0644)
 	if err != nil {
 		logger.Errorf("Error writing file with path %s: %s\n", aFile, err.Error())
-		os.Exit(1)
+		exitcode = 1
+		return
 	} else {
 		logger.Infof("\nAggregate cost accounting for all supplied uuids in %s\n", aFile)
 	}
diff --git a/lib/costanalyzer/costanalyzer_test.go b/lib/costanalyzer/costanalyzer_test.go
index 2ef8733b0..0a44be8d8 100644
--- a/lib/costanalyzer/costanalyzer_test.go
+++ b/lib/costanalyzer/costanalyzer_test.go
@@ -6,6 +6,7 @@ package costanalyzer
 
 import (
 	"bytes"
+	"context"
 	"io"
 	"io/ioutil"
 	"os"
@@ -105,13 +106,13 @@ func (s *Suite) SetUpSuite(c *check.C) {
 func createNodeJSON(c *check.C, arv *arvadosclient.ArvadosClient, ac *arvados.Client, kc *keepclient.KeepClient, crUUID string, logUUID string, nodeJSON string) {
 	// Get the CR
 	var cr arvados.ContainerRequest
-	err := arv.Get("container_requests", crUUID, arvadosclient.Dict{}, &cr)
+	err := ac.RequestAndDecodeContext(context.Background(), &cr, "GET", "arvados/v1/container_requests/"+crUUID, nil, nil)
 	c.Assert(err, check.Equals, nil)
 	c.Assert(cr.LogUUID, check.Equals, logUUID)
 
 	// Get the log collection
 	var coll arvados.Collection
-	err = arv.Get("collections", cr.LogUUID, arvadosclient.Dict{}, &coll)
+	err = ac.RequestAndDecodeContext(context.Background(), &coll, "GET", "arvados/v1/collections/"+cr.LogUUID, nil, nil)
 	c.Assert(err, check.IsNil)
 
 	// Create a node.json file -- the fixture doesn't actually contain the contents of the collection.
@@ -130,7 +131,11 @@ func createNodeJSON(c *check.C, arv *arvadosclient.ArvadosClient, ac *arvados.Cl
 	c.Assert(mtxt, check.NotNil)
 
 	// Update collection record
-	err = arv.Update("collections", cr.LogUUID, arvadosclient.Dict{"collection": arvadosclient.Dict{"manifest_text": mtxt}}, &coll)
+	err = ac.RequestAndDecodeContext(context.Background(), &coll, "PUT", "arvados/v1/collections/"+cr.LogUUID, nil, map[string]interface{}{
+		"collection": map[string]interface{}{
+			"manifest_text": mtxt,
+		},
+	})
 	c.Assert(err, check.IsNil)
 }
 
@@ -147,13 +152,13 @@ func (*Suite) TestContainerRequestUUID(c *check.C) {
 	// Run costanalyzer with 1 container request uuid
 	exitcode := Command.RunCommand("costanalyzer.test", []string{"-uuid", arvadostest.CompletedContainerRequestUUID}, &bytes.Buffer{}, &stdout, &stderr)
 	c.Check(exitcode, check.Equals, 0)
-	c.Check(stderr.String(), check.Matches, "(?ms).*supplied uuids in .*")
+	c.Assert(stdout.String(), check.Matches, "(?ms).*supplied uuids in .*")
 
 	uuidReport, err := ioutil.ReadFile("results/" + arvadostest.CompletedContainerRequestUUID + ".csv")
 	c.Assert(err, check.IsNil)
 	c.Check(string(uuidReport), check.Matches, "(?ms).*TOTAL,,,,,,,,,7.01302889")
 	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'
+	matches := re.FindStringSubmatch(stdout.String()) // matches[1] contains a string like 'results/2020-11-02-18-57-45-aggregate-costaccounting.csv'
 
 	aggregateCostReport, err := ioutil.ReadFile(matches[1])
 	c.Assert(err, check.IsNil)
@@ -166,7 +171,7 @@ func (*Suite) TestDoubleContainerRequestUUID(c *check.C) {
 	// Run costanalyzer with 2 container request uuids
 	exitcode := Command.RunCommand("costanalyzer.test", []string{"-uuid", arvadostest.CompletedContainerRequestUUID, "-uuid", arvadostest.CompletedContainerRequestUUID2}, &bytes.Buffer{}, &stdout, &stderr)
 	c.Check(exitcode, check.Equals, 0)
-	c.Check(stderr.String(), check.Matches, "(?ms).*supplied uuids in .*")
+	c.Assert(stdout.String(), check.Matches, "(?ms).*supplied uuids in .*")
 
 	uuidReport, err := ioutil.ReadFile("results/" + arvadostest.CompletedContainerRequestUUID + ".csv")
 	c.Assert(err, check.IsNil)
@@ -177,28 +182,36 @@ func (*Suite) TestDoubleContainerRequestUUID(c *check.C) {
 	c.Check(string(uuidReport2), check.Matches, "(?ms).*TOTAL,,,,,,,,,42.27031111")
 
 	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'
+	matches := re.FindStringSubmatch(stdout.String()) // matches[1] contains a string like 'results/2020-11-02-18-57-45-aggregate-costaccounting.csv'
 
 	aggregateCostReport, err := ioutil.ReadFile(matches[1])
 	c.Assert(err, check.IsNil)
 
 	c.Check(string(aggregateCostReport), check.Matches, "(?ms).*TOTAL,49.28334000")
+	stdout.Truncate(0)
+	stderr.Truncate(0)
 
 	// Now move both container requests into an existing project, and then re-run
 	// the analysis with the project uuid. The results should be identical.
-	arv, err := arvadosclient.MakeArvadosClient()
-	c.Assert(err, check.Equals, nil)
-
+	ac := arvados.NewClientFromEnv()
 	var cr arvados.ContainerRequest
-	err = arv.Update("container_requests", arvadostest.CompletedContainerRequestUUID, arvadosclient.Dict{"container_request": arvadosclient.Dict{"owner_uuid": arvadostest.AProjectUUID}}, &cr)
+	err = ac.RequestAndDecodeContext(context.Background(), &cr, "PUT", "arvados/v1/container_requests/"+arvadostest.CompletedContainerRequestUUID, nil, map[string]interface{}{
+		"container_request": map[string]interface{}{
+			"owner_uuid": arvadostest.AProjectUUID,
+		},
+	})
 	c.Assert(err, check.IsNil)
-	err = arv.Update("container_requests", arvadostest.CompletedContainerRequestUUID2, arvadosclient.Dict{"container_request": arvadosclient.Dict{"owner_uuid": arvadostest.AProjectUUID}}, &cr)
+	err = ac.RequestAndDecodeContext(context.Background(), &cr, "PUT", "arvados/v1/container_requests/"+arvadostest.CompletedContainerRequestUUID2, nil, map[string]interface{}{
+		"container_request": map[string]interface{}{
+			"owner_uuid": arvadostest.AProjectUUID,
+		},
+	})
 	c.Assert(err, check.IsNil)
 
 	// Run costanalyzer with the project uuid
-	exitcode = Command.RunCommand("costanalyzer.test", []string{"-uuid", arvadostest.AProjectUUID}, &bytes.Buffer{}, &stdout, &stderr)
+	exitcode = Command.RunCommand("costanalyzer.test", []string{"-uuid", arvadostest.AProjectUUID, "-cache=false", "-log-level", "debug"}, &bytes.Buffer{}, &stdout, &stderr)
 	c.Check(exitcode, check.Equals, 0)
-	c.Check(stderr.String(), check.Matches, "(?ms).*supplied uuids in .*")
+	c.Assert(stdout.String(), check.Matches, "(?ms).*supplied uuids in .*")
 
 	uuidReport, err = ioutil.ReadFile("results/" + arvadostest.CompletedContainerRequestUUID + ".csv")
 	c.Assert(err, check.IsNil)
@@ -209,7 +222,7 @@ func (*Suite) TestDoubleContainerRequestUUID(c *check.C) {
 	c.Check(string(uuidReport2), check.Matches, "(?ms).*TOTAL,,,,,,,,,42.27031111")
 
 	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'
+	matches = re.FindStringSubmatch(stdout.String()) // matches[1] contains a string like 'results/2020-11-02-18-57-45-aggregate-costaccounting.csv'
 
 	aggregateCostReport, err = ioutil.ReadFile(matches[1])
 	c.Assert(err, check.IsNil)
@@ -222,7 +235,7 @@ func (*Suite) TestMultipleContainerRequestUUIDWithReuse(c *check.C) {
 	// Run costanalyzer with 2 container request uuids
 	exitcode := Command.RunCommand("costanalyzer.test", []string{"-uuid", arvadostest.CompletedDiagnosticsContainerRequest1UUID, "-uuid", arvadostest.CompletedDiagnosticsContainerRequest2UUID}, &bytes.Buffer{}, &stdout, &stderr)
 	c.Check(exitcode, check.Equals, 0)
-	c.Check(stderr.String(), check.Matches, "(?ms).*supplied uuids in .*")
+	c.Assert(stdout.String(), check.Matches, "(?ms).*supplied uuids in .*")
 
 	uuidReport, err := ioutil.ReadFile("results/" + arvadostest.CompletedDiagnosticsContainerRequest1UUID + ".csv")
 	c.Assert(err, check.IsNil)
@@ -233,7 +246,7 @@ func (*Suite) TestMultipleContainerRequestUUIDWithReuse(c *check.C) {
 	c.Check(string(uuidReport2), check.Matches, "(?ms).*TOTAL,,,,,,,,,0.00586435")
 
 	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'
+	matches := re.FindStringSubmatch(stdout.String()) // matches[1] contains a string like 'results/2020-11-02-18-57-45-aggregate-costaccounting.csv'
 
 	aggregateCostReport, err := ioutil.ReadFile(matches[1])
 	c.Assert(err, check.IsNil)

-----------------------------------------------------------------------


hooks/post-receive
-- 




More information about the arvados-commits mailing list