Skip to content

Commit

Permalink
Revert "Location based Load Balancing (istio#10720)" (istio#11371)
Browse files Browse the repository at this point in the history
This reverts commit 3f05706.
  • Loading branch information
rshriram authored and louiscryan committed Feb 14, 2019
1 parent 838c6f7 commit 9b48b45
Show file tree
Hide file tree
Showing 30 changed files with 86 additions and 1,271 deletions.
4 changes: 0 additions & 4 deletions istioctl/cmd/istioctl/kubeinject.go
Original file line number Diff line number Diff line change
Expand Up @@ -254,10 +254,6 @@ istioctl kube-inject -f deployment.yaml -o deployment-injected.yaml --injectConf
return err
}
}
err = model.ValidateMeshConfig(meshConfig)
if err != nil {
return err
}

var sidecarTemplate string

Expand Down
3 changes: 0 additions & 3 deletions mixer/test/client/gateway/gateway_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -223,9 +223,6 @@ func (mock) ID(*core.Node) string {
func (mock) GetProxyServiceInstances(_ *model.Proxy) ([]*model.ServiceInstance, error) {
return nil, nil
}
func (mock) GetProxyLocality(_ *model.Proxy) string {
return ""
}
func (mock) GetService(_ model.Hostname) (*model.Service, error) { return nil, nil }
func (mock) InstancesByPort(_ model.Hostname, _ int, _ model.LabelsCollection) ([]*model.ServiceInstance, error) {
return nil, nil
Expand Down
3 changes: 0 additions & 3 deletions mixer/test/client/pilotplugin/pilotplugin_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -297,9 +297,6 @@ func (mock) ID(*core.Node) string {
func (mock) GetProxyServiceInstances(_ *model.Proxy) ([]*model.ServiceInstance, error) {
return nil, nil
}
func (mock) GetProxyLocality(_ *model.Proxy) string {
return ""
}
func (mock) GetService(_ model.Hostname) (*model.Service, error) { return nil, nil }
func (mock) InstancesByPort(_ model.Hostname, _ int, _ model.LabelsCollection) ([]*model.ServiceInstance, error) {
return nil, nil
Expand Down
3 changes: 0 additions & 3 deletions mixer/test/client/pilotplugin_mtls/pilotplugin_mtls_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -312,9 +312,6 @@ func (mock) ID(*core.Node) string {
func (mock) GetProxyServiceInstances(_ *model.Proxy) ([]*model.ServiceInstance, error) {
return nil, nil
}
func (mock) GetProxyLocality(_ *model.Proxy) string {
return ""
}
func (mock) GetService(_ model.Hostname) (*model.Service, error) { return nil, nil }
func (mock) InstancesByPort(_ model.Hostname, _ int, _ model.LabelsCollection) ([]*model.ServiceInstance, error) {
return nil, nil
Expand Down
3 changes: 0 additions & 3 deletions mixer/test/client/pilotplugin_tcp/pilotplugin_tcp_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -174,9 +174,6 @@ func (mock) ID(*core.Node) string {
func (mock) GetProxyServiceInstances(_ *model.Proxy) ([]*model.ServiceInstance, error) {
return nil, nil
}
func (mock) GetProxyLocality(_ *model.Proxy) string {
return ""
}
func (mock) GetService(_ model.Hostname) (*model.Service, error) { return nil, nil }
func (mock) InstancesByPort(_ model.Hostname, _ int, _ model.LabelsCollection) ([]*model.ServiceInstance, error) {
return nil, nil
Expand Down
5 changes: 0 additions & 5 deletions pilot/pkg/bootstrap/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -413,11 +413,6 @@ func (s *Server) initMesh(args *PilotArgs) error {
}
}

if err = model.ValidateMeshConfig(mesh); err != nil {
log.Errorf("invalid mesh configuration: %v", err)
return err
}

log.Infof("mesh configuration %s", spew.Sdump(mesh))
log.Infof("version %s", version.Info.String())
log.Infof("flags %s", spew.Sdump(args))
Expand Down
5 changes: 0 additions & 5 deletions pilot/pkg/kube/inject/webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@ import (

meshconfig "istio.io/api/mesh/v1alpha1"
"istio.io/istio/pilot/cmd"
"istio.io/istio/pilot/pkg/model"
"istio.io/istio/pkg/log"
)

Expand Down Expand Up @@ -88,10 +87,6 @@ func loadConfig(injectFile, meshFile string) (*Config, *meshconfig.MeshConfig, e
if err != nil {
return nil, nil, err
}
err = model.ValidateMeshConfig(meshConfig)
if err != nil {
return nil, nil, err
}

log.Infof("New configuration: sha256sum %x", sha256.Sum256(data))
log.Infof("Policy: %v", c.Policy)
Expand Down
17 changes: 0 additions & 17 deletions pilot/pkg/model/context.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ import (
"strings"
"time"

"github.com/envoyproxy/go-control-plane/envoy/api/v2/core"
"github.com/gogo/protobuf/types"
multierror "github.com/hashicorp/go-multierror"

Expand Down Expand Up @@ -87,9 +86,6 @@ type Proxy struct {
// namespace.
ID string

// Locality is the location of where Envoy proxy runs.
Locality Locality

// DNSDomain defines the DNS domain suffix for short hostnames (e.g.
// "default.svc.cluster.local")
DNSDomain string
Expand Down Expand Up @@ -286,19 +282,6 @@ func GetProxyConfigNamespace(proxy *Proxy) string {
return ""
}

// GetProxyLocality returns the locality where Envoy proxy is running.
func GetProxyLocality(proxy *core.Node) *Locality {
if proxy == nil || proxy.Locality == nil {
return nil
}

return &Locality{
Region: proxy.Locality.Region,
Zone: proxy.Locality.Zone,
SubZone: proxy.Locality.SubZone,
}
}

const (
serviceNodeSeparator = "~"

Expand Down
61 changes: 0 additions & 61 deletions pilot/pkg/model/locality.go

This file was deleted.

11 changes: 0 additions & 11 deletions pilot/pkg/model/push_context.go
Original file line number Diff line number Diff line change
Expand Up @@ -494,17 +494,6 @@ func (ps *PushContext) GetAllSidecarScopes() map[string][]*SidecarScope {
return ps.sidecarsByNamespace
}

// ServicePort returns the port model for the given service and port.
func (ps *PushContext) ServicePort(hostname Hostname, port int) *Port {
portList := ps.ServicePort2Name[string(hostname)]
for i := range portList {
if portList[i] != nil && portList[i].Port == port {
return portList[i]
}
}
return nil
}

// DestinationRule returns a destination rule for a service name in a given domain.
func (ps *PushContext) DestinationRule(proxy *Proxy, service *Service) *Config {
// If proxy has a sidecar scope that is user supplied, then get the destination rules from the sidecar scope
Expand Down
11 changes: 4 additions & 7 deletions pilot/pkg/model/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -500,9 +500,6 @@ type ServiceDiscovery interface {
// determine the intended destination of a connection without a Host header on the request.
GetProxyServiceInstances(*Proxy) ([]*ServiceInstance, error)

// GetProxyLocality returns the locality where the proxy runs.
GetProxyLocality(*Proxy) string

// ManagementPorts lists set of management ports associated with an IPv4 address.
// These management ports are typically used by the platform for out of band management
// tasks such as health checks, etc. In a scenario where the proxy functions in the
Expand Down Expand Up @@ -542,12 +539,12 @@ func (h Hostname) Matches(o Hostname) bool {
return true
}

hWildcard := len(h) > 0 && string(h[0]) == "*"
hWildcard := string(h[0]) == "*"
if hWildcard && len(o) == 0 {
return true
}

oWildcard := len(o) > 0 && string(o[0]) == "*"
oWildcard := string(o[0]) == "*"
if !hWildcard && !oWildcard {
// both are non-wildcards, so do normal string comparison
return h == o
Expand Down Expand Up @@ -581,8 +578,8 @@ func (h Hostname) SubsetOf(o Hostname) bool {
return true
}

hWildcard := len(h) > 0 && string(h[0]) == "*"
oWildcard := len(o) > 0 && string(o[0]) == "*"
hWildcard := string(h[0]) == "*"
oWildcard := string(o[0]) == "*"
if !oWildcard {
if hWildcard {
return false
Expand Down
2 changes: 0 additions & 2 deletions pilot/pkg/model/service_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -228,8 +228,6 @@ func TestHostnameMatches(t *testing.T) {
out bool
}{
{"empty", "", "", true},
{"first empty", "", "foo.com", false},
{"second empty", "foo.com", "", false},

{"non-wildcard domain",
"foo.com", "foo.com", true},
Expand Down
113 changes: 1 addition & 112 deletions pilot/pkg/model/validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -864,6 +864,7 @@ func validateLoadBalancer(settings *networking.LoadBalancerSettings) (errs error
}
}
}

return
}

Expand Down Expand Up @@ -1083,10 +1084,6 @@ func ValidateMeshConfig(mesh *meshconfig.MeshConfig) (errs error) {
errs = multierror.Append(errs, err)
}

if err := validateLocalityLbSetting(mesh.LocalityLbSetting); err != nil {
errs = multierror.Append(errs, err)
}

return
}

Expand Down Expand Up @@ -2275,111 +2272,3 @@ func ValidateNetworkEndpointAddress(n *NetworkEndpoint) error {
}
return nil
}

// validateLocalityLbSetting checks the LocalityLbSetting of MeshConfig
func validateLocalityLbSetting(lb *meshconfig.LocalityLoadBalancerSetting) error {
if lb == nil {
return nil
}

if len(lb.GetDistribute()) > 0 && len(lb.GetFailover()) > 0 {
return fmt.Errorf("can not simultaneously specify 'distribute' and 'failover'")
}

srcLocalities := []string{}
for _, locality := range lb.GetDistribute() {
srcLocalities = append(srcLocalities, locality.From)
var totalWeight uint32
destLocalities := []string{}
for loc, weight := range locality.To {
destLocalities = append(destLocalities, loc)
if weight == 0 {
return fmt.Errorf("locality weight must not be in range [1, 100]")
}
totalWeight += weight
}
if totalWeight != 100 {
return fmt.Errorf("total locality weight %v != 100", totalWeight)
}
if err := validateLocalities(destLocalities); err != nil {
return err
}
}

if err := validateLocalities(srcLocalities); err != nil {
return err
}

for _, failover := range lb.GetFailover() {
if failover.From == failover.To {
return fmt.Errorf("locality lb failover settings must specify different regions")
}
if strings.Contains(failover.To, "*") {
return fmt.Errorf("locality lb failover region should not contain '*' wildcard")
}
}

return nil
}

func validateLocalities(localities []string) error {
regionZoneSubZoneMap := map[string]map[string]map[string]bool{}

for _, locality := range localities {
if n := strings.Count(locality, "*"); n > 0 {
if n > 1 || !strings.HasSuffix(locality, "*") {
return fmt.Errorf("locality %s wildcard '*' number can not exceed 1 and must be in the end", locality)
}
}

items := strings.SplitN(locality, "/", 3)
for _, item := range items {
if item == "" {
return fmt.Errorf("locality %s must not contain empty region/zone/subzone info", locality)
}
}
if _, ok := regionZoneSubZoneMap["*"]; ok {
return fmt.Errorf("locality %s overlap with previous specified ones", locality)
}
switch len(items) {
case 1:
if _, ok := regionZoneSubZoneMap[items[0]]; ok {
return fmt.Errorf("locality %s overlap with previous specified ones", locality)
}
regionZoneSubZoneMap[items[0]] = map[string]map[string]bool{"*": {"*": true}}
case 2:
if _, ok := regionZoneSubZoneMap[items[0]]; ok {
if _, ok := regionZoneSubZoneMap[items[0]]["*"]; ok {
return fmt.Errorf("locality %s overlap with previous specified ones", locality)
}
if _, ok := regionZoneSubZoneMap[items[0]][items[1]]; ok {
return fmt.Errorf("locality %s overlap with previous specified ones", locality)
}
regionZoneSubZoneMap[items[0]][items[1]] = map[string]bool{"*": true}
} else {
regionZoneSubZoneMap[items[0]] = map[string]map[string]bool{items[1]: {"*": true}}
}
case 3:
if _, ok := regionZoneSubZoneMap[items[0]]; ok {
if _, ok := regionZoneSubZoneMap[items[0]]["*"]; ok {
return fmt.Errorf("locality %s overlap with previous specified ones", locality)
}
if _, ok := regionZoneSubZoneMap[items[0]][items[1]]; ok {
if regionZoneSubZoneMap[items[0]][items[1]]["*"] {
return fmt.Errorf("locality %s overlap with previous specified ones", locality)
}
if regionZoneSubZoneMap[items[0]][items[1]][items[2]] {
return fmt.Errorf("locality %s overlap with previous specified ones", locality)
}
regionZoneSubZoneMap[items[0]][items[1]][items[2]] = true
} else {
regionZoneSubZoneMap[items[0]][items[1]] = map[string]bool{items[2]: true}
}
} else {
regionZoneSubZoneMap[items[0]] = map[string]map[string]bool{items[1]: {items[2]: true}}
}
}
}

return nil
}
Loading

0 comments on commit 9b48b45

Please sign in to comment.