From 1fc5b4fe726b9dc98f9c65d0a8ec48ac7e0b6841 Mon Sep 17 00:00:00 2001 From: Sven Tennie Date: Thu, 21 Nov 2024 14:18:40 +0100 Subject: [PATCH 1/4] Improve ES migration test coverage Call migration code instead of faking it. --- services/brig/src/Brig/Index/Options.hs | 2 +- services/brig/test/integration/API/Search.hs | 53 +++++++++++++------- 2 files changed, 37 insertions(+), 18 deletions(-) diff --git a/services/brig/src/Brig/Index/Options.hs b/services/brig/src/Brig/Index/Options.hs index f9f382c0043..a12ca78111c 100644 --- a/services/brig/src/Brig/Index/Options.hs +++ b/services/brig/src/Brig/Index/Options.hs @@ -39,7 +39,7 @@ module Brig.Index.Options commandParser, mkCreateIndexSettings, toESServer, - ReindexFromAnotherIndexSettings, + ReindexFromAnotherIndexSettings (..), reindexDestIndex, reindexTimeoutSeconds, reindexEsConnection, diff --git a/services/brig/test/integration/API/Search.hs b/services/brig/test/integration/API/Search.hs index 14832d5e377..0689f3abcae 100644 --- a/services/brig/test/integration/API/Search.hs +++ b/services/brig/test/integration/API/Search.hs @@ -1,6 +1,7 @@ {-# LANGUAGE OverloadedRecordDot #-} {-# LANGUAGE PartialTypeSignatures #-} {-# LANGUAGE QuasiQuotes #-} +{-# LANGUAGE RecordWildCards #-} {-# OPTIONS_GHC -Wno-incomplete-uni-patterns #-} {-# OPTIONS_GHC -Wno-partial-type-signatures #-} {-# OPTIONS_GHC -Wno-redundant-constraints #-} @@ -33,12 +34,15 @@ import API.User.Util import Bilge import Bilge.Assert import Brig.App (initHttpManagerWithTLSConfig) +import Brig.Index.Eval (runCommand) +import Brig.Index.Options +import Brig.Index.Options qualified as IndexOpts +import Brig.Options (ElasticSearchOpts) import Brig.Options qualified as Opt import Brig.Options qualified as Opts import Control.Lens ((.~), (?~), (^.)) -import Control.Monad.Catch (MonadCatch, MonadThrow) -import Control.Retry -import Data.Aeson (FromJSON, Value, decode) +import Control.Monad.Catch (MonadCatch) +import Data.Aeson (Value, decode) import Data.Aeson qualified as Aeson import Data.Domain (Domain (Domain)) import Data.Handle (fromHandle) @@ -58,6 +62,7 @@ import Network.Wai qualified as Wai import Network.Wai.Handler.Warp qualified as Warp import Network.Wai.Test qualified as WaiTest import Safe (headMay) +import System.Logger qualified as Log import Test.QuickCheck (Arbitrary (arbitrary), generate) import Test.Tasty import Test.Tasty.HUnit @@ -653,8 +658,24 @@ testMigrationToNewIndex opts brig = do -- Run Migrations let newIndexName = opts ^. Opt.elasticsearchLens . Opt.indexLens - taskNodeId <- assertRight =<< runBH opts (ES.reindexAsync $ ES.mkReindexRequest (ES.IndexName oldESIndex) newIndexName) - runBH opts $ waitForTaskToComplete @ES.ReindexResponse taskNodeId + esOldOpts :: Opt.ElasticSearchOpts = (opts ^. Opt.elasticsearchLens) & (Opt.indexLens .~ oldIndexName) + esOldConnectionSettings :: ESConnectionSettings = toESConnectionSettings esOldOpts + reindexSettings = ReindexFromAnotherIndexSettings esOldConnectionSettings newIndexName 5 + esNewConnectionSettings = esOldConnectionSettings {esIndex = newIndexName} + replicas = 2 + shards = 2 + refreshInterval = 5 + esSettings = + IndexOpts.localElasticSettings + & IndexOpts.esConnection .~ esNewConnectionSettings + & IndexOpts.esIndexReplicas .~ ES.ReplicaCount replicas + & IndexOpts.esIndexShardCount .~ shards + & IndexOpts.esIndexRefreshInterval .~ refreshInterval + + logger <- Log.create Log.StdOut + liftIO $ do + runCommand logger $ Create esSettings opts.galley + runCommand logger $ ReindexFromAnotherIndex reindexSettings -- Phase 3: Using old index for search, writing to both indices, migrations have run refreshIndex brig @@ -688,6 +709,16 @@ testMigrationToNewIndex opts brig = do assertCanFindByName brig phase1TeamUser1 phase3NonTeamUser assertCanFindByName brig phase1TeamUser1 phase3TeamUser +toESConnectionSettings :: ElasticSearchOpts -> ESConnectionSettings +toESConnectionSettings opts = ESConnectionSettings {..} + where + toText (ES.Server url) = url + esServer = (fromRight undefined . URI.parseURI URI.strictURIParserOptions . Text.encodeUtf8 . toText) opts.url + esIndex = opts.index + esCaCert = opts.caCert + esInsecureSkipVerifyTls = opts.insecureSkipVerifyTls + esCredentials = opts.credentials + withOldESProxy :: (TestConstraints m, MonadUnliftIO m, HasCallStack) => Opt.Opts -> (Text -> Text -> m a) -> m a withOldESProxy opts f = do indexName <- randomHandle @@ -712,18 +743,6 @@ indexProxyServer idx opts mgr = else Wai.WPRResponse (Wai.responseLBS HTTP.status400 [] $ "Refusing to proxy to path=" <> cs (Wai.rawPathInfo req)) in waiProxyTo proxyApp Wai.defaultOnExc mgr -waitForTaskToComplete :: forall a m. (ES.MonadBH m, MonadThrow m, FromJSON a) => ES.TaskNodeId -> m () -waitForTaskToComplete taskNodeId = do - let policy = constantDelay 100000 <> limitRetries 30 - let retryCondition _ = fmap not . isTaskComplete - task <- retrying policy retryCondition (const $ ES.getTask @m @a taskNodeId) - taskCompleted <- isTaskComplete task - liftIO $ assertBool "Timed out waiting for task" taskCompleted - where - isTaskComplete :: Either ES.EsError (ES.TaskResponse a) -> m Bool - isTaskComplete (Left e) = liftIO $ assertFailure $ "Expected Right, got Left: " <> show e - isTaskComplete (Right taskRes) = pure $ ES.taskResponseCompleted taskRes - testWithBothIndices :: Opt.Opts -> Manager -> TestName -> WaiTest.Session a -> TestTree testWithBothIndices opts mgr name f = do testGroup From 815cb09b4d571b5fc55cd12e9086210d66d9b4cd Mon Sep 17 00:00:00 2001 From: Sven Tennie Date: Thu, 21 Nov 2024 14:40:21 +0100 Subject: [PATCH 2/4] Fix Types changed between Bloodhound versions. --- services/brig/test/integration/API/Search.hs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/brig/test/integration/API/Search.hs b/services/brig/test/integration/API/Search.hs index 0689f3abcae..aa0eb7c14aa 100644 --- a/services/brig/test/integration/API/Search.hs +++ b/services/brig/test/integration/API/Search.hs @@ -658,7 +658,7 @@ testMigrationToNewIndex opts brig = do -- Run Migrations let newIndexName = opts ^. Opt.elasticsearchLens . Opt.indexLens - esOldOpts :: Opt.ElasticSearchOpts = (opts ^. Opt.elasticsearchLens) & (Opt.indexLens .~ oldIndexName) + esOldOpts :: Opt.ElasticSearchOpts = (opts ^. Opt.elasticsearchLens) & (Opt.indexLens .~ (ES.IndexName oldESIndex)) esOldConnectionSettings :: ESConnectionSettings = toESConnectionSettings esOldOpts reindexSettings = ReindexFromAnotherIndexSettings esOldConnectionSettings newIndexName 5 esNewConnectionSettings = esOldConnectionSettings {esIndex = newIndexName} From d06e66f7722bc30658a63e26e60f70ae2aaadf08 Mon Sep 17 00:00:00 2001 From: Sven Tennie Date: Thu, 21 Nov 2024 17:18:10 +0100 Subject: [PATCH 3/4] Migrate with Reindex and ReindexSameOrNewer commands These were previously untested. --- services/brig/test/integration/API/Search.hs | 62 ++++++++++++++++++-- 1 file changed, 56 insertions(+), 6 deletions(-) diff --git a/services/brig/test/integration/API/Search.hs b/services/brig/test/integration/API/Search.hs index aa0eb7c14aa..b522a075735 100644 --- a/services/brig/test/integration/API/Search.hs +++ b/services/brig/test/integration/API/Search.hs @@ -40,6 +40,8 @@ import Brig.Index.Options qualified as IndexOpts import Brig.Options (ElasticSearchOpts) import Brig.Options qualified as Opt import Brig.Options qualified as Opts +import Cassandra qualified as C +import Cassandra.Options qualified as CassOpts import Control.Lens ((.~), (?~), (^.)) import Control.Monad.Catch (MonadCatch) import Data.Aeson (Value, decode) @@ -70,6 +72,7 @@ import Text.RawString.QQ (r) import URI.ByteString qualified as URI import UnliftIO (Concurrently (..), async, bracket, cancel, runConcurrently) import Util +import Util.Options (Endpoint) import Wire.API.Federation.API.Brig (SearchResponse (SearchResponse)) import Wire.API.Team.Feature import Wire.API.Team.SearchVisibility @@ -95,7 +98,11 @@ tests opts mgr galley brig = do testWithBothIndices opts mgr "Non ascii names" $ testSearchNonAsciiNames brig, testWithBothIndices opts mgr "user with umlaut" $ testSearchWithUmlaut brig, testWithBothIndices opts mgr "user with japanese name" $ testSearchCJK brig, - test mgr "migration to new index" $ testMigrationToNewIndex opts brig, + testGroup "index migration" $ + [ test mgr "migration to new index from existing index" $ testMigrationToNewIndex opts brig runReindexFromAnotherIndex, + test mgr "migration to new index from database" $ testMigrationToNewIndex opts brig (runReindexFromDatabase Reindex), + test mgr "migration to new index from database (force sync)" $ testMigrationToNewIndex opts brig (runReindexFromDatabase ReindexSameOrNewer) + ], testGroup "team A: SearchVisibilityStandard (= unrestricted outbound search)" $ [ testGroup "team A: SearchableByOwnTeam (= restricted inbound search)" $ [ testWithBothIndices opts mgr " I. non-team user cannot find team A member by display name" $ testSearchTeamMemberAsNonMemberDisplayName mgr brig galley FeatureStatusDisabled, @@ -613,8 +620,13 @@ testSearchOtherDomain opts brig = do -- cluster. This test spins up a proxy server to pass requests to our only ES -- server. The proxy server ensures that only requests to the 'old' index go -- through. -testMigrationToNewIndex :: (TestConstraints m, MonadUnliftIO m) => Opt.Opts -> Brig -> m () -testMigrationToNewIndex opts brig = do +testMigrationToNewIndex :: + (TestConstraints m, MonadUnliftIO m) => + Opt.Opts -> + Brig -> + (Log.Logger -> Opt.Opts -> ES.IndexName -> IO ()) -> + m () +testMigrationToNewIndex opts brig reindexCommand = do withOldESProxy opts $ \oldESUrl oldESIndex -> do let optsOldIndex = opts @@ -658,9 +670,9 @@ testMigrationToNewIndex opts brig = do -- Run Migrations let newIndexName = opts ^. Opt.elasticsearchLens . Opt.indexLens - esOldOpts :: Opt.ElasticSearchOpts = (opts ^. Opt.elasticsearchLens) & (Opt.indexLens .~ (ES.IndexName oldESIndex)) + oldIndexName = ES.IndexName oldESIndex + esOldOpts :: Opt.ElasticSearchOpts = (opts ^. Opt.elasticsearchLens) & (Opt.indexLens .~ oldIndexName) esOldConnectionSettings :: ESConnectionSettings = toESConnectionSettings esOldOpts - reindexSettings = ReindexFromAnotherIndexSettings esOldConnectionSettings newIndexName 5 esNewConnectionSettings = esOldConnectionSettings {esIndex = newIndexName} replicas = 2 shards = 2 @@ -675,7 +687,7 @@ testMigrationToNewIndex opts brig = do logger <- Log.create Log.StdOut liftIO $ do runCommand logger $ Create esSettings opts.galley - runCommand logger $ ReindexFromAnotherIndex reindexSettings + reindexCommand logger opts oldIndexName -- Phase 3: Using old index for search, writing to both indices, migrations have run refreshIndex brig @@ -709,6 +721,44 @@ testMigrationToNewIndex opts brig = do assertCanFindByName brig phase1TeamUser1 phase3NonTeamUser assertCanFindByName brig phase1TeamUser1 phase3TeamUser +runReindexFromAnotherIndex :: Log.Logger -> Opt.Opts -> ES.IndexName -> IO () +runReindexFromAnotherIndex logger opts oldIndexName = + let newIndexName = opts ^. Opt.elasticsearchLens . Opt.indexLens + esOldOpts :: Opt.ElasticSearchOpts = (opts ^. Opt.elasticsearchLens) & (Opt.indexLens .~ oldIndexName) + esOldConnectionSettings :: ESConnectionSettings = toESConnectionSettings esOldOpts + reindexSettings = ReindexFromAnotherIndexSettings esOldConnectionSettings newIndexName 5 + in runCommand logger $ ReindexFromAnotherIndex reindexSettings + +runReindexFromDatabase :: + (ElasticSettings -> CassandraSettings -> Endpoint -> Command) -> + Log.Logger -> + Opt.Opts -> + ES.IndexName -> + IO () +runReindexFromDatabase syncCommand logger opts oldIndexName = + let newIndexName = opts ^. Opt.elasticsearchLens . Opt.indexLens + esOldOpts :: Opt.ElasticSearchOpts = (opts ^. Opt.elasticsearchLens) & (Opt.indexLens .~ oldIndexName) + esOldConnectionSettings :: ESConnectionSettings = toESConnectionSettings esOldOpts + esNewConnectionSettings = esOldConnectionSettings {esIndex = newIndexName} + replicas = 2 + shards = 2 + refreshInterval = 5 + elasticSettings :: ElasticSettings = + IndexOpts.localElasticSettings + & IndexOpts.esConnection .~ esNewConnectionSettings + & IndexOpts.esIndexReplicas .~ ES.ReplicaCount replicas + & IndexOpts.esIndexShardCount .~ shards + & IndexOpts.esIndexRefreshInterval .~ refreshInterval + cassandraSettings :: CassandraSettings = + ( localCassandraSettings + & IndexOpts.cHost .~ (Text.unpack opts.cassandra.endpoint.host) + & IndexOpts.cPort .~ (opts.cassandra.endpoint.port) + & IndexOpts.cKeyspace .~ (C.Keyspace opts.cassandra.keyspace) + ) + + endpoint :: Endpoint = opts.galley + in runCommand logger $ syncCommand elasticSettings cassandraSettings endpoint + toESConnectionSettings :: ElasticSearchOpts -> ESConnectionSettings toESConnectionSettings opts = ESConnectionSettings {..} where From dedc6e09cf11b5065980f9d808afb25d27319245 Mon Sep 17 00:00:00 2001 From: Sven Tennie Date: Thu, 21 Nov 2024 17:45:16 +0100 Subject: [PATCH 4/4] Refactor: Extract function to align abstraction levels --- services/brig/test/integration/API/Search.hs | 35 +++++++++++--------- 1 file changed, 19 insertions(+), 16 deletions(-) diff --git a/services/brig/test/integration/API/Search.hs b/services/brig/test/integration/API/Search.hs index b522a075735..44ea01d712d 100644 --- a/services/brig/test/integration/API/Search.hs +++ b/services/brig/test/integration/API/Search.hs @@ -669,24 +669,10 @@ testMigrationToNewIndex opts brig reindexCommand = do assertCanFindByName brig phase1TeamUser1 phase2TeamUser -- Run Migrations - let newIndexName = opts ^. Opt.elasticsearchLens . Opt.indexLens - oldIndexName = ES.IndexName oldESIndex - esOldOpts :: Opt.ElasticSearchOpts = (opts ^. Opt.elasticsearchLens) & (Opt.indexLens .~ oldIndexName) - esOldConnectionSettings :: ESConnectionSettings = toESConnectionSettings esOldOpts - esNewConnectionSettings = esOldConnectionSettings {esIndex = newIndexName} - replicas = 2 - shards = 2 - refreshInterval = 5 - esSettings = - IndexOpts.localElasticSettings - & IndexOpts.esConnection .~ esNewConnectionSettings - & IndexOpts.esIndexReplicas .~ ES.ReplicaCount replicas - & IndexOpts.esIndexShardCount .~ shards - & IndexOpts.esIndexRefreshInterval .~ refreshInterval - + let oldIndexName = ES.IndexName oldESIndex logger <- Log.create Log.StdOut liftIO $ do - runCommand logger $ Create esSettings opts.galley + createCommand logger opts oldIndexName reindexCommand logger opts oldIndexName -- Phase 3: Using old index for search, writing to both indices, migrations have run @@ -721,6 +707,23 @@ testMigrationToNewIndex opts brig reindexCommand = do assertCanFindByName brig phase1TeamUser1 phase3NonTeamUser assertCanFindByName brig phase1TeamUser1 phase3TeamUser +createCommand :: Log.Logger -> Opt.Opts -> ES.IndexName -> IO () +createCommand logger opts oldIndexName = + let newIndexName = opts ^. Opt.elasticsearchLens . Opt.indexLens + esOldOpts :: Opt.ElasticSearchOpts = (opts ^. Opt.elasticsearchLens) & (Opt.indexLens .~ oldIndexName) + esOldConnectionSettings :: ESConnectionSettings = toESConnectionSettings esOldOpts + esNewConnectionSettings = esOldConnectionSettings {esIndex = newIndexName} + replicas = 2 + shards = 2 + refreshInterval = 5 + esSettings = + IndexOpts.localElasticSettings + & IndexOpts.esConnection .~ esNewConnectionSettings + & IndexOpts.esIndexReplicas .~ ES.ReplicaCount replicas + & IndexOpts.esIndexShardCount .~ shards + & IndexOpts.esIndexRefreshInterval .~ refreshInterval + in runCommand logger $ Create esSettings opts.galley + runReindexFromAnotherIndex :: Log.Logger -> Opt.Opts -> ES.IndexName -> IO () runReindexFromAnotherIndex logger opts oldIndexName = let newIndexName = opts ^. Opt.elasticsearchLens . Opt.indexLens