From 41e0df276eaaa548e692e44cd8f3c27c4692375e Mon Sep 17 00:00:00 2001 From: Ward Vandewege Date: Mon, 7 Jun 2021 18:00:46 -0400 Subject: [PATCH] 17717: address further review comments: remove unused code and refactor a bit. Arvados-DCO-1.1-Signed-off-by: Ward Vandewege --- go.mod | 6 ++-- go.sum | 16 ++++++++++ lib/costanalyzer/cmd.go | 19 +++++++----- lib/costanalyzer/costanalyzer.go | 43 ++++++++++++++------------- lib/costanalyzer/costanalyzer_test.go | 4 +++ 5 files changed, 56 insertions(+), 32 deletions(-) diff --git a/go.mod b/go.mod index aa289761ba..0ff679a576 100644 --- a/go.mod +++ b/go.mod @@ -59,10 +59,10 @@ require ( github.com/src-d/gcfg v1.3.0 // indirect github.com/xanzy/ssh-agent v0.1.0 // indirect golang.org/x/crypto v0.0.0-20200622213623-75b288015ac9 - golang.org/x/net v0.0.0-20201021035429-f5854403a974 + golang.org/x/net v0.0.0-20210405180319-a5a99cb37ef4 golang.org/x/oauth2 v0.0.0-20190604053449-0f29369cfe45 - golang.org/x/sys v0.0.0-20210119212857-b64e53b001e4 - golang.org/x/tools v0.1.0 // indirect + golang.org/x/sys v0.0.0-20210510120138-977fb7262007 + golang.org/x/tools v0.1.2 // indirect google.golang.org/api v0.13.0 gopkg.in/asn1-ber.v1 v1.0.0-20181015200546-f715ec2f112d // indirect gopkg.in/check.v1 v1.0.0-20161208181325-20d25e280405 diff --git a/go.sum b/go.sum index 006118ad94..c28ab46240 100644 --- a/go.sum +++ b/go.sum @@ -249,6 +249,8 @@ github.com/xanzy/ssh-agent v0.1.0 h1:lOhdXLxtmYjaHc76ZtNmJWPg948y/RnT+3N3cvKWFzY github.com/xanzy/ssh-agent v0.1.0/go.mod h1:0NyE30eGUDliuLEHJgYte/zncp2zdTStcOnWhgSqHD8= github.com/yuin/goldmark v1.2.1 h1:ruQGxdhGHe7FWOJPT0mKs5+pD2Xs1Bm/kdGlHO04FmM= github.com/yuin/goldmark v1.2.1/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74= +github.com/yuin/goldmark v1.3.5 h1:dPmz1Snjq0kmkz159iL7S6WzdahUTHnHB5M56WFVifs= +github.com/yuin/goldmark v1.3.5/go.mod h1:mwnBkeHKe2W/ZEtQ+71ViKU8L12m81fl3OWwC1Zlc8k= go.opencensus.io v0.21.0 h1:mU6zScU4U1YAFPHEHYk+3JC4SY7JxgkqS10ZOSyksNg= go.opencensus.io v0.21.0/go.mod h1:mSImk1erAIZhrmZN+AvHh14ztQfjbGwt4TtuofqLduU= golang.org/x/crypto v0.0.0-20180904163835-0709b304e793/go.mod h1:6SG95UA2DQfeDnfUPMdvaQW0Q7yPrPDi9nlGo2tz2b4= @@ -266,6 +268,8 @@ golang.org/x/lint v0.0.0-20190409202823-959b441ac422 h1:QzoH/1pFpZguR8NrRHLcO6jK golang.org/x/lint v0.0.0-20190409202823-959b441ac422/go.mod h1:6SW0HCj/g11FgYtHlgUYUwCkIfeOF89ocIRzGO/8vkc= golang.org/x/mod v0.3.0 h1:RM4zey1++hCTbCVQfnWeKs9/IEsaBLA8vTkd0WVtmH4= golang.org/x/mod v0.3.0/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA= +golang.org/x/mod v0.4.2 h1:Gz96sIWK3OalVv/I/qNygP42zyoKp3xptRVCWRFEBvo= +golang.org/x/mod v0.4.2/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA= golang.org/x/net v0.0.0-20180724234803-3673e40ba225/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4= golang.org/x/net v0.0.0-20180826012351-8a410e7b638d/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4= golang.org/x/net v0.0.0-20181114220301-adae6a3d119a/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4= @@ -280,6 +284,8 @@ golang.org/x/net v0.0.0-20190620200207-3b0461eec859/go.mod h1:z5CRVTTTmAJ677TzLL golang.org/x/net v0.0.0-20200202094626-16171245cfb2/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s= golang.org/x/net v0.0.0-20201021035429-f5854403a974 h1:IX6qOQeG5uLjB/hjjwjedwfjND0hgjPMMyO1RoIXQNI= golang.org/x/net v0.0.0-20201021035429-f5854403a974/go.mod h1:sp8m0HH+o8qH0wwXwYZr8TS3Oi6o0r6Gce1SSxlDquU= +golang.org/x/net v0.0.0-20210405180319-a5a99cb37ef4 h1:4nGaVu0QrbjT/AK2PRLuQfQuh6DJve+pELhqTdAj3x0= +golang.org/x/net v0.0.0-20210405180319-a5a99cb37ef4/go.mod h1:p54w0d4576C0XHj96bSt6lcn1PtDYWL6XObtHCRCNQM= golang.org/x/oauth2 v0.0.0-20180821212333-d2e6202438be/go.mod h1:N/0e6XlmueqKjAGxoOufVs8QHGRruUQn6yWY3a++T0U= golang.org/x/oauth2 v0.0.0-20190226205417-e64efc72b421/go.mod h1:gOpvHmFTYa4IltrdGE7lF6nIHvwfUNPOp7c8zoXwtLw= golang.org/x/oauth2 v0.0.0-20190604053449-0f29369cfe45 h1:SVwTIAaPC2U/AvvLNZ2a7OVsmBpC8L5BlwK1whH3hm0= @@ -291,6 +297,8 @@ golang.org/x/sync v0.0.0-20190227155943-e225da77a7e6/go.mod h1:RxMgew5VJxzue5/jJ golang.org/x/sync v0.0.0-20190423024810-112230192c58/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sync v0.0.0-20201020160332-67f06af15bc9 h1:SQFwaSi55rU7vdNs9Yr0Z324VNlrF+0wMqRXT4St8ck= golang.org/x/sync v0.0.0-20201020160332-67f06af15bc9/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= +golang.org/x/sync v0.0.0-20210220032951-036812b2e83c h1:5KslGYwFpkhGh+Q16bwMP3cOontH8FOep7tGV86Y7SQ= +golang.org/x/sync v0.0.0-20210220032951-036812b2e83c/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sys v0.0.0-20180830151530-49385e6e1522/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= golang.org/x/sys v0.0.0-20180905080454-ebe1bf3edb33/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= golang.org/x/sys v0.0.0-20181116152217-5ac8a444bdc5/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= @@ -302,8 +310,14 @@ golang.org/x/sys v0.0.0-20190507160741-ecd444e8653b/go.mod h1:h1NjWce9XRLGQEsW7w golang.org/x/sys v0.0.0-20191005200804-aed5e4c7ecf9/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20191010194322-b09406accb47/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20200930185726-fdedc70b468f/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= +golang.org/x/sys v0.0.0-20201119102817-f84b799fce68/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20210119212857-b64e53b001e4 h1:myAQVi0cGEoqQVR5POX+8RR2mrocKqNN1hmeMqhX27k= golang.org/x/sys v0.0.0-20210119212857-b64e53b001e4/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= +golang.org/x/sys v0.0.0-20210330210617-4fbd30eecc44/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= +golang.org/x/sys v0.0.0-20210510120138-977fb7262007 h1:gG67DSER+11cZvqIMb8S8bt0vZtiN6xWYARwirrOSfE= +golang.org/x/sys v0.0.0-20210510120138-977fb7262007/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= +golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1 h1:v+OssWQX+hTHEmOBgwxdZxK4zHq3yOs8F9J7mk0PY8E= +golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo= golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= golang.org/x/text v0.3.1-0.20180807135948-17ff2d5776d2/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= golang.org/x/text v0.3.2/go.mod h1:bEr9sfX3Q8Zfm5fL9x+3itogRgK3+ptLWKqgva+5dAk= @@ -321,6 +335,8 @@ golang.org/x/tools v0.0.0-20190506145303-2d16b83fe98c/go.mod h1:RgjU9mgBXZiqYHBn golang.org/x/tools v0.0.0-20191119224855-298f0cb1881e/go.mod h1:b+2E5dAYhXwXZwtnZ6UAqBI28+e2cm9otk0dWdXHAEo= golang.org/x/tools v0.1.0 h1:po9/4sTYwZU9lPhi1tOrb4hCv3qrhiQ77LZfGa2OjwY= golang.org/x/tools v0.1.0/go.mod h1:xkSsbof2nBLbhDlRMhhhyNLN/zl3eTqcnHD5viDpcZ0= +golang.org/x/tools v0.1.2 h1:kRBLX7v7Af8W7Gdbbc908OJcdgtK8bOz9Uaj8/F1ACA= +golang.org/x/tools v0.1.2/go.mod h1:o0xws9oXOQQZyjljx8fwUC0k7L1pTE6eaCbjGeHmOkk= golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= golang.org/x/xerrors v0.0.0-20191011141410-1b5146add898/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= diff --git a/lib/costanalyzer/cmd.go b/lib/costanalyzer/cmd.go index 9b0685225b..c44f328620 100644 --- a/lib/costanalyzer/cmd.go +++ b/lib/costanalyzer/cmd.go @@ -6,15 +6,21 @@ package costanalyzer import ( "io" + "time" - "git.arvados.org/arvados.git/lib/config" "git.arvados.org/arvados.git/sdk/go/ctxlog" "github.com/sirupsen/logrus" ) -var Command command +var Command = &command{} -type command struct{} +type command struct { + uuids arrayFlags + resultsDir string + cache bool + begin time.Time + end time.Time +} type NoPrefixFormatter struct{} @@ -23,7 +29,7 @@ func (f *NoPrefixFormatter) Format(entry *logrus.Entry) ([]byte, error) { } // RunCommand implements the subcommand "costanalyzer ..." -func (command) RunCommand(prog string, args []string, stdin io.Reader, stdout, stderr io.Writer) int { +func (c *command) RunCommand(prog string, args []string, stdin io.Reader, stdout, stderr io.Writer) int { var err error logger := ctxlog.New(stderr, "text", "info") defer func() { @@ -34,10 +40,7 @@ func (command) RunCommand(prog string, args []string, stdin io.Reader, stdout, s logger.SetFormatter(new(NoPrefixFormatter)) - loader := config.NewLoader(stdin, logger) - loader.SkipLegacy = true - - exitcode, err := costanalyzer(prog, args, loader, logger, stdout, stderr) + exitcode, err := c.costAnalyzer(prog, args, logger, stdout, stderr) return exitcode } diff --git a/lib/costanalyzer/costanalyzer.go b/lib/costanalyzer/costanalyzer.go index 6087c88e41..9773109ad4 100644 --- a/lib/costanalyzer/costanalyzer.go +++ b/lib/costanalyzer/costanalyzer.go @@ -9,7 +9,6 @@ import ( "errors" "flag" "fmt" - "git.arvados.org/arvados.git/lib/config" "git.arvados.org/arvados.git/sdk/go/arvados" "git.arvados.org/arvados.git/sdk/go/arvadosclient" "git.arvados.org/arvados.git/sdk/go/keepclient" @@ -53,7 +52,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, begin time.Time, end time.Time, err error) { +func (c *command) parseFlags(prog string, args []string, logger *logrus.Logger, stderr io.Writer) (exitCode int, err error) { var beginStr, endStr string flags := flag.NewFlagSet("", flag.ContinueOnError) flags.SetOutput(stderr) @@ -122,10 +121,10 @@ Options: flags.PrintDefaults() } loglevel := flags.String("log-level", "info", "logging `level` (debug, info, ...)") - flags.StringVar(&resultsDir, "output", "", "output `directory` for the CSV reports") + flags.StringVar(&c.resultsDir, "output", "", "output `directory` for the CSV reports") flags.StringVar(&beginStr, "begin", "", fmt.Sprintf("timestamp `begin` for date range operation (format: %s)", timestampFormat)) flags.StringVar(&endStr, "end", "", fmt.Sprintf("timestamp `end` for date range operation (format: %s)", timestampFormat)) - flags.BoolVar(&cache, "cache", true, "create and use a local disk cache of Arvados objects") + flags.BoolVar(&c.cache, "cache", true, "create and use a local disk cache of Arvados objects") err = flags.Parse(args) if err == flag.ErrHelp { err = nil @@ -135,7 +134,7 @@ Options: exitCode = 2 return } - uuids = flags.Args() + c.uuids = flags.Args() if (len(beginStr) != 0 && len(endStr) == 0) || (len(beginStr) == 0 && len(endStr) != 0) { flags.Usage() @@ -146,8 +145,8 @@ Options: if len(beginStr) != 0 { var errB, errE error - begin, errB = time.Parse(timestampFormat, beginStr) - end, errE = time.Parse(timestampFormat, endStr) + c.begin, errB = time.Parse(timestampFormat, beginStr) + c.end, errE = time.Parse(timestampFormat, endStr) if (errB != nil) || (errE != nil) { flags.Usage() err = fmt.Errorf("When specifying a date range, both begin and end must be of the format %s %+v, %+v", timestampFormat, errB, errE) @@ -156,12 +155,13 @@ Options: } } - if (len(uuids) < 1) && (len(beginStr) == 0) { + if (len(c.uuids) < 1) && (len(beginStr) == 0) { flags.Usage() err = fmt.Errorf("error: no uuid(s) provided") exitCode = 2 return } + fmt.Printf("UUIDS: %s\n", c.uuids) lvl, err := logrus.ParseLevel(*loglevel) if err != nil { @@ -169,7 +169,7 @@ Options: return } logger.SetLevel(lvl) - if !cache { + if !c.cache { logger.Debug("Caching disabled\n") } return @@ -509,13 +509,14 @@ func generateCrCsv(logger *logrus.Logger, uuid string, arv *arvadosclient.Arvado return } -func costanalyzer(prog string, args []string, loader *config.Loader, logger *logrus.Logger, stdout, stderr io.Writer) (exitcode int, err error) { - exitcode, uuids, resultsDir, cache, begin, end, err := parseFlags(prog, args, loader, logger, stderr) +func (c *command) costAnalyzer(prog string, args []string, logger *logrus.Logger, stdout, stderr io.Writer) (exitcode int, err error) { + exitcode, err = c.parseFlags(prog, args, logger, stderr) + if exitcode != 0 { return } - if resultsDir != "" { - err = ensureDirectory(logger, resultsDir) + if c.resultsDir != "" { + err = ensureDirectory(logger, c.resultsDir) if err != nil { exitcode = 3 return @@ -543,13 +544,13 @@ func costanalyzer(prog string, args []string, loader *config.Loader, logger *log // Populate uuidChannel with the requested uuid list go func() { defer close(uuidChannel) - for _, uuid := range uuids { + for _, uuid := range c.uuids { uuidChannel <- uuid } - if !begin.IsZero() { + if !c.begin.IsZero() { initialParams := arvados.ResourceListParams{ - Filters: []arvados.Filter{{"container.finished_at", ">=", begin}, {"container.finished_at", "<", end}, {"requesting_container_uuid", "=", nil}}, + Filters: []arvados.Filter{{"container.finished_at", ">=", c.begin}, {"container.finished_at", "<", c.end}, {"requesting_container_uuid", "=", nil}}, Order: "created_at", } params := initialParams @@ -584,7 +585,7 @@ func costanalyzer(prog string, args []string, loader *config.Loader, logger *log fmt.Printf("Considering %s\n", uuid) if strings.Contains(uuid, "-j7d0g-") { // This is a project (group) - cost, err = handleProject(logger, uuid, arv, ac, kc, resultsDir, cache) + cost, err = handleProject(logger, uuid, arv, ac, kc, c.resultsDir, c.cache) if err != nil { exitcode = 1 return @@ -595,7 +596,7 @@ func costanalyzer(prog string, args []string, loader *config.Loader, logger *log } else if strings.Contains(uuid, "-xvhdp-") || strings.Contains(uuid, "-4zz18-") { // This is a container request var crCsv map[string]float64 - crCsv, err = generateCrCsv(logger, uuid, arv, ac, kc, resultsDir, cache) + crCsv, err = generateCrCsv(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 @@ -625,7 +626,7 @@ func costanalyzer(prog string, args []string, loader *config.Loader, logger *log var csv string csv = "# Aggregate cost accounting for uuids:\n" - for _, uuid := range uuids { + for _, uuid := range c.uuids { csv += "# " + uuid + "\n" } @@ -637,9 +638,9 @@ func costanalyzer(prog string, args []string, loader *config.Loader, logger *log csv += "TOTAL," + strconv.FormatFloat(total, 'f', 8, 64) + "\n" - if resultsDir != "" { + if c.resultsDir != "" { // Write the resulting CSV file - aFile := resultsDir + "/" + time.Now().Format("2006-01-02-15-04-05") + "-aggregate-costaccounting.csv" + aFile := c.resultsDir + "/" + time.Now().Format("2006-01-02-15-04-05") + "-aggregate-costaccounting.csv" err = ioutil.WriteFile(aFile, []byte(csv), 0644) if err != nil { err = fmt.Errorf("error writing file with path %s: %s", aFile, err.Error()) diff --git a/lib/costanalyzer/costanalyzer_test.go b/lib/costanalyzer/costanalyzer_test.go index bf280ec0c5..a3c123b516 100644 --- a/lib/costanalyzer/costanalyzer_test.go +++ b/lib/costanalyzer/costanalyzer_test.go @@ -114,6 +114,10 @@ func (s *Suite) SetUpSuite(c *check.C) { createNodeJSON(c, arv, ac, kc, arvadostest.CompletedDiagnosticsHasher3ContainerRequestUUID, arvadostest.Hasher3LogCollectionUUID, legacyD1V2JSON) } +func (s *Suite) SetUpTest(c *check.C) { + Command = &command{} +} + 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 -- 2.30.2