Fix unnecessary registering master and slave

cmd/stolonctl/cmd/register.go

@@ -20,11 +20,10 @@ import (
 	"syscall"
 	"time"
 
-	"github.com/sorintlab/stolon/internal/store"
-
 	"github.com/sorintlab/stolon/cmd"
 	"github.com/sorintlab/stolon/cmd/stolonctl/cmd/register"
 	slog "github.com/sorintlab/stolon/internal/log"
+	"github.com/sorintlab/stolon/internal/store"
 	"github.com/spf13/cobra"
 	"go.uber.org/zap"
 )
@@ -120,74 +119,51 @@ func registerCluster(sigs chan os.Signal, cfg *config, rCfg *register.Config) er
 }
 
 func checkAndRegisterMasterAndSlaves(store store.Store, service register.ServiceDiscovery) {
-	_, master, slaves := getMasterAndSlavesFromServiceDiscovery(cfg.ClusterName, service)
+	discoveredServices, err := service.Services(cfg.ClusterName)
+	if err != nil {
+		log.Errorf("unable to get info about existing services: %v", err)
+		return
+	}
 	cluster, err := register.NewCluster(cfg.ClusterName, store)
 	if err != nil {
 		log.Errorf("cannot get cluster data: %v", err)
 		return
+	}
+	existingServices, err := cluster.ServiceInfos()
+	if err == nil {
+		log.Debugf("found services %v", existingServices)
 	} else {
-		newMaster, err := cluster.Master()
-		if err == nil {
-			log.Debugf("found master %s with uid %s", newMaster.Name, newMaster.ID)
-		} else {
-			log.Warnf("no master found %s", err.Error())
-		}
-		newSlaves, err := cluster.Slaves()
-
-		if master == nil || newMaster == nil || (newMaster != nil && newMaster.ID != master.ID) || (len(newSlaves) != len(slaves)) {
-			deRegisterService(service, master, "master")
-			registerService(service, newMaster, "master")
-			for _, slave := range slaves {
-				deRegisterService(service, &slave, "slave")
-			}
-			for _, slave := range newSlaves {
-				registerService(service, &slave, "slave")
-			}
-		}
-		return
+		log.Warnf("%s", err.Error())
 	}
-}
 
-func getMasterAndSlavesFromServiceDiscovery(clusterName string, service register.ServiceDiscovery) (error, *register.ServiceInfo, []register.ServiceInfo) {
-	agentServices, err := service.Services()
-	if err != nil {
-		log.Errorf("unable to get services from consul, reason: %s", err.Error())
-		return err, nil, []register.ServiceInfo{}
-	}
-
-	var master *register.ServiceInfo
-	var slaves []register.ServiceInfo
-	for _, service := range agentServices {
-		if service.Service == clusterName {
-			serviceInfo := register.ServiceInfo{Name: service.Service, Tags: service.Tags, ID: service.ID}
-			if a := service.Tags[0]; a == "master" {
-				master = &serviceInfo
-			} else if service.Tags[0] == "slave" {
-				slaves = append(slaves, serviceInfo)
-			}
-		}
+	diff := existingServices.Diff(discoveredServices)
+
+	for _, removed := range diff.Removed {
+		deRegisterService(service, &removed)
+	}
+	for _, added := range diff.Added {
+		registerService(service, &added)
 	}
-	return nil, master, slaves
 }
 
-func registerService(service register.ServiceDiscovery, serviceInfo *register.ServiceInfo, name string) {
+func registerService(service register.ServiceDiscovery, serviceInfo *register.ServiceInfo) {
 	if serviceInfo == nil {
 		return
 	}
 	if err := service.Register(serviceInfo); err != nil {
-		log.Errorf("unable to register %s %s with uid %s, reason: %s", name, serviceInfo.Name, serviceInfo.ID, err.Error())
+		log.Errorf("unable to register %s with uid %s as %v, reason: %s", serviceInfo.Name, serviceInfo.ID, serviceInfo.Tags, err.Error())
 	} else {
-		log.Infof("successfully registered %s %s with uid %s", name, serviceInfo.Name, serviceInfo.ID)
+		log.Infof("successfully registered %s with uid %s as %v", serviceInfo.Name, serviceInfo.ID, serviceInfo.Tags)
 	}
 }
 
-func deRegisterService(service register.ServiceDiscovery, serviceInfo *register.ServiceInfo, name string) {
+func deRegisterService(service register.ServiceDiscovery, serviceInfo *register.ServiceInfo) {
 	if serviceInfo == nil {
 		return
 	}
 	if err := service.DeRegister(serviceInfo); err != nil {
-		log.Errorf("unable to de-register %s %s with uid %s, reason: %s", name, serviceInfo.Name, serviceInfo.ID, err.Error())
+		log.Errorf("unable to de-register %s with uid %s as %v, reason: %s", serviceInfo.Name, serviceInfo.ID, serviceInfo.Tags, err.Error())
 	} else {
-		log.Infof("successfully de-registered %s %s with uid %s", name, serviceInfo.Name, serviceInfo.ID)
+		log.Infof("successfully de-registered %s with uid %s as %v", serviceInfo.Name, serviceInfo.ID, serviceInfo.Tags)
 	}
 }

cmd/stolonctl/cmd/register/cluster.go

@@ -41,34 +41,27 @@ func NewCluster(name string, store store.Store) (*Cluster, error) {
 	return &Cluster{name: name, cd: cd}, nil
 }
 
-// Master returns the current master from the cluster data in underlying store
-func (c *Cluster) Master() (*ServiceInfo, error) {
-	if c.cd.Cluster != nil {
-		master := c.cd.Cluster.Status.Master
-		if db, ok := c.cd.DBs[master]; ok && db.Status.Healthy {
-			return NewServiceInfo(c.name, db, []string{"master"})
-		}
-	}
-	return nil, errors.New("cluster master data not available")
-}
-
-// Slaves returns all the slave information from the cluster data in underlying store
-func (c *Cluster) Slaves() ([]ServiceInfo, error) {
+// ServiceInfos returns all the service information from the cluster data in underlying store
+func (c *Cluster) ServiceInfos() (ServiceInfos, error) {
 	if c.cd.Cluster == nil {
-		return nil, errors.New("cluster slave data not available")
+		return nil, errors.New("cluster data not available")
 	}
 
-	var slaves []ServiceInfo
+	serviceInfos := ServiceInfos{}
 	master := c.cd.Cluster.Status.Master
 	for uid, db := range c.cd.DBs {
-		if uid != master && db.Status.Healthy {
-			info, err := NewServiceInfo(c.name, db, []string{"slave"})
+		if db.Status.Healthy {
+			tags := Tags{"slave"}
+			if uid == master {
+				tags = Tags{"master"}
+			}
+			info, err := NewServiceInfo(c.name, db, tags)
 			if err != nil {
 				return nil, err
 			}
-			slaves = append(slaves, *info)
+			serviceInfos[uid] = *info
 		}
 	}
 
-	return slaves, nil
+	return serviceInfos, nil
 }