Update comments (TODOs), fix queries and rebase issues

Author: AMecea

pkg/controller/mysqlcluster/internal/syncer/secret.go

@@ -31,7 +31,7 @@ import (
 
 // NewSecretSyncer returns secret syncer
 // nolint: gocyclo
-// TODO: this syncer is not needed anymore and can be removed in future version
+// TODO: this syncer is not needed anymore and can be removed in future version (v0.4)
 func NewSecretSyncer(c client.Client, scheme *runtime.Scheme, cluster *mysqlcluster.MysqlCluster, opt *options.Options) syncer.Interface {
 	obj := &core.Secret{
 		ObjectMeta: metav1.ObjectMeta{

pkg/controller/mysqlcluster/internal/syncer/statefullset.go

@@ -281,8 +281,7 @@ func (s *sfsSyncer) ensureContainersSpec() []core.Container {
 				"mysql",
 				fmt.Sprintf("--defaults-file=%s", confClientPath),
 				"-e",
-				"SELECT 1", // TODO: ...
-				// fmt.Sprintf("SELECT * FROM %s.%s", constants.OperatorDbName, "readiness"),
+				fmt.Sprintf("SELECT * FROM %s.%s", constants.OperatorDbName, constants.OperatorReadinessTableName),
 			},
 		},
 	})

pkg/controller/node/node_controller.go

@@ -72,7 +72,6 @@ func isOwnedByMySQL(meta metav1.Object) bool {
 		return false
 	}
 
-	// TODO: add more checks here
 	labels := meta.GetLabels()
 	if val, ok := labels["app.kubernetes.io/managed-by"]; ok {
 		return val == "mysql.presslabs.org"
@@ -106,7 +105,6 @@ func add(mgr manager.Manager, r reconcile.Reconciler) error {
 			return isOwnedByMySQL(evt.Meta) && !isInitialized(evt.Object)
 		},
 		UpdateFunc: func(evt event.UpdateEvent) bool {
-			log.V(1).Info("pod update event", "meta", evt.MetaNew)
 			return isOwnedByMySQL(evt.MetaNew) && !isInitialized(evt.ObjectNew)
 		},
 		DeleteFunc: func(evt event.DeleteEvent) bool {

pkg/controller/node/sql.go

@@ -80,7 +80,7 @@ func (r *nodeSQLRunner) Wait(ctx context.Context) error {
 
 func (r *nodeSQLRunner) DisableSuperReadOnly(ctx context.Context) (func(), error) {
 	enable := func() {
-		err := r.runQuery(ctx, "SET GLOBAL SUPER_READ_ONLY = 0;")
+		err := r.runQuery(ctx, "SET GLOBAL SUPER_READ_ONLY = 1;")
 		if err != nil {
 			log.Error(err, "failed to set node super read only", "node", r.Host())
 		}
@@ -123,6 +123,7 @@ func (r *nodeSQLRunner) ChangeMasterTo(ctx context.Context, masterHost, user, pa
 	return nil
 }
 
+// MarkConfigurationDone write in a MEMORY table value. The readiness probe checks for that value to exist to succeed.
 func (r *nodeSQLRunner) MarkConfigurationDone(ctx context.Context) error {
 	query := `
     CREATE TABLE IF NOT EXISTS %s.%s (
@@ -131,7 +132,7 @@ func (r *nodeSQLRunner) MarkConfigurationDone(ctx context.Context) error {
 
     INSERT INTO %[1]s.%[2]s VALUES (1);
     `
-	query = fmt.Sprintf(query, constants.OperatorDbName, "readiness")
+	query = fmt.Sprintf(query, constants.OperatorDbName, constants.OperatorReadinessTableName)
 
 	if err := r.runQuery(ctx, query); err != nil {
 		return fmt.Errorf("failed to mark configuration done, err: %s", err)
@@ -210,10 +211,11 @@ func (r *nodeSQLRunner) SetPurgedGTID(ctx context.Context) error {
 	var used bool
 	if err := r.readFromMysql(ctx, qq, &used); err != nil {
 		// if it's a: "Table doesn't exist" error then GTID should not be set, it's a master case.
-		if isMySQLError(err, 1146) {
+		if isMySQLError(err, 1146) || err == sql.ErrNoRows {
 			log.V(1).Info("GTID purged table does not exists", "host", r.Host())
 			return nil
 		}
+
 		return err
 	}
 

pkg/controller/orchestrator/orchestrator_reconcile.go

@@ -391,7 +391,7 @@ func (ou *orcUpdater) updateNodeCondition(host string, cType api.NodeConditionTy
 }
 
 // removeNodeConditionNotInOrc marks nodes not in orc with unknown condition
-// TODO: this function should remove completly from cluster.Status.Nodes nodes
+// TODO: this function should remove completely from cluster.Status.Nodes nodes
 // that are no longer in orchestrator and in k8s
 func (ou *orcUpdater) removeNodeConditionNotInOrc(insts InstancesSet) {
 	for _, ns := range ou.cluster.Status.Nodes {

pkg/controller/orchestrator/orchestrator_syncers.go

@@ -35,7 +35,7 @@ func newFinalizerSyncer(c client.Client, scheme *runtime.Scheme, cluster *mysqlc
 
 		// always add finalizer, this action is idempotent
 		addFinalizer(out, OrchestratorFinalizer)
-		// TODO: remove this in next version
+		// TODO: remove this in next version (v0.4)
 		removeFinalizer(out, OldOrchestratorFinalizer)
 
 		// if cluster is deleted then check orchestrator status and remove finalizer if no node is in orchestrator

pkg/internal/mysqlcluster/mysqlcluster.go

@@ -188,7 +188,7 @@ func (c *MysqlCluster) GetMysqlImage() string {
 
 // UpdateSpec updates the cluster specs that need to be saved
 func (c *MysqlCluster) UpdateSpec() {
-	// TODO: remove this in next major release
+	// TODO: remove this in next major release (v0.4)
 	if len(c.Spec.InitBucketURL) == 0 {
 		c.Spec.InitBucketURL = c.Spec.InitBucketURI
 	}

pkg/sidecar/appconf.go

@@ -151,7 +151,7 @@ func initFileQuery(cfg *Config, gtidPurged string) []byte {
 	}
 
 	// create operator database because GRANTS need it
-	queries = append(queries, fmt.Sprintf("CREATE DATABASE IF NOT EXISTS %s;", toolsDbName))
+	queries = append(queries, fmt.Sprintf("CREATE DATABASE IF NOT EXISTS %s", toolsDbName))
 
 	// configure operator utility user
 	queries = append(queries, createUserQuery(cfg.OperatorUser, cfg.OperatorPassword, "%",
@@ -190,11 +190,11 @@ func initFileQuery(cfg *Config, gtidPurged string) []byte {
 		)`, constants.OperatorDbName, constants.OperatorGtidsTableName))
 
 		// nolint: gosec
-		queries = append(queries, fmt.Sprintf(`REPLACE INTO %s.%s VALUES (1, '%s')`,
+		queries = append(queries, fmt.Sprintf(`REPLACE INTO %s.%s (id, gtid) VALUES (1, '%s')`,
 			constants.OperatorDbName, constants.OperatorGtidsTableName, gtidPurged))
 	}
 
-	return []byte(strings.Join(queries, ";\n"))
+	return []byte(strings.Join(queries, ";\n") + ";\n")
 }
 
 func createUserQuery(name, pass, host string, rights ...interface{}) []string {

pkg/util/constants/constants.go

@@ -44,6 +44,9 @@ const (
 	// OperatorGtidsTableName represents the name of the table that is used to store the GTID
 	OperatorGtidsTableName = "gtids"
 
+	// OperatorReadinessTableName is the name of the table that is used to store a readiness flag (boolean)
+	OperatorReadinessTableName = "readiness"
+
 	// ConfVolumeMountPath is the path where mysql configs will be mounted
 	ConfVolumeMountPath = "/etc/mysql"