Merge pull request istio#8108 from rshriram/new_release-1.0

Cherry pick select commits from release-1.0 into master

install/kubernetes/helm/istio/charts/certmanager/templates/deployment.yaml

@@ -19,6 +19,8 @@ spec:
 {{ toYaml .Values.podLabels | indent 8 }}
 {{- end }}
       annotations:
+        sidecar.istio.io/inject: "false"
+        scheduler.alpha.kubernetes.io/critical-pod: ""
       {{- if .Values.podAnnotations }}
 {{ toYaml .Values.podAnnotations | indent 8 }}
       {{- end }}

install/kubernetes/helm/istio/charts/pilot/templates/deployment.yaml

@@ -149,5 +149,6 @@ spec:
       - name: istio-certs
         secret:
           secretName: istio.istio-pilot-service-account
+          optional: true   
       affinity:
       {{- include "nodeaffinity" . | indent 6 }}

pilot/pkg/model/config.go

@@ -541,7 +541,6 @@ func ResolveShortnameToFQDN(host string, meta ConfigMeta) Hostname {
 // MostSpecificHostMatch compares the elements of the stack to the needle, and returns the longest stack element
 // matching the needle, or false if no element in the stack matches the needle.
 func MostSpecificHostMatch(needle Hostname, stack []Hostname) (Hostname, bool) {
-	sort.Sort(Hostnames(stack))
 	for _, h := range stack {
 		if needle.Matches(h) {
 			return h, true

pilot/pkg/model/config_test.go

@@ -584,10 +584,11 @@ func TestMostSpecificHostMatch(t *testing.T) {
 		needle model.Hostname
 		want   model.Hostname
 	}{
+		// this has to be a sorted list
 		{[]model.Hostname{}, "*", ""},
-		{[]model.Hostname{"*.com", "*.foo.com"}, "bar.foo.com", "*.foo.com"},
-		{[]model.Hostname{"*.com", "*.foo.com"}, "foo.com", "*.com"},
-		{[]model.Hostname{"*.com", "foo.com"}, "*.foo.com", "*.com"},
+		{[]model.Hostname{"*.foo.com", "*.com"}, "bar.foo.com", "*.foo.com"},
+		{[]model.Hostname{"*.foo.com", "*.com"}, "foo.com", "*.com"},
+		{[]model.Hostname{"foo.com", "*.com"}, "*.foo.com", "*.com"},
 
 		{[]model.Hostname{"*.foo.com", "foo.com"}, "foo.com", "foo.com"},
 		{[]model.Hostname{"*.foo.com", "foo.com"}, "*.foo.com", "*.foo.com"},

pilot/pkg/model/push_context.go

@@ -16,6 +16,8 @@ package model
 
 import (
 	"encoding/json"
+	"fmt"
+	"sort"
 	"sync"
 	"time"
 
@@ -73,7 +75,7 @@ type PushContext struct {
 	VirtualServiceConfigs []Config `json:"-,omitempty"`
 
 	destinationRuleHosts   []Hostname
-	destinationRuleByHosts map[Hostname]*Config
+	destinationRuleByHosts map[Hostname]*combinedDestinationRule
 
 	//TODO: gateways              []*networking.Gateway
 
@@ -96,6 +98,12 @@ type PushMetric struct {
 	gauge prometheus.Gauge
 }
 
+type combinedDestinationRule struct {
+	subsets map[string]bool // list of subsets seen so far
+	// We are not doing ports
+	config *Config
+}
+
 func newPushMetric(name, help string) *PushMetric {
 	pm := &PushMetric{
 		gauge: prometheus.NewGauge(prometheus.GaugeOpts{
@@ -189,6 +197,12 @@ var (
 		"Virtual services with dup domains.",
 	)
 
+	// DuplicatedSubsets tracks duplicate subsets that we rejected while merging multiple destination rules for same host
+	DuplicatedSubsets = newPushMetric(
+		"pilot_destrule_subsets",
+		"Duplicate subsets across destination rules for same host",
+	)
+
 	// LastPushStatus preserves the metrics and data collected during lasts global push.
 	// It can be used by debugging tools to inspect the push event. It will be reset after each push with the
 	// new version.
@@ -306,13 +320,22 @@ func (ps *PushContext) initServiceRegistry(env *Environment) error {
 	if err != nil {
 		return err
 	}
-	ps.Services = services
+	// Sort the services in order of creation.
+	ps.Services = sortServicesByCreationTime(services)
 	for _, s := range services {
 		ps.ServiceByHostname[s.Hostname] = s
 	}
 	return nil
 }
 
+// sortServicesByCreationTime sorts the list of services in ascending order by their creation time (if available).
+func sortServicesByCreationTime(services []*Service) []*Service {
+	sort.SliceStable(services, func(i, j int) bool {
+		return services[i].CreationTime.Before(services[j].CreationTime)
+	})
+	return services
+}
+
 // Caches list of virtual services
 func (ps *PushContext) initVirtualServices(env *Environment) error {
 	vservices, err := env.List(VirtualService.Type, NamespaceAll)
@@ -395,23 +418,58 @@ func (ps *PushContext) initDestinationRules(env *Environment) error {
 // Split out of DestinationRule expensive conversions, computed once per push.
 // This also allows tests to inject a config without having the mock.
 func (ps *PushContext) SetDestinationRules(configs []Config) {
+	// Sort by time first. So if two destination rule have top level traffic policies
+	// we take the first one.
 	sortConfigByCreationTime(configs)
-	hosts := make([]Hostname, len(configs))
-	byHosts := make(map[Hostname]*Config, len(configs))
+	hosts := make([]Hostname, 0)
+	combinedDestinationRuleMap := make(map[Hostname]*combinedDestinationRule, len(configs))
+
 	for i := range configs {
 		rule := configs[i].Spec.(*networking.DestinationRule)
-		hosts[i] = ResolveShortnameToFQDN(rule.Host, configs[i].ConfigMeta)
-		byHosts[hosts[i]] = &configs[i]
+		resolvedHost := ResolveShortnameToFQDN(rule.Host, configs[i].ConfigMeta)
+		if mdr, exists := combinedDestinationRuleMap[resolvedHost]; exists {
+			combinedRule := mdr.config.Spec.(*networking.DestinationRule)
+			// we have an another destination rule for same host.
+			// concatenate both of them -- essentially add subsets from one to other.
+			for _, subset := range rule.Subsets {
+				if _, subsetExists := mdr.subsets[subset.Name]; !subsetExists {
+					mdr.subsets[subset.Name] = true
+					combinedRule.Subsets = append(combinedRule.Subsets, subset)
+				} else {
+					ps.Add(DuplicatedSubsets, string(resolvedHost), nil,
+						fmt.Sprintf("Duplicate subset %s found while merging destination rules for %s",
+							subset.Name, string(resolvedHost)))
+				}
+
+				// If there is no top level policy and the incoming rule has top level
+				// traffic policy, use the one from the incoming rule.
+				if combinedRule.TrafficPolicy == nil && rule.TrafficPolicy != nil {
+					combinedRule.TrafficPolicy = rule.TrafficPolicy
+				}
+			}
+			continue
+		}
+
+		combinedDestinationRuleMap[resolvedHost] = &combinedDestinationRule{
+			subsets: make(map[string]bool),
+			config:  &configs[i],
+		}
+		for _, subset := range rule.Subsets {
+			combinedDestinationRuleMap[resolvedHost].subsets[subset.Name] = true
+		}
+		hosts = append(hosts, resolvedHost)
 	}
 
+	// presort it so that we don't sort it for each DestinationRule call.
+	sort.Sort(Hostnames(hosts))
 	ps.destinationRuleHosts = hosts
-	ps.destinationRuleByHosts = byHosts
+	ps.destinationRuleByHosts = combinedDestinationRuleMap
 }
 
 // DestinationRule returns a destination rule for a service name in a given domain.
 func (ps *PushContext) DestinationRule(hostname Hostname) *Config {
 	if c, ok := MostSpecificHostMatch(hostname, ps.destinationRuleHosts); ok {
-		return ps.destinationRuleByHosts[c]
+		return ps.destinationRuleByHosts[c].config
 	}
 	return nil
 }

pilot/pkg/networking/core/v1alpha3/listener.go

@@ -341,14 +341,6 @@ type listenerEntry struct {
 	listener    *xdsapi.Listener
 }
 
-// sortServicesByCreationTime sorts the list of services in ascending order by their creation time (if available).
-func sortServicesByCreationTime(services []*model.Service) []*model.Service {
-	sort.SliceStable(services, func(i, j int) bool {
-		return services[i].CreationTime.Before(services[j].CreationTime)
-	})
-	return services
-}
-
 func protocolName(p model.Protocol) string {
 	switch plugin.ModelProtocolToListenerProtocol(p) {
 	case plugin.ListenerProtocolHTTP:
@@ -407,9 +399,6 @@ func (c outboundListenerConflict) addMetric(push *model.PushContext) {
 func (configgen *ConfigGeneratorImpl) buildSidecarOutboundListeners(env *model.Environment, node *model.Proxy, push *model.PushContext,
 	proxyInstances []*model.ServiceInstance, services []*model.Service) []*xdsapi.Listener {
 
-	// Sort the services in order of creation.
-	services = sortServicesByCreationTime(services)
-
 	var proxyLabels model.LabelsCollection
 	for _, w := range proxyInstances {
 		proxyLabels = append(proxyLabels, w.Labels)

pilot/pkg/networking/core/v1alpha3/listener_test.go

@@ -155,7 +155,11 @@ func getOldestService(services ...*model.Service) *model.Service {
 func buildOutboundListeners(p plugin.Plugin, services ...*model.Service) []*xdsapi.Listener {
 	configgen := NewConfigGenerator([]plugin.Plugin{p})
 
-	env := buildListenerEnv()
+	env := buildListenerEnv(services)
+
+	if err := env.PushContext.InitContext(&env); err != nil {
+		return nil
+	}
 
 	instances := make([]*model.ServiceInstance, len(services))
 	for i, s := range services {
@@ -221,8 +225,9 @@ func buildService(hostname string, ip string, protocol model.Protocol, creationT
 	}
 }
 
-func buildListenerEnv() model.Environment {
-	serviceDiscovery := &fakes.ServiceDiscovery{}
+func buildListenerEnv(services []*model.Service) model.Environment {
+	serviceDiscovery := new(fakes.ServiceDiscovery)
+	serviceDiscovery.ServicesReturns(services, nil)
 
 	configStore := &fakes.IstioConfigStore{}
 

tests/e2e/tests/pilot/routing_test.go

@@ -196,10 +196,11 @@ func TestRoutes(t *testing.T) {
 	}
 
 	t.Run("v1alpha3", func(t *testing.T) {
-		destRule := maybeAddTLSForDestinationRule(tc, "testdata/networking/v1alpha3/destination-rule-c.yaml")
+		destRule1 := maybeAddTLSForDestinationRule(tc, "testdata/networking/v1alpha3/destination-rule-c.yaml")
+		destRule2 := "testdata/networking/v1alpha3/destination-rule-c-headersubset.yaml"
 		cfgs := &deployableConfig{
 			Namespace:  tc.Kube.Namespace,
-			YamlFiles:  []string{destRule},
+			YamlFiles:  []string{destRule1, destRule2},
 			kubeconfig: tc.Kube.KubeConfig,
 		}
 		if err := cfgs.Setup(); err != nil {

tests/e2e/tests/pilot/testdata/networking/v1alpha3/destination-rule-c-headersubset.yaml

@@ -0,0 +1,11 @@
+# Also an example of defining subsets for same host in different files
+apiVersion: networking.istio.io/v1alpha3
+kind: DestinationRule
+metadata:
+  name: destination-rule-c-headersubset
+spec:
+  host: c
+  subsets:
+    - name: headersubset
+      labels:
+        version: v2

tests/e2e/tests/pilot/testdata/networking/v1alpha3/rule-content-route.yaml

@@ -15,7 +15,7 @@ spec:
       route:
       - destination:
           host: c
-          subset: v2
+          subset: headersubset
         weight: 100
     - route:
       - destination: