From 55502267d690fed1a6f496bc5e680225d60dbcae Mon Sep 17 00:00:00 2001 From: Carlos Salguero Date: Thu, 14 May 2020 23:53:01 -0300 Subject: [PATCH 1/6] PT-1822 Fixed get hostnames for standalone --- src/go/docker-compose.yml | 9 +++------ src/go/internal/testutils/env.go | 5 +++++ src/go/mongolib/util/util.go | 10 ++++++++-- src/go/mongolib/util/{main_test.go => util_test.go} | 11 ++++++++++- 4 files changed, 26 insertions(+), 9 deletions(-) rename src/go/mongolib/util/{main_test.go => util_test.go} (94%) diff --git a/src/go/docker-compose.yml b/src/go/docker-compose.yml index 76df6a5b..5c8a8e0c 100644 --- a/src/go/docker-compose.yml +++ b/src/go/docker-compose.yml @@ -4,13 +4,10 @@ services: standalone: network_mode: host image: ${TEST_MONGODB_FLAVOR}:${TEST_PSMDB_VERSION} + environment: + MONGO_INITDB_ROOT_USERNAME: ${TEST_MONGODB_ADMIN_USERNAME} + MONGO_INITDB_ROOT_PASSWORD: ${TEST_MONGODB_ADMIN_PASSWORD} command: --port=27017 - volumes: - - ./docker/test/entrypoint-mongod.sh:/entrypoint.sh:ro - - ./docker/test/entrypoint-mongod.sh:/usr/local/bin/docker-entrypoint.sh:ro - - ./docker/test/mongod.key:/mongod.key:ro - - ./docker/test/ssl/rootCA.crt:/rootCA.crt:ro - - ./docker/test/ssl/mongodb.pem:/mongod.pem:ro s1-mongo1: network_mode: host image: ${TEST_MONGODB_FLAVOR}:${TEST_PSMDB_VERSION} diff --git a/src/go/internal/testutils/env.go b/src/go/internal/testutils/env.go index 906ce368..7e3fe148 100644 --- a/src/go/internal/testutils/env.go +++ b/src/go/internal/testutils/env.go @@ -30,6 +30,8 @@ const ( envMongoDBConfigsvr3Port = "TEST_MONGODB_CONFIGSVR3_PORT" // envMongoDBMongosPort = "TEST_MONGODB_MONGOS_PORT" + + envMongoDBStandalonePort = "TEST_MONGODB_STANDALONE_PORT" // envMongoDBUser = "TEST_MONGODB_ADMIN_USERNAME" envMongoDBPassword = "TEST_MONGODB_ADMIN_PASSWORD" @@ -39,6 +41,9 @@ var ( // MongoDBHost is the hostname. Since it runs locally, it is localhost MongoDBHost = "127.0.0.1" + // Port for standalone instance + MongoDBStandalonePort = os.Getenv(envMongoDBStandalonePort) + // MongoDBShard1ReplsetName Replicaset name for shard 1 MongoDBShard1ReplsetName = os.Getenv(envMongoDBShard1ReplsetName) // MongoDBShard1PrimaryPort is the port for the primary instance of shard 1 diff --git a/src/go/mongolib/util/util.go b/src/go/mongolib/util/util.go index 234c00fe..f27cd247 100644 --- a/src/go/mongolib/util/util.go +++ b/src/go/mongolib/util/util.go @@ -2,7 +2,6 @@ package util import ( "context" - "fmt" "sort" "strings" @@ -13,6 +12,10 @@ import ( "go.mongodb.org/mongo-driver/mongo/options" ) +const ( + shardingNotEnabledError = 203 +) + var ( CANNOT_GET_QUERY_ERROR = errors.New("cannot get query field from the profile document (it is not a map)") ) @@ -119,6 +122,9 @@ func GetHostnames(ctx context.Context, client *mongo.Client) ([]string, error) { var shardsMap proto.ShardsMap smRes := client.Database("admin").RunCommand(ctx, primitive.M{"getShardMap": 1}) if smRes.Err() != nil { + if e, ok := smRes.Err().(mongo.CommandError); ok && e.Code == shardingNotEnabledError { + return nil, nil // standalone instance + } return nil, errors.Wrap(smRes.Err(), "cannot getShardMap for GetHostnames") } if err := smRes.Decode(&shardsMap); err != nil { @@ -134,7 +140,7 @@ func GetHostnames(ctx context.Context, client *mongo.Client) ([]string, error) { } } - return nil, fmt.Errorf("cannot get shards map") + return nil, nil // standalone instance } func buildHostsListFromReplStatus(replStatus proto.ReplicaSetStatus) []string { diff --git a/src/go/mongolib/util/main_test.go b/src/go/mongolib/util/util_test.go similarity index 94% rename from src/go/mongolib/util/main_test.go rename to src/go/mongolib/util/util_test.go index 853046a4..ec6a2ca7 100644 --- a/src/go/mongolib/util/main_test.go +++ b/src/go/mongolib/util/util_test.go @@ -100,6 +100,11 @@ func TestGetReplicasetMembers(t *testing.T) { uri: fmt.Sprintf("mongodb://%s:%s@%s:%s", tu.MongoDBUser, tu.MongoDBPassword, tu.MongoDBHost, tu.MongoDBShard3PrimaryPort), want: 3, }, + { + name: "from_standalone", + uri: fmt.Sprintf("mongodb://%s:%s@%s:%s", tu.MongoDBUser, tu.MongoDBPassword, tu.MongoDBHost, tu.MongoDBStandalonePort), + want: 0, + }, } for _, test := range testCases { @@ -146,7 +151,7 @@ func TestGetShardedHosts(t *testing.T) { }, } - for _, test := range testCases { + for i, test := range testCases { t.Run(test.name, func(t *testing.T) { clientOptions := options.Client().ApplyURI(test.uri) ctx, cancel := context.WithTimeout(context.Background(), 20*time.Second) @@ -156,6 +161,10 @@ func TestGetShardedHosts(t *testing.T) { if err != nil { t.Errorf("Cannot get a new client for host %s: %s", test.uri, err) } + if client == nil { + panic(fmt.Sprintf("i: %d, uri: %s\n", i, test.uri)) + } + if err := client.Connect(ctx); err != nil { t.Errorf("Cannot connect to host %s: %s", test.uri, err) } From 1f33cb97e6b6e8c9146cc4d3fd5f0750e6e031cf Mon Sep 17 00:00:00 2001 From: Carlos Salguero Date: Mon, 25 May 2020 22:35:35 -0300 Subject: [PATCH 2/6] PT-1822 Fixed for CR --- Gopkg.lock | 6 +++--- src/go/mongolib/util/util.go | 11 ++++++----- src/go/mongolib/util/util_test.go | 2 +- src/go/pt-mongodb-summary/main.go | 21 +++++++++++++-------- 4 files changed, 23 insertions(+), 17 deletions(-) diff --git a/Gopkg.lock b/Gopkg.lock index 3b6ea521..b67395bc 100644 --- a/Gopkg.lock +++ b/Gopkg.lock @@ -188,12 +188,12 @@ revision = "197f4ad8db8d1b04ff408042119176907c971f0a" [[projects]] - digest = "1:1d7e1867c49a6dd9856598ef7c3123604ea3daabf5b83f303ff457bcbc410b1d" + digest = "1:c45802472e0c06928cd997661f2af610accd85217023b1d5f6331bebce0671d3" name = "github.com/pkg/errors" packages = ["."] pruneopts = "" - revision = "ba968bfe8b2f7e042a574c888954fccecfa385b4" - version = "v0.8.1" + revision = "614d223910a179a466c1767a985424175c39b465" + version = "v0.9.1" [[projects]] digest = "1:55dcddb2ba6ab25098ee6b96f176f39305f1fde7ea3d138e7e10bb64a5bf45be" diff --git a/src/go/mongolib/util/util.go b/src/go/mongolib/util/util.go index f27cd247..5936dca1 100644 --- a/src/go/mongolib/util/util.go +++ b/src/go/mongolib/util/util.go @@ -13,11 +13,12 @@ import ( ) const ( - shardingNotEnabledError = 203 + shardingNotEnabledErrorCode = 203 ) var ( - CANNOT_GET_QUERY_ERROR = errors.New("cannot get query field from the profile document (it is not a map)") + CANNOT_GET_QUERY_ERROR = errors.New("cannot get query field from the profile document (it is not a map)") + ShardingNotEnabledError = errors.New("sharding not enabled") ) func GetReplicasetMembers(ctx context.Context, clientOptions *options.ClientOptions) ([]proto.Members, error) { @@ -95,7 +96,7 @@ func GetReplicasetMembers(ctx context.Context, clientOptions *options.ClientOpti membersMap[m.Name] = m } - client.Disconnect(ctx) + client.Disconnect(ctx) //nolint } for _, member := range membersMap { @@ -122,8 +123,8 @@ func GetHostnames(ctx context.Context, client *mongo.Client) ([]string, error) { var shardsMap proto.ShardsMap smRes := client.Database("admin").RunCommand(ctx, primitive.M{"getShardMap": 1}) if smRes.Err() != nil { - if e, ok := smRes.Err().(mongo.CommandError); ok && e.Code == shardingNotEnabledError { - return nil, nil // standalone instance + if e, ok := smRes.Err().(mongo.CommandError); ok && e.Code == shardingNotEnabledErrorCode { + return nil, ShardingNotEnabledError // standalone instance } return nil, errors.Wrap(smRes.Err(), "cannot getShardMap for GetHostnames") } diff --git a/src/go/mongolib/util/util_test.go b/src/go/mongolib/util/util_test.go index ec6a2ca7..2165e243 100644 --- a/src/go/mongolib/util/util_test.go +++ b/src/go/mongolib/util/util_test.go @@ -162,7 +162,7 @@ func TestGetShardedHosts(t *testing.T) { t.Errorf("Cannot get a new client for host %s: %s", test.uri, err) } if client == nil { - panic(fmt.Sprintf("i: %d, uri: %s\n", i, test.uri)) + t.Fatalf("mongodb client is nil i: %d, uri: %s\n", i, test.uri) } if err := client.Connect(ctx); err != nil { diff --git a/src/go/pt-mongodb-summary/main.go b/src/go/pt-mongodb-summary/main.go index fe6c0d9d..b5d9a014 100644 --- a/src/go/pt-mongodb-summary/main.go +++ b/src/go/pt-mongodb-summary/main.go @@ -38,13 +38,19 @@ const ( DefaultRunningOpsSamples = 5 DefaultOutputFormat = "text" typeMongos = "mongos" + + // Exit Codes + cannotFormatResults = 1 + cannotParseCommandLineParameters = 2 + cannotGetHostInfo = 3 + cannotGetReplicasetMembers = 4 ) var ( Build string = "2020-04-23" // nolint GoVersion string = "1.14.1" // nolint - Version string = "3.2.0" // nolint - Commit string // nolint + Version string = "3.2.0" + Commit string ) type TimedStats struct { @@ -158,7 +164,7 @@ func main() { opts, err := parseFlags() if err != nil { log.Errorf("cannot get parameters: %s", err.Error()) - os.Exit(2) + os.Exit(cannotParseCommandLineParameters) } if opts == nil && err == nil { return @@ -206,7 +212,7 @@ func main() { defer client.Disconnect(ctx) // nolint hostnames, err := util.GetHostnames(ctx, client) - if err != nil { + if err != nil && errors.Is(err, util.ShardingNotEnabledError) { log.Errorf("Cannot get hostnames: %s", err) } log.Debugf("hostnames: %v", hostnames) @@ -217,12 +223,12 @@ func main() { if err != nil { message := fmt.Sprintf("Cannot get host info for %q: %s", opts.Host, err.Error()) log.Errorf(message) - os.Exit(2) + os.Exit(cannotGetHostInfo) } if ci.ReplicaMembers, err = util.GetReplicasetMembers(ctx, clientOptions); err != nil { log.Warnf("[Error] cannot get replicaset members: %v\n", err) - os.Exit(2) + os.Exit(cannotGetReplicasetMembers) } log.Debugf("replicaMembers:\n%+v\n", ci.ReplicaMembers) @@ -270,10 +276,9 @@ func main() { out, err := formatResults(ci, opts.OutputFormat) if err != nil { log.Errorf("Cannot format the results: %s", err.Error()) - os.Exit(1) + os.Exit(cannotFormatResults) } fmt.Println(string(out)) - } func formatResults(ci *collectedInfo, format string) ([]byte, error) { From 596b62c23b055d99cd9ea892dd9e02dd807f2848 Mon Sep 17 00:00:00 2001 From: Carlos Salguero Date: Wed, 27 May 2020 21:24:18 -0300 Subject: [PATCH 3/6] PT-1822 Fixed test --- src/go/mongolib/util/util_test.go | 37 ++++++++++++++++++------------- 1 file changed, 21 insertions(+), 16 deletions(-) diff --git a/src/go/mongolib/util/util_test.go b/src/go/mongolib/util/util_test.go index 2165e243..faf37f25 100644 --- a/src/go/mongolib/util/util_test.go +++ b/src/go/mongolib/util/util_test.go @@ -81,29 +81,34 @@ func TestGetServerStatus(t *testing.T) { func TestGetReplicasetMembers(t *testing.T) { testCases := []struct { - name string - uri string - want int + name string + uri string + want int + wantErr bool }{ { - name: "from_mongos", - uri: fmt.Sprintf("mongodb://%s:%s@%s:%s", tu.MongoDBUser, tu.MongoDBPassword, tu.MongoDBHost, tu.MongoDBMongosPort), - want: 7, + name: "from_mongos", + uri: fmt.Sprintf("mongodb://%s:%s@%s:%s", tu.MongoDBUser, tu.MongoDBPassword, tu.MongoDBHost, tu.MongoDBMongosPort), + want: 7, + wantErr: false, }, { - name: "from_mongod", - uri: fmt.Sprintf("mongodb://%s:%s@%s:%s", tu.MongoDBUser, tu.MongoDBPassword, tu.MongoDBHost, tu.MongoDBShard1PrimaryPort), - want: 3, + name: "from_mongod", + uri: fmt.Sprintf("mongodb://%s:%s@%s:%s", tu.MongoDBUser, tu.MongoDBPassword, tu.MongoDBHost, tu.MongoDBShard1PrimaryPort), + want: 3, + wantErr: false, }, { - name: "from_non_sharded", - uri: fmt.Sprintf("mongodb://%s:%s@%s:%s", tu.MongoDBUser, tu.MongoDBPassword, tu.MongoDBHost, tu.MongoDBShard3PrimaryPort), - want: 3, + name: "from_non_sharded", + uri: fmt.Sprintf("mongodb://%s:%s@%s:%s", tu.MongoDBUser, tu.MongoDBPassword, tu.MongoDBHost, tu.MongoDBShard3PrimaryPort), + want: 3, + wantErr: false, }, { - name: "from_standalone", - uri: fmt.Sprintf("mongodb://%s:%s@%s:%s", tu.MongoDBUser, tu.MongoDBPassword, tu.MongoDBHost, tu.MongoDBStandalonePort), - want: 0, + name: "from_standalone", + uri: fmt.Sprintf("mongodb://%s:%s@%s:%s", tu.MongoDBUser, tu.MongoDBPassword, tu.MongoDBHost, tu.MongoDBStandalonePort), + want: 0, + wantErr: true, }, } @@ -114,7 +119,7 @@ func TestGetReplicasetMembers(t *testing.T) { defer cancel() rsm, err := GetReplicasetMembers(ctx, clientOptions) - if err != nil { + if err != nil && !test.wantErr { t.Errorf("Got an error while getting replicaset members: %s", err) } if len(rsm) != test.want { From 1f62be3279e5f706adebe121234dd3ded84a7ad5 Mon Sep 17 00:00:00 2001 From: Carlos Salguero Date: Mon, 1 Jun 2020 11:50:13 -0300 Subject: [PATCH 4/6] Fixes fro CR --- src/go/mongolib/util/util.go | 9 +++++---- src/go/mongolib/util/util_test.go | 7 ++++++- 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/src/go/mongolib/util/util.go b/src/go/mongolib/util/util.go index 5936dca1..a96d7069 100644 --- a/src/go/mongolib/util/util.go +++ b/src/go/mongolib/util/util.go @@ -17,7 +17,7 @@ const ( ) var ( - CANNOT_GET_QUERY_ERROR = errors.New("cannot get query field from the profile document (it is not a map)") + CannotGetQueryError = errors.New("cannot get query field from the profile document (it is not a map)") ShardingNotEnabledError = errors.New("sharding not enabled") ) @@ -141,6 +141,7 @@ func GetHostnames(ctx context.Context, client *mongo.Client) ([]string, error) { } } + // Some MongoDB servers won't return ShardingNotEnabledError for stand alone instances. return nil, nil // standalone instance } @@ -272,7 +273,7 @@ func GetQueryField(doc proto.SystemProfile) (primitive.M, error) { if ssquery, ok := squery.(primitive.M); ok { return ssquery, nil } - return nil, CANNOT_GET_QUERY_ERROR + return nil, CannotGetQueryError } } } @@ -308,7 +309,7 @@ func GetQueryField(doc proto.SystemProfile) (primitive.M, error) { if ssquery, ok := squery.(primitive.M); ok { return ssquery, nil } - return nil, CANNOT_GET_QUERY_ERROR + return nil, CannotGetQueryError } // "query" in MongoDB 3.2+ is better structured and always has a "filter" subkey: @@ -316,7 +317,7 @@ func GetQueryField(doc proto.SystemProfile) (primitive.M, error) { if ssquery, ok := squery.(primitive.M); ok { return ssquery, nil } - return nil, CANNOT_GET_QUERY_ERROR + return nil, CannotGetQueryError } // {"ns":"test.system.js","op":"query","query":{"find":"system.js"}} diff --git a/src/go/mongolib/util/util_test.go b/src/go/mongolib/util/util_test.go index faf37f25..3f8b2fac 100644 --- a/src/go/mongolib/util/util_test.go +++ b/src/go/mongolib/util/util_test.go @@ -33,6 +33,11 @@ func TestGetHostnames(t *testing.T) { uri: fmt.Sprintf("mongodb://%s:%s@%s:%s", tu.MongoDBUser, tu.MongoDBPassword, tu.MongoDBHost, tu.MongoDBShard3PrimaryPort), want: []string{"127.0.0.1:17021", "127.0.0.1:17022", "127.0.0.1:17023"}, }, + { + name: "from_standalone", + uri: fmt.Sprintf("mongodb://%s:%s@%s:%s", tu.MongoDBUser, tu.MongoDBPassword, tu.MongoDBHost, tu.MongoDBStandalonePort), + want: nil, + }, } for _, test := range testCases { @@ -54,7 +59,7 @@ func TestGetHostnames(t *testing.T) { } if !reflect.DeepEqual(hostnames, test.want) { - t.Errorf("Invalid hostnames from mongos. Got: %+v, want %+v", hostnames, test.want) + t.Errorf("Invalid hostnames. Got: %+v, want %+v", hostnames, test.want) } }) } From 1da2cc944b9f995ee5c58c9eaa201a28f62cd19a Mon Sep 17 00:00:00 2001 From: Carlos Salguero Date: Mon, 1 Jun 2020 14:57:36 -0300 Subject: [PATCH 5/6] PT-1822 fixed test --- src/go/Makefile | 2 +- src/go/mongolib/util/util_test.go | 39 +++++++++++++++++-------------- 2 files changed, 23 insertions(+), 18 deletions(-) diff --git a/src/go/Makefile b/src/go/Makefile index b5231f2f..57064f2e 100644 --- a/src/go/Makefile +++ b/src/go/Makefile @@ -18,7 +18,7 @@ BIN_DIR=$(shell git rev-parse --show-toplevel)/bin SRC_DIR=$(shell git rev-parse --show-toplevel)/src/go LDFLAGS="-X main.Version=${VERSION} -X main.Build=${BUILD} -X main.GoVersion=${GOVERSION} -X main.Commit=${COMMIT} -s -w" -TEST_PSMDB_VERSION?=3.6 +TEST_PSMDB_VERSION?=4.0 TEST_MONGODB_FLAVOR?=percona/percona-server-mongodb TEST_MONGODB_ADMIN_USERNAME?=admin TEST_MONGODB_ADMIN_PASSWORD?=admin123456 diff --git a/src/go/mongolib/util/util_test.go b/src/go/mongolib/util/util_test.go index 3f8b2fac..0af6cf1d 100644 --- a/src/go/mongolib/util/util_test.go +++ b/src/go/mongolib/util/util_test.go @@ -14,29 +14,34 @@ import ( func TestGetHostnames(t *testing.T) { testCases := []struct { - name string - uri string - want []string + name string + uri string + want []string + wantError bool }{ { - name: "from_mongos", - uri: fmt.Sprintf("mongodb://%s:%s@%s:%s", tu.MongoDBUser, tu.MongoDBPassword, tu.MongoDBHost, tu.MongoDBMongosPort), - want: []string{"127.0.0.1:17001", "127.0.0.1:17002", "127.0.0.1:17004", "127.0.0.1:17005", "127.0.0.1:17007"}, + name: "from_mongos", + uri: fmt.Sprintf("mongodb://%s:%s@%s:%s", tu.MongoDBUser, tu.MongoDBPassword, tu.MongoDBHost, tu.MongoDBMongosPort), + want: []string{"127.0.0.1:17001", "127.0.0.1:17002", "127.0.0.1:17004", "127.0.0.1:17005", "127.0.0.1:17007"}, + wantError: false, }, { - name: "from_mongod", - uri: fmt.Sprintf("mongodb://%s:%s@%s:%s", tu.MongoDBUser, tu.MongoDBPassword, tu.MongoDBHost, tu.MongoDBShard1PrimaryPort), - want: []string{"127.0.0.1:17001", "127.0.0.1:17002", "127.0.0.1:17003"}, + name: "from_mongod", + uri: fmt.Sprintf("mongodb://%s:%s@%s:%s", tu.MongoDBUser, tu.MongoDBPassword, tu.MongoDBHost, tu.MongoDBShard1PrimaryPort), + want: []string{"127.0.0.1:17001", "127.0.0.1:17002", "127.0.0.1:17003"}, + wantError: false, }, { - name: "from_non_sharded", - uri: fmt.Sprintf("mongodb://%s:%s@%s:%s", tu.MongoDBUser, tu.MongoDBPassword, tu.MongoDBHost, tu.MongoDBShard3PrimaryPort), - want: []string{"127.0.0.1:17021", "127.0.0.1:17022", "127.0.0.1:17023"}, + name: "from_non_sharded", + uri: fmt.Sprintf("mongodb://%s:%s@%s:%s", tu.MongoDBUser, tu.MongoDBPassword, tu.MongoDBHost, tu.MongoDBShard3PrimaryPort), + want: []string{"127.0.0.1:17021", "127.0.0.1:17022", "127.0.0.1:17023"}, + wantError: false, }, { - name: "from_standalone", - uri: fmt.Sprintf("mongodb://%s:%s@%s:%s", tu.MongoDBUser, tu.MongoDBPassword, tu.MongoDBHost, tu.MongoDBStandalonePort), - want: nil, + name: "from_standalone", + uri: fmt.Sprintf("mongodb://%s:%s@%s:%s", tu.MongoDBUser, tu.MongoDBPassword, tu.MongoDBHost, tu.MongoDBStandalonePort), + want: nil, + wantError: true, }, } @@ -54,8 +59,8 @@ func TestGetHostnames(t *testing.T) { } hostnames, err := GetHostnames(ctx, client) - if err != nil { - t.Errorf("getHostnames: %v", err) + if err != nil && !test.wantError { + t.Errorf("Expecting error=nil, got: %v", err) } if !reflect.DeepEqual(hostnames, test.want) { From 3530c7bccd97619a7bd32a2796c1ee8dbe7237bc Mon Sep 17 00:00:00 2001 From: Carlos Salguero Date: Tue, 2 Jun 2020 11:59:26 -0300 Subject: [PATCH 6/6] PT-1822 Do not exit if rs is not enabled --- src/go/pt-mongodb-summary/main.go | 1 - 1 file changed, 1 deletion(-) diff --git a/src/go/pt-mongodb-summary/main.go b/src/go/pt-mongodb-summary/main.go index b5d9a014..1c443d66 100644 --- a/src/go/pt-mongodb-summary/main.go +++ b/src/go/pt-mongodb-summary/main.go @@ -228,7 +228,6 @@ func main() { if ci.ReplicaMembers, err = util.GetReplicasetMembers(ctx, clientOptions); err != nil { log.Warnf("[Error] cannot get replicaset members: %v\n", err) - os.Exit(cannotGetReplicasetMembers) } log.Debugf("replicaMembers:\n%+v\n", ci.ReplicaMembers)