Small changes to improve the code

Author: AMecea

pkg/sidecar/app/configs.go

@@ -43,7 +43,8 @@ const (
 
 // BaseConfig contains information related with the pod.
 type BaseConfig struct {
-	StopCh <-chan struct{}
+	// Stop represents the shutdown channel
+	Stop <-chan struct{}
 
 	// Hostname represents the pod hostname
 	Hostname string
@@ -54,81 +55,79 @@ type BaseConfig struct {
 	// ServiceName is the name of the headless service
 	ServiceName string
 
-	// NodeRole represents the MySQL role of the node, can be on of: msater, slave
-	NodeRole NodeRole
 	// ServerID represents the MySQL server id
 	ServerID int
 
 	// InitBucketURL represents the init bucket to initialize mysql
-	InitBucketURL *string
+	InitBucketURL string
 
 	// OrchestratorURL is the URL to connect to orchestrator
-	OrchestratorURL *string
-
-	// MasterHost represents the cluster master hostname
-	MasterHost string
+	OrchestratorURL string
 
 	// backup user and password for http endpoint
 	BackupUser     string
 	BackupPassword string
 }
 
-// GetHostFor returns the pod hostname for given MySQL server id
-func (cfg *BaseConfig) GetHostFor(id int) string {
+// FQDNForServer returns the pod hostname for given MySQL server id
+func (cfg *BaseConfig) FQDNForServer(id int) string {
 	base := mysqlcluster.GetNameForResource(mysqlcluster.StatefulSet, cfg.ClusterName)
 	return fmt.Sprintf("%s-%d.%s.%s", base, id-100, cfg.ServiceName, cfg.Namespace)
 }
 
-func (cfg *BaseConfig) getOrcClient() orc.Interface {
-	if cfg.OrchestratorURL == nil {
+func (cfg *BaseConfig) newOrcClient() orc.Interface {
+	if len(cfg.OrchestratorURL) == 0 {
+		log.Info("OrchestratorURL not set")
 		return nil
 	}
 
-	return orc.NewFromURI(*cfg.OrchestratorURL)
+	return orc.NewFromURI(cfg.OrchestratorURL)
 }
 
-func (cfg *BaseConfig) getFQClusterName() string {
+// ClusterFQDN returns the cluster FQ Name of the cluster from which the node belongs
+func (cfg *BaseConfig) ClusterFQDN() string {
 	return fmt.Sprintf("%s.%s", cfg.ClusterName, cfg.Namespace)
 }
 
-func (cfg *BaseConfig) getMasterHost() string {
-	if client := cfg.getOrcClient(); client != nil {
-		if master, err := client.Master(cfg.getFQClusterName()); err == nil {
+// MasterFQDN the FQ Name of the cluster's master
+func (cfg *BaseConfig) MasterFQDN() string {
+	if client := cfg.newOrcClient(); client != nil {
+		if master, err := client.Master(cfg.ClusterFQDN()); err == nil {
 			return master.Key.Hostname
 		}
 	}
 
 	log.V(-1).Info("failed to obtain master from orchestrator, go for default master",
-		"master", cfg.GetHostFor(100))
-	return cfg.GetHostFor(100)
+		"master", cfg.FQDNForServer(100))
+	return cfg.FQDNForServer(100)
 
 }
 
+// NodeRole returns the role of the current node
+func (cfg *BaseConfig) NodeRole() NodeRole {
+	if cfg.Hostname == cfg.MasterFQDN() {
+		return MasterNode
+	}
+
+	return SlaveNode
+}
+
 // NewBasicConfig returns a pointer to BaseConfig configured from environment variables
 func NewBasicConfig(stop <-chan struct{}) *BaseConfig {
 	cfg := &BaseConfig{
-		StopCh:      stop,
+		Stop:        stop,
 		Hostname:    getEnvValue("HOSTNAME"),
 		ClusterName: getEnvValue("MY_CLUSTER_NAME"),
 		Namespace:   getEnvValue("MY_NAMESPACE"),
 		ServiceName: getEnvValue("MY_SERVICE_NAME"),
 
-		InitBucketURL:   getEnvP("INIT_BUCKET_URI"),
-		OrchestratorURL: getEnvP("ORCHESTRATOR_URI"),
+		InitBucketURL:   getEnvValue("INIT_BUCKET_URI"),
+		OrchestratorURL: getEnvValue("ORCHESTRATOR_URI"),
 
 		BackupUser:     getEnvValue("MYSQL_BACKUP_USER"),
 		BackupPassword: getEnvValue("MYSQL_BACKUP_PASSWORD"),
 	}
 
-	// get master host
-	cfg.MasterHost = cfg.getMasterHost()
-
-	// set node role
-	cfg.NodeRole = SlaveNode
-	if cfg.Hostname == cfg.MasterHost {
-		cfg.NodeRole = MasterNode
-	}
-
 	// get server id
 	ordinal := getOrdinalFromHostname(cfg.Hostname)
 	cfg.ServerID = ordinal + 100
@@ -145,13 +144,6 @@ func getEnvValue(key string) string {
 	return value
 }
 
-func getEnvP(key string) *string {
-	if value := getEnvValue(key); len(value) != 0 {
-		return &value
-	}
-	return nil
-}
-
 func getOrdinalFromHostname(hn string) int {
 	// mysql-master-1
 	// or
@@ -171,7 +163,7 @@ type MysqlConfig struct {
 	// inherit from base config
 	BaseConfig
 
-	MysqlDSN *string
+	MysqlDSN string
 
 	// replication user and password
 	ReplicationUser     string
@@ -211,24 +203,24 @@ func NewMysqlConfig(cfg *BaseConfig) *MysqlConfig {
 }
 
 // getMySQLConnectionString returns the mysql DSN
-func getMySQLConnectionString() (*string, error) {
+func getMySQLConnectionString() (string, error) {
 	cnfPath := path.Join(ConfigDir, "client.cnf")
 	cfg, err := ini.Load(cnfPath)
 	if err != nil {
-		return nil, fmt.Errorf("Could not open %s: %s", cnfPath, err)
+		return "", fmt.Errorf("Could not open %s: %s", cnfPath, err)
 	}
 
 	client := cfg.Section("client")
 	host := client.Key("host").String()
 	user := client.Key("user").String()
 	password := client.Key("password").String()
 	port, err := client.Key("port").Int()
-
 	if err != nil {
-		return nil, fmt.Errorf("Invalid port in %s: %s", cnfPath, err)
+		return "", fmt.Errorf("Invalid port in %s: %s", cnfPath, err)
 	}
+
 	dsn := fmt.Sprintf("%s:%s@tcp(%s:%d)/?timeout=5s&multiStatements=true&interpolateParams=true",
 		user, password, host, port,
 	)
-	return &dsn, nil
+	return dsn, nil
 }

pkg/sidecar/app/util.go

@@ -33,12 +33,12 @@ var log = logf.Log.WithName("sidecar.app")
 
 // RunQuery executes a query
 func RunQuery(cfg *MysqlConfig, q string, args ...interface{}) error {
-	if cfg.MysqlDSN == nil {
+	if len(cfg.MysqlDSN) == 0 {
 		log.Info("could not get mysql connection DSN")
 		return fmt.Errorf("no DSN specified")
 	}
 
-	db, err := sql.Open("mysql", *cfg.MysqlDSN)
+	db, err := sql.Open("mysql", cfg.MysqlDSN)
 	if err != nil {
 		return err
 	}

pkg/sidecar/appclone/appclone.go

@@ -49,20 +49,20 @@ func RunCloneCommand(cfg *app.BaseConfig) error {
 		return fmt.Errorf("removing lost+found: %s", err)
 	}
 
-	if cfg.NodeRole == app.MasterNode {
-		if cfg.InitBucketURL == nil {
+	if cfg.NodeRole() == app.MasterNode {
+		if len(cfg.InitBucketURL) == 0 {
 			log.Info("skip cloning init bucket uri is not set.")
 			// let mysqld initialize data dir
 			return nil
 		}
-		err := cloneFromBucket(*cfg.InitBucketURL)
+		err := cloneFromBucket(cfg.InitBucketURL)
 		if err != nil {
 			return fmt.Errorf("failed to clone from bucket, err: %s", err)
 		}
 	} else {
 		// clonging from prior node
 		if cfg.ServerID > 100 {
-			sourceHost := cfg.GetHostFor(cfg.ServerID - 1)
+			sourceHost := cfg.FQDNForServer(cfg.ServerID - 1)
 			err := cloneFromSource(cfg, sourceHost)
 			if err != nil {
 				return fmt.Errorf("failed to clone from %s, err: %s", sourceHost, err)

pkg/sidecar/appconf/appconf.go

@@ -36,14 +36,14 @@ const (
 
 // RunConfigCommand generates my.cnf, client.cnf and 10-dynamic.cnf files.
 func RunConfigCommand(cfg *app.BaseConfig) error {
-	log.Info("configuring server", "host", cfg.Hostname, "role", cfg.NodeRole)
+	log.Info("configuring server", "host", cfg.Hostname, "role", cfg.NodeRole())
 
 	if err := app.CopyFile(app.MountConfigDir+"/my.cnf", app.ConfigDir+"/my.cnf"); err != nil {
 		return fmt.Errorf("copy file my.cnf: %s", err)
 	}
 
 	uPass := pkgutil.RandomString(rStrLen)
-	reportHost := cfg.GetHostFor(cfg.ServerID)
+	reportHost := cfg.FQDNForServer(cfg.ServerID)
 
 	var dynCFG, utilityCFG, clientCFG *ini.File
 	var err error

pkg/sidecar/apphelper/apphelper.go

@@ -157,7 +157,7 @@ func waitForMysqlReady(cfg *app.MysqlConfig) error {
 
 func configReadOnly(cfg *app.MysqlConfig) error {
 	var query string
-	if cfg.NodeRole == app.MasterNode {
+	if cfg.NodeRole() == app.MasterNode {
 		query = "SET GLOBAL READ_ONLY = 0"
 	} else {
 		query = "SET GLOBAL SUPER_READ_ONLY = 1"
@@ -169,7 +169,7 @@ func configReadOnly(cfg *app.MysqlConfig) error {
 }
 
 func configTopology(cfg *app.MysqlConfig) error {
-	if cfg.NodeRole == app.SlaveNode {
+	if cfg.NodeRole() == app.SlaveNode {
 		log.Info("setting up as slave")
 		if app.ShouldBootstrapNode() {
 			log.Info("doing bootstrap")
@@ -192,7 +192,7 @@ func configTopology(cfg *app.MysqlConfig) error {
 		    MASTER_CONNECT_RETRY=?;
 		`
 		if err := app.RunQuery(cfg, query,
-			cfg.MasterHost, cfg.ReplicationUser, cfg.ReplicationPassword, connRetry,
+			cfg.MasterFQDN(), cfg.ReplicationUser, cfg.ReplicationPassword, connRetry,
 		); err != nil {
 			return fmt.Errorf("failed to configure slave node, err: %s", err)
 		}

pkg/sidecar/apphelper/server.go

@@ -48,7 +48,7 @@ func newServer(cfg *app.MysqlConfig) *server {
 
 	// Shutdown gracefully the http server
 	go func() {
-		<-cfg.StopCh // wait for stop signal
+		<-cfg.Stop // wait for stop signal
 		if err := srv.Shutdown(context.Background()); err != nil {
 			log.Error(err, "failed to stop http server")