8724: performKeepBlockCheck() returns error when any of the listed blocks are not...

diff --git a/tools/keep-block-check/keep-block-check.go b/tools/keep-block-check/keep-block-check.go
index 431703545a60017ec0b1d6bc83a7c85132036a23..caed9d04699cb3987c30af57c5cbd1823db08b5d 100644 (file)
--- a/tools/keep-block-check/keep-block-check.go
+++ b/tools/keep-block-check/keep-block-check.go
@@ -68,8 +68,7 @@ func doMain() error {
                return fmt.Errorf("Error configuring keepclient: %s", err.Error())
        }
 
-       performKeepBlockCheck(kc, blobSigningKey, *prefix, blockLocators)
-       return nil
+       return performKeepBlockCheck(kc, blobSigningKey, *prefix, blockLocators)
 }
 
 type apiConfig struct {
@@ -159,7 +158,7 @@ func setupKeepClient(config apiConfig, keepServicesJSON string) (kc *keepclient.
        return
 }
 
-// Get list of block locators from the given file
+// Get list of unique block locators from the given file
 func getBlockLocators(locatorFile string) (locators []string, err error) {
        if locatorFile == "" {
                err = errors.New("block-hash-file not specified")
@@ -172,23 +171,33 @@ func getBlockLocators(locatorFile string) (locators []string, err error) {
                return
        }
 
+       locatorMap := make(map[string]string)
        lines := strings.Split(string(content), "\n")
        for _, line := range lines {
                if line == "" {
                        continue
                }
-               locators = append(locators, strings.TrimSpace(line))
+               trimmedLine := strings.TrimSpace(line)
+               locatorMap[trimmedLine] = trimmedLine
+       }
+
+       for _, locator := range locatorMap {
+               locators = append(locators, locator)
        }
 
        return
 }
 
 // Get block headers from keep. Log any errors.
-func performKeepBlockCheck(kc *keepclient.KeepClient, blobSigningKey, prefix string, blockLocators []string) {
+func performKeepBlockCheck(kc *keepclient.KeepClient, blobSigningKey, prefix string, blockLocators []string) error {
+       totalBlocks := 0
+       notFoundBlocks := 0
        for _, locator := range blockLocators {
                if !strings.HasPrefix(locator, prefix) {
                        continue
                }
+
+               totalBlocks++
                getLocator := locator
                if blobSigningKey != "" {
                        expiresAt := time.Now().AddDate(0, 0, 1)
@@ -197,7 +206,13 @@ func performKeepBlockCheck(kc *keepclient.KeepClient, blobSigningKey, prefix str
 
                _, _, err := kc.Ask(getLocator)
                if err != nil {
+                       notFoundBlocks++
                        log.Printf("Error getting head info for block: %v %v", locator, err)
                }
        }
+       if notFoundBlocks > 0 {
+               return fmt.Errorf("Head information not found for %d out of %d blocks with matching prefix.", notFoundBlocks, totalBlocks)
+       }
+
+       return nil
 }

diff --git a/tools/keep-block-check/keep-block-check_test.go b/tools/keep-block-check/keep-block-check_test.go
index 8f8b927c9923150b174d2613ba3fd5c4620314f0..2ea1972a576d31237c021f6397043ed18ef8af63 100644 (file)
--- a/tools/keep-block-check/keep-block-check_test.go
+++ b/tools/keep-block-check/keep-block-check_test.go
@@ -96,7 +96,7 @@ func setupKeepBlockCheck(c *C, enforcePermissions bool) {
        arvadostest.StartKeep(2, enforcePermissions)
 
        // setup keepclients
-  var err error
+       var err error
        kc, err = setupKeepClient(config, keepServicesJSON)
        c.Check(err, IsNil)
 }
@@ -120,11 +120,13 @@ func setupConfigFile(c *C, fileName string) string {
        file, err := ioutil.TempFile(os.TempDir(), fileName)
        c.Check(err, IsNil)
 
+       // Add config to file. While at it, throw some extra white space
        fileContent := "ARVADOS_API_HOST=" + os.Getenv("ARVADOS_API_HOST") + "\n"
        fileContent += "ARVADOS_API_TOKEN=" + arvadostest.DataManagerToken + "\n"
+       fileContent += "\n"
        fileContent += "ARVADOS_API_HOST_INSECURE=" + os.Getenv("ARVADOS_API_HOST_INSECURE") + "\n"
-       fileContent += "ARVADOS_EXTERNAL_CLIENT=false\n"
-       fileContent += "ARVADOS_BLOB_SIGNING_KEY=abcdefg"
+       fileContent += " ARVADOS_EXTERNAL_CLIENT = false \n"
+       fileContent += "ARVADOS_BLOB_SIGNING_KEY=abcdefg\n"
 
        _, err = file.Write([]byte(fileContent))
        c.Check(err, IsNil)
@@ -137,10 +139,12 @@ func setupBlockHashFile(c *C, name string, blocks []string) string {
        file, err := ioutil.TempFile(os.TempDir(), name)
        c.Check(err, IsNil)
 
+       // Add the hashes to the file. While at it, throw some extra white space
        fileContent := ""
        for _, hash := range blocks {
-               fileContent += fmt.Sprintf("%s\n", hash)
+               fileContent += fmt.Sprintf(" %s \n", hash)
        }
+       fileContent += "\n"
        _, err = file.Write([]byte(fileContent))
        c.Check(err, IsNil)
 
@@ -165,35 +169,49 @@ func checkErrorLog(c *C, blocks []string, prefix, suffix string) {
 func (s *ServerRequiredSuite) TestBlockCheck(c *C) {
        setupKeepBlockCheck(c, false)
        setupTestData(c)
-       performKeepBlockCheck(kc, blobSigningKey, "", allLocators)
+       err := performKeepBlockCheck(kc, blobSigningKey, "", allLocators)
+       c.Check(err, IsNil)
        checkErrorLog(c, []string{}, "head", "Block not found") // no errors
 }
 
 func (s *ServerRequiredSuite) TestBlockCheckWithBlobSigning(c *C) {
        setupKeepBlockCheck(c, true)
        setupTestData(c)
-       performKeepBlockCheck(kc, blobSigningKey, "", allLocators)
+       err := performKeepBlockCheck(kc, blobSigningKey, "", allLocators)
+       c.Check(err, IsNil)
        checkErrorLog(c, []string{}, "head", "Block not found") // no errors
 }
 
 func (s *ServerRequiredSuite) TestBlockCheck_NoSuchBlock(c *C) {
        setupKeepBlockCheck(c, false)
        setupTestData(c)
-       performKeepBlockCheck(kc, blobSigningKey, "", []string{TestHash, TestHash2})
+       allLocators = append(allLocators, TestHash)
+       allLocators = append(allLocators, TestHash2)
+       err := performKeepBlockCheck(kc, blobSigningKey, "", allLocators)
+       c.Check(err, NotNil)
+       c.Assert(err.Error(), Equals, "Head information not found for 2 out of 7 blocks with matching prefix.")
        checkErrorLog(c, []string{TestHash, TestHash2}, "head", "Block not found")
 }
 
 func (s *ServerRequiredSuite) TestBlockCheck_NoSuchBlock_WithMatchingPrefix(c *C) {
        setupKeepBlockCheck(c, false)
        setupTestData(c)
-       performKeepBlockCheck(kc, blobSigningKey, "aaa", []string{TestHash, TestHash2})
+       allLocators = append(allLocators, TestHash)
+       allLocators = append(allLocators, TestHash2)
+       err := performKeepBlockCheck(kc, blobSigningKey, "aaa", allLocators)
+       c.Check(err, NotNil)
+       // Of the 7 blocks given, only two match the prefix and hence only those are checked
+       c.Assert(err.Error(), Equals, "Head information not found for 2 out of 2 blocks with matching prefix.")
        checkErrorLog(c, []string{TestHash, TestHash2}, "head", "Block not found")
 }
 
 func (s *ServerRequiredSuite) TestBlockCheck_NoSuchBlock_WithPrefixMismatch(c *C) {
        setupKeepBlockCheck(c, false)
        setupTestData(c)
-       performKeepBlockCheck(kc, blobSigningKey, "999", []string{TestHash, TestHash2})
+       allLocators = append(allLocators, TestHash)
+       allLocators = append(allLocators, TestHash2)
+       err := performKeepBlockCheck(kc, blobSigningKey, "999", allLocators)
+       c.Check(err, IsNil)
        checkErrorLog(c, []string{}, "head", "Block not found") // no errors
 }
 
@@ -202,14 +220,16 @@ func (s *ServerRequiredSuite) TestBlockCheck_NoSuchBlock_WithPrefixMismatch(c *C
 func (s *ServerRequiredSuite) TestErrorDuringKeepBlockCheck_FakeKeepservers(c *C) {
        keepServicesJSON = testKeepServicesJSON
        setupKeepBlockCheck(c, false)
-       performKeepBlockCheck(kc, blobSigningKey, "", []string{TestHash, TestHash2})
+       err := performKeepBlockCheck(kc, blobSigningKey, "", []string{TestHash, TestHash2})
+       c.Assert(err.Error(), Equals, "Head information not found for 2 out of 2 blocks with matching prefix.")
        checkErrorLog(c, []string{TestHash, TestHash2}, "head", "no such host")
 }
 
 func (s *ServerRequiredSuite) TestBlockCheck_BadSignature(c *C) {
        setupKeepBlockCheck(c, true)
        setupTestData(c)
-       performKeepBlockCheck(kc, "badblobsigningkey", "", []string{TestHash, TestHash2})
+       err := performKeepBlockCheck(kc, "badblobsigningkey", "", []string{TestHash, TestHash2})
+       c.Assert(err.Error(), Equals, "Head information not found for 2 out of 2 blocks with matching prefix.")
        checkErrorLog(c, []string{TestHash, TestHash2}, "head", "HTTP 403")
 }
 
@@ -290,20 +310,21 @@ func (s *DoMainTestSuite) Test_doMain_WithNoSuchBlockHashFile(c *C) {
 }
 
 func (s *DoMainTestSuite) Test_doMain(c *C) {
+       // Start keepservers.
+       arvadostest.StartKeep(2, false)
+       defer arvadostest.StopKeep(2)
+
        config := setupConfigFile(c, "config")
        defer os.Remove(config)
 
        locatorFile := setupBlockHashFile(c, "block-hash", []string{TestHash, TestHash2})
        defer os.Remove(locatorFile)
 
-       // Start keepservers.
-       arvadostest.StartKeep(2, false)
-       defer arvadostest.StopKeep(2)
-
        args := []string{"-config", config, "-block-hash-file", locatorFile}
        os.Args = append(os.Args, args...)
 
        err := doMain()
-       c.Check(err, IsNil)
+       c.Check(err, NotNil)
+       c.Assert(err.Error(), Equals, "Head information not found for 2 out of 2 blocks with matching prefix.")
        checkErrorLog(c, []string{TestHash, TestHash2}, "head", "Block not found")
 }