Optimize the definition and usage of ServiceRegistry (istio#4556)

Signed-off-by: PingWang <wang.ping5@zte.com.cn> modify the failures Signed-off-by: PingWang <wang.ping5@zte.com.cn> polish oncall instructions (istio#4532) * polish oncall instructions * nite gofmt Signed-off-by: PingWang <wang.ping5@zte.com.cn> gofmt Signed-off-by: PingWang <wang.ping5@zte.com.cn> gofmt Signed-off-by: PingWang <wang.ping5@zte.com.cn>
mbhavya · Mar 29, 2018 · d91916b · d91916b
1 parent bbc767f
commit d91916b
Show file tree

Hide file tree

Showing 5 changed files with 32 additions and 34 deletions.
diff --git a/pilot/cmd/pilot-agent/main.go b/pilot/cmd/pilot-agent/main.go
@@ -229,8 +229,9 @@ func timeDuration(dur *duration.Duration) time.Duration {
 func init() {
 	proxyCmd.PersistentFlags().StringVar((*string)(&registry), "serviceregistry",
 		string(serviceregistry.KubernetesRegistry),
-		fmt.Sprintf("Select the platform for service registry, options are {%s, %s, %s}",
-			serviceregistry.KubernetesRegistry, serviceregistry.ConsulRegistry, serviceregistry.EurekaRegistry))
+		fmt.Sprintf("Select the platform for service registry, options are {%s, %s, %s, %s, %s}",
+			serviceregistry.KubernetesRegistry, serviceregistry.ConsulRegistry, serviceregistry.EurekaRegistry,
+			serviceregistry.CloudFoundryRegistry, serviceregistry.MockRegistry))
 	proxyCmd.PersistentFlags().StringVar(&role.IPAddress, "ip", "",
 		"Proxy IP address. If not provided uses ${INSTANCE_IP} environment variable.")
 	proxyCmd.PersistentFlags().StringVar(&role.ID, "id", "",

diff --git a/pilot/cmd/pilot-discovery/main.go b/pilot/cmd/pilot-discovery/main.go
@@ -26,6 +26,7 @@ import (
 
 	"istio.io/istio/pilot/cmd"
 	"istio.io/istio/pilot/pkg/bootstrap"
+	"istio.io/istio/pilot/pkg/serviceregistry"
 	"istio.io/istio/pkg/collateral"
 	"istio.io/istio/pkg/log"
 	"istio.io/istio/pkg/version"
@@ -75,9 +76,10 @@ func init() {
 	discoveryCmd.PersistentFlags().BoolVar(&serverArgs.RDSv2, "rdsv2", false, "Enable RDS v2")
 
 	discoveryCmd.PersistentFlags().StringSliceVar(&serverArgs.Service.Registries, "registries",
-		[]string{string(bootstrap.KubernetesRegistry)},
+		[]string{string(serviceregistry.KubernetesRegistry)},
 		fmt.Sprintf("Comma separated list of platform service registries to read from (choose one or more from {%s, %s, %s, %s, %s})",
-			bootstrap.KubernetesRegistry, bootstrap.ConsulRegistry, bootstrap.EurekaRegistry, bootstrap.CloudFoundryRegistry, bootstrap.MockRegistry))
+			serviceregistry.KubernetesRegistry, serviceregistry.ConsulRegistry, serviceregistry.EurekaRegistry,
+			serviceregistry.CloudFoundryRegistry, serviceregistry.MockRegistry))
 	discoveryCmd.PersistentFlags().StringVar(&serverArgs.Config.CFConfig, "cfConfig", "",
 		"Cloud Foundry config file")
 	discoveryCmd.PersistentFlags().StringVar(&serverArgs.Config.ClusterRegistriesDir, "clusterRegistriesDir", "",

diff --git a/pilot/pkg/bootstrap/server.go b/pilot/pkg/bootstrap/server.go
@@ -58,20 +58,7 @@ import (
 	"istio.io/istio/pkg/version"
 )
 
-// ServiceRegistry is an expansion of the platform.ServiceRegistry enum that adds a mock registry.
-type ServiceRegistry string
-
 const (
-	// MockRegistry environment flag
-	MockRegistry ServiceRegistry = "Mock"
-	// KubernetesRegistry environment flag
-	KubernetesRegistry ServiceRegistry = "Kubernetes"
-	// ConsulRegistry environment flag
-	ConsulRegistry ServiceRegistry = "Consul"
-	// EurekaRegistry environment flag
-	EurekaRegistry ServiceRegistry = "Eureka"
-	// CloudFoundryRegistry environment flag
-	CloudFoundryRegistry ServiceRegistry = "CloudFoundry"
 	// ConfigMapKey should match the expected MeshConfig file name
 	ConfigMapKey = "mesh"
 	// CopilotTimeout when to cancel remote gRPC call to copilot
@@ -393,14 +380,19 @@ func (s *Server) getKubeCfgFile(args *PilotArgs) (kubeCfgFile string) {
 func (s *Server) initKubeClient(args *PilotArgs) error {
 	needToCreateClient := false
 	for _, r := range args.Service.Registries {
-		switch ServiceRegistry(r) {
-		case KubernetesRegistry:
+		switch serviceregistry.ServiceRegistry(r) {
+		case serviceregistry.KubernetesRegistry:
+			needToCreateClient = true
+		case serviceregistry.ConsulRegistry:
 			needToCreateClient = true
-		case ConsulRegistry:
+		case serviceregistry.EurekaRegistry:
 			needToCreateClient = true
-		case EurekaRegistry:
+		case serviceregistry.CloudFoundryRegistry:
 			needToCreateClient = true
 		}
+		if needToCreateClient {
+			break
+		}
 	}
 
 	if needToCreateClient && args.Config.FileDir == "" {
@@ -523,7 +515,7 @@ func (s *Server) createK8sServiceControllers(serviceControllers *aggregate.Contr
 	kubectl := kube.NewController(s.kubeClient, args.Config.ControllerOptions)
 	serviceControllers.AddRegistry(
 		aggregate.Registry{
-			Name:             serviceregistry.ServiceRegistry(KubernetesRegistry),
+			Name:             serviceregistry.KubernetesRegistry,
 			ServiceDiscovery: kubectl,
 			ServiceAccounts:  kubectl,
 			Controller:       kubectl,
@@ -544,7 +536,7 @@ func (s *Server) createK8sServiceControllers(serviceControllers *aggregate.Contr
 			kubectl := kube.NewController(client, args.Config.ControllerOptions)
 			serviceControllers.AddRegistry(
 				aggregate.Registry{
-					Name:             serviceregistry.ServiceRegistry(KubernetesRegistry),
+					Name:             serviceregistry.KubernetesRegistry,
 					ClusterName:      clusterregistry.GetClusterName(cluster),
 					ServiceDiscovery: kubectl,
 					ServiceAccounts:  kubectl,
@@ -558,19 +550,19 @@ func (s *Server) createK8sServiceControllers(serviceControllers *aggregate.Contr
 // initServiceControllers creates and initializes the service controllers
 func (s *Server) initServiceControllers(args *PilotArgs) error {
 	serviceControllers := aggregate.NewController()
-	registered := make(map[ServiceRegistry]bool)
+	registered := make(map[serviceregistry.ServiceRegistry]bool)
 	for _, r := range args.Service.Registries {
-		serviceRegistry := ServiceRegistry(r)
+		serviceRegistry := serviceregistry.ServiceRegistry(r)
 		if _, exists := registered[serviceRegistry]; exists {
 			log.Warnf("%s registry specified multiple times.", r)
 			continue
 		}
 		registered[serviceRegistry] = true
 		log.Infof("Adding %s registry adapter", serviceRegistry)
 		switch serviceRegistry {
-		case MockRegistry:
+		case serviceregistry.MockRegistry:
 			initMemoryRegistry(s, serviceControllers)
-		case KubernetesRegistry:
+		case serviceregistry.KubernetesRegistry:
 			if err := s.createK8sServiceControllers(serviceControllers, args); err != nil {
 				return err
 			}
@@ -597,7 +589,7 @@ func (s *Server) initServiceControllers(args *PilotArgs) error {
 					})
 				}
 			}
-		case ConsulRegistry:
+		case serviceregistry.ConsulRegistry:
 			log.Infof("Consul url: %v", args.Service.Consul.ServerURL)
 			conctl, conerr := consul.NewController(
 				args.Service.Consul.ServerURL, args.Service.Consul.Interval)
@@ -606,23 +598,23 @@ func (s *Server) initServiceControllers(args *PilotArgs) error {
 			}
 			serviceControllers.AddRegistry(
 				aggregate.Registry{
-					Name:             serviceregistry.ServiceRegistry(r),
+					Name:             serviceRegistry,
 					ServiceDiscovery: conctl,
 					ServiceAccounts:  conctl,
 					Controller:       conctl,
 				})
-		case EurekaRegistry:
+		case serviceregistry.EurekaRegistry:
 			log.Infof("Eureka url: %v", args.Service.Eureka.ServerURL)
 			eurekaClient := eureka.NewClient(args.Service.Eureka.ServerURL)
 			serviceControllers.AddRegistry(
 				aggregate.Registry{
-					Name:             serviceregistry.ServiceRegistry(r),
+					Name:             serviceRegistry,
 					Controller:       eureka.NewController(eurekaClient, args.Service.Eureka.Interval),
 					ServiceDiscovery: eureka.NewServiceDiscovery(eurekaClient),
 					ServiceAccounts:  eureka.NewServiceAccounts(),
 				})
 
-		case CloudFoundryRegistry:
+		case serviceregistry.CloudFoundryRegistry:
 			cfConfig, err := cloudfoundry.LoadConfig(args.Config.CFConfig)
 			if err != nil {
 				return multierror.Prefix(err, "loading cloud foundry config")
@@ -636,7 +628,7 @@ func (s *Server) initServiceControllers(args *PilotArgs) error {
 				return multierror.Prefix(err, "creating cloud foundry client")
 			}
 			serviceControllers.AddRegistry(aggregate.Registry{
-				Name: serviceregistry.ServiceRegistry(r),
+				Name: serviceRegistry,
 				Controller: &cloudfoundry.Controller{
 					Ticker: cloudfoundry.NewTicker(cfConfig.Copilot.PollInterval),
 					Client: client,

diff --git a/pilot/pkg/serviceregistry/platform.go b/pilot/pkg/serviceregistry/platform.go
@@ -18,6 +18,8 @@ package serviceregistry
 type ServiceRegistry string
 
 const (
+	// MockRegistry environment flag
+	MockRegistry ServiceRegistry = "Mock"
 	// KubernetesRegistry environment flag
 	KubernetesRegistry ServiceRegistry = "Kubernetes"
 	// ConsulRegistry environment flag

diff --git a/tests/util/pilotServer.go b/tests/util/pilotServer.go
@@ -23,6 +23,7 @@ import (
 	"github.com/golang/protobuf/ptypes"
 
 	"istio.io/istio/pilot/pkg/bootstrap"
+	"istio.io/istio/pilot/pkg/serviceregistry"
 
 	"bytes"
 	"runtime"
@@ -134,7 +135,7 @@ func setup() error {
 		Service: bootstrap.ServiceArgs{
 			// Using the Mock service registry, which provides the hello and world services.
 			Registries: []string{
-				string(bootstrap.MockRegistry)},
+				string(serviceregistry.MockRegistry)},
 		},
 	}
 	// Static testdata, should include all configs we want to test.