Merge pull request #263 from github/implied-key-fix

assume-master-host now applies ImpliedKey

Author: Shlomi Noach

go/logic/migrator.go

@@ -616,16 +616,22 @@ func (this *Migrator) initiateInspector() (err error) {
 	}
 	// So far so good, table is accessible and valid.
 	// Let's get master connection config
-	if this.migrationContext.ApplierConnectionConfig, err = this.inspector.getMasterConnectionConfig(); err != nil {
-		return err
-	}
-	if this.migrationContext.AssumeMasterHostname != "" {
-		if key, err := mysql.ParseRawInstanceKeyLoose(this.migrationContext.AssumeMasterHostname); err != nil {
+	if this.migrationContext.AssumeMasterHostname == "" {
+		// No forced master host; detect master
+		if this.migrationContext.ApplierConnectionConfig, err = this.inspector.getMasterConnectionConfig(); err != nil {
+			return err
+		}
+		log.Infof("Master found to be %+v", *this.migrationContext.ApplierConnectionConfig.ImpliedKey)
+	} else {
+		// Forced master host.
+		key, err := mysql.ParseRawInstanceKeyLoose(this.migrationContext.AssumeMasterHostname)
+		if err != nil {
 			return err
-		} else {
-			this.migrationContext.ApplierConnectionConfig.Key = *key
 		}
+		this.migrationContext.ApplierConnectionConfig = this.migrationContext.InspectorConnectionConfig.DuplicateCredentials(*key)
+		log.Infof("Master forced to be %+v", *this.migrationContext.ApplierConnectionConfig.ImpliedKey)
 	}
+	// validate configs
 	if this.migrationContext.TestOnReplica || this.migrationContext.MigrateOnReplica {
 		if this.migrationContext.InspectorIsAlsoApplier() {
 			return fmt.Errorf("Instructed to --test-on-replica or --migrate-on-replica, but the server we connect to doesn't seem to be a replica")
@@ -638,13 +644,12 @@ func (this *Migrator) initiateInspector() (err error) {
 			this.migrationContext.AddThrottleControlReplicaKey(this.migrationContext.InspectorConnectionConfig.Key)
 		}
 	} else if this.migrationContext.InspectorIsAlsoApplier() && !this.migrationContext.AllowedRunningOnMaster {
-		return fmt.Errorf("It seems like this migration attempt to run directly on master. Preferably it would be executed on a replica (and this reduces load from the master). To proceed please provide --allow-on-master")
+		return fmt.Errorf("It seems like this migration attempt to run directly on master. Preferably it would be executed on a replica (and this reduces load from the master). To proceed please provide --allow-on-master. Inspector config=%+v, applier config=%+v", this.migrationContext.InspectorConnectionConfig, this.migrationContext.ApplierConnectionConfig)
 	}
 	if err := this.inspector.validateLogSlaveUpdates(); err != nil {
 		return err
 	}
 
-	log.Infof("Master found to be %+v", *this.migrationContext.ApplierConnectionConfig.ImpliedKey)
 	return nil
 }
 

go/mysql/connection.go

@@ -26,19 +26,21 @@ func NewConnectionConfig() *ConnectionConfig {
 	return config
 }
 
-func (this *ConnectionConfig) Duplicate() *ConnectionConfig {
+// DuplicateCredentials creates a new connection config with given key and with same credentials as this config
+func (this *ConnectionConfig) DuplicateCredentials(key InstanceKey) *ConnectionConfig {
 	config := &ConnectionConfig{
-		Key: InstanceKey{
-			Hostname: this.Key.Hostname,
-			Port:     this.Key.Port,
-		},
+		Key:      key,
 		User:     this.User,
 		Password: this.Password,
 	}
 	config.ImpliedKey = &config.Key
 	return config
 }
 
+func (this *ConnectionConfig) Duplicate() *ConnectionConfig {
+	return this.DuplicateCredentials(this.Key)
+}
+
 func (this *ConnectionConfig) String() string {
 	return fmt.Sprintf("%s, user=%s", this.Key.DisplayString(), this.User)
 }

go/mysql/connection_test.go

@@ -0,0 +1,57 @@
+/*
+   Copyright 2016 GitHub Inc.
+	 See https://github.com/github/gh-ost/blob/master/LICENSE
+*/
+
+package mysql
+
+import (
+	"testing"
+
+	"github.com/outbrain/golib/log"
+	test "github.com/outbrain/golib/tests"
+)
+
+func init() {
+	log.SetLevel(log.ERROR)
+}
+
+func TestNewConnectionConfig(t *testing.T) {
+	c := NewConnectionConfig()
+	test.S(t).ExpectEquals(c.Key.Hostname, "")
+	test.S(t).ExpectEquals(c.Key.Port, 0)
+	test.S(t).ExpectEquals(c.ImpliedKey.Hostname, "")
+	test.S(t).ExpectEquals(c.ImpliedKey.Port, 0)
+	test.S(t).ExpectEquals(c.User, "")
+	test.S(t).ExpectEquals(c.Password, "")
+}
+
+func TestDuplicateCredentials(t *testing.T) {
+	c := NewConnectionConfig()
+	c.Key = InstanceKey{Hostname: "myhost", Port: 3306}
+	c.User = "gromit"
+	c.Password = "penguin"
+
+	dup := c.DuplicateCredentials(InstanceKey{Hostname: "otherhost", Port: 3310})
+	test.S(t).ExpectEquals(dup.Key.Hostname, "otherhost")
+	test.S(t).ExpectEquals(dup.Key.Port, 3310)
+	test.S(t).ExpectEquals(dup.ImpliedKey.Hostname, "otherhost")
+	test.S(t).ExpectEquals(dup.ImpliedKey.Port, 3310)
+	test.S(t).ExpectEquals(dup.User, "gromit")
+	test.S(t).ExpectEquals(dup.Password, "penguin")
+}
+
+func TestDuplicate(t *testing.T) {
+	c := NewConnectionConfig()
+	c.Key = InstanceKey{Hostname: "myhost", Port: 3306}
+	c.User = "gromit"
+	c.Password = "penguin"
+
+	dup := c.Duplicate()
+	test.S(t).ExpectEquals(dup.Key.Hostname, "myhost")
+	test.S(t).ExpectEquals(dup.Key.Port, 3306)
+	test.S(t).ExpectEquals(dup.ImpliedKey.Hostname, "myhost")
+	test.S(t).ExpectEquals(dup.ImpliedKey.Port, 3306)
+	test.S(t).ExpectEquals(dup.User, "gromit")
+	test.S(t).ExpectEquals(dup.Password, "penguin")
+}