Repository: istio/istio (go)

Issue:
concurrent map read and map write in GetServiceAddressForProxy
https://storage.googleapis.com/istio-prow/logs/integ-pilot-multicluster-tests_istio_postsubmit/1367553922817855488/artifacts/pilot-26c62a9f7f924718bb0375036/_suite_context/istio-state-831956234/primary/istiod-85d7db58dc-gtbxr_discovery.previous.log

Broken by https://github.com/istio/istio/commit/a0b66178103fd0fd114d81ddec37905a12db0d5d and/or https://github.com/istio/istio/commit/6c9315b1c58a8310ea7a8a681c3424a619e62f19 I assume

Pull request description:
fix race cluster vip mutex in service (#31204)"
Fix #31247

Patch:
diff --git a/go.mod b/go.mod
index 7cddb7c71d..1424367d0b 100644
--- a/go.mod
+++ b/go.mod
@@ -99,7 +99,7 @@ require (
 	gopkg.in/yaml.v3 v3.0.0-20210107192922-496545a6307b
 	helm.sh/helm/v3 v3.4.2
 	honnef.co/go/tools v0.0.1-2020.1.5 // indirect
-	istio.io/api v0.0.0-20210302211031-2e1e4d7e6f4b
+	istio.io/api v0.0.0-20210305035442-003b5cf6f68d
 	istio.io/client-go v0.0.0-20210218000043-b598dd019200
 	istio.io/gogo-genproto v0.0.0-20210226185354-42d28d740a8c
 	istio.io/pkg v0.0.0-20210226185257-1f58c1049e9f
diff --git a/go.sum b/go.sum
index f31cc1dde5..74488d4bd7 100644
--- a/go.sum
+++ b/go.sum
@@ -1471,8 +1471,8 @@ honnef.co/go/tools v0.0.1-2020.1.4/go.mod h1:X/FiERA/W4tHapMX5mGpAtMSVEeEUOyHaw9
 honnef.co/go/tools v0.0.1-2020.1.5 h1:nI5egYTGJakVyOryqLs1cQO5dO0ksin5XXs2pspk75k=
 honnef.co/go/tools v0.0.1-2020.1.5/go.mod h1:X/FiERA/W4tHapMX5mGpAtMSVEeEUOyHaw9vFzvIQ3k=
 istio.io/api v0.0.0-20210131044048-bfeb10697307/go.mod h1:fnENIXK5mEYv7Acq1AE8bSEvba9kG2dltqEzsaCw7ys=
-istio.io/api v0.0.0-20210302211031-2e1e4d7e6f4b h1:FLyq/9lqcIhGh69iICahWOKJGJezT/HBVxoG+y2H4dc=
-istio.io/api v0.0.0-20210302211031-2e1e4d7e6f4b/go.mod h1:nsSFw1LIMmGL7r/+6fJI6FxeG/UGlLxRK8bkojIvBVs=
+istio.io/api v0.0.0-20210305035442-003b5cf6f68d h1:ApLqckaE/XH3dWx7jtbtKxYD3b0NTH0UsLigFlkVflg=
+istio.io/api v0.0.0-20210305035442-003b5cf6f68d/go.mod h1:nsSFw1LIMmGL7r/+6fJI6FxeG/UGlLxRK8bkojIvBVs=
 istio.io/client-go v0.0.0-20210218000043-b598dd019200 h1:9M2ZxIeSilnH03pFVovgxbm2RFL/J5mASQzlbHAvgJs=
 istio.io/client-go v0.0.0-20210218000043-b598dd019200/go.mod h1:gmwBYzvw0UEaguyZG6E9mYv4HXGr1V9MChouM0tj5KQ=
 istio.io/gogo-genproto v0.0.0-20210113155706-4daf5697332f/go.mod h1:6BwTZRNbWS570wHX/uR1Wqk5e0157TofTAUMzT7N4+s=
diff --git a/istio.deps b/istio.deps
index 62b68dfb4f..556610af3d 100644
--- a/istio.deps
+++ b/istio.deps
@@ -4,6 +4,6 @@
     "name": "PROXY_REPO_SHA",
     "repoName": "proxy",
     "file": "",
-    "lastStableSHA": "669f1859bca1348cae2a7ce93a817b1e31eb47e8"
+    "lastStableSHA": "32a906d56b93b815143597a1cbdfa1d2921ef8a5"
   }
 ]
diff --git a/pilot/pkg/model/service.go b/pilot/pkg/model/service.go
index 681f75bb74..3e92a0169d 100644
--- a/pilot/pkg/model/service.go
+++ b/pilot/pkg/model/service.go
@@ -31,7 +31,6 @@ import (
 
 	endpoint "github.com/envoyproxy/go-control-plane/envoy/config/endpoint/v3"
 	"github.com/mitchellh/copystructure"
-	"go.uber.org/atomic"
 
 	"istio.io/api/label"
 	"istio.io/istio/pilot/pkg/networking"
@@ -105,9 +104,6 @@ type Service struct {
 	// MeshExternal (if true) indicates that the service is external to the mesh.
 	// These services are defined using Istio's ServiceEntry spec.
 	MeshExternal bool
-
-	// MultiCluster indicates whether the service exists in multiple clusters.
-	MultiCluster *atomic.Bool
 }
 
 // Resolution indicates how the service instances need to be resolved before routing
@@ -562,14 +558,6 @@ func (s *Service) External() bool {
 	return s.MeshExternal
 }
 
-// IsMultiCluster predicate checks whether the service is multi cluster.
-func (s *Service) IsMultiCluster() bool {
-	if s.MultiCluster == nil {
-		return false
-	}
-	return s.MultiCluster.Load()
-}
-
 // BuildSubsetKey generates a unique string referencing service instances for a given service name, a subset and a port.
 // The proxy queries Pilot with this key to obtain the list of instances in a subset.
 func BuildSubsetKey(direction TrafficDirection, subsetName string, hostname host.Name, port int) string {
@@ -631,10 +619,6 @@ func (s *Service) GetServiceAddressForProxy(node *Proxy) string {
 		if node.Metadata == nil || node.Metadata.ClusterID == "" {
 			return ""
 		}
-		// If it is not multi clustered, there will be a single one - just return that.
-		if !s.IsMultiCluster() {
-			return s.ClusterVIPs[node.Metadata.ClusterID]
-		}
 		s.Mutex.RLock()
 		defer s.Mutex.RUnlock()
 		return s.ClusterVIPs[node.Metadata.ClusterID]
diff --git a/pilot/pkg/serviceregistry/aggregate/controller.go b/pilot/pkg/serviceregistry/aggregate/controller.go
index e2b51e98aa..935ba04b00 100644
--- a/pilot/pkg/serviceregistry/aggregate/controller.go
+++ b/pilot/pkg/serviceregistry/aggregate/controller.go
@@ -172,9 +172,6 @@ func (c *Controller) GetService(hostname host.Name) (*model.Service, error) {
 }
 
 func mergeService(dst, src *model.Service, srcCluster string) {
-	if dst.MultiCluster == nil {
-		dst.MultiCluster = atomic.NewBool(true)
-	}
 	dst.Mutex.Lock()
 	if dst.ClusterVIPs == nil {
 		dst.ClusterVIPs = make(map[string]string)
diff --git a/pilot/pkg/serviceregistry/kube/conversion.go b/pilot/pkg/serviceregistry/kube/conversion.go
index f4973a7914..b5308d5aea 100644
--- a/pilot/pkg/serviceregistry/kube/conversion.go
+++ b/pilot/pkg/serviceregistry/kube/conversion.go
@@ -19,7 +19,6 @@ import (
 	"sort"
 	"strings"
 
-	"go.uber.org/atomic"
 	coreV1 "k8s.io/api/core/v1"
 
 	"istio.io/api/annotation"
@@ -104,7 +103,6 @@ func ConvertService(svc coreV1.Service, domainSuffix string, clusterID string) *
 		Address:         addr,
 		ServiceAccounts: serviceaccounts,
 		MeshExternal:    meshExternal,
-		MultiCluster:    atomic.NewBool(false),
 		Resolution:      resolution,
 		CreationTime:    svc.CreationTimestamp.Time,
 		ClusterVIPs:     map[string]string{clusterID: addr},


Test patch:
diff --git a/pilot/pkg/serviceregistry/kube/controller/controller_test.go b/pilot/pkg/serviceregistry/kube/controller/controller_test.go
index cb95cb976b..d0d1d44bd4 100644
--- a/pilot/pkg/serviceregistry/kube/controller/controller_test.go
+++ b/pilot/pkg/serviceregistry/kube/controller/controller_test.go
@@ -24,7 +24,6 @@ import (
 	"time"
 
 	core "github.com/envoyproxy/go-control-plane/envoy/config/core/v3"
-	"go.uber.org/atomic"
 	coreV1 "k8s.io/api/core/v1"
 	discovery "k8s.io/api/discovery/v1beta1"
 	"k8s.io/apimachinery/pkg/api/errors"
@@ -419,7 +418,6 @@ func TestGetProxyServiceInstances(t *testing.T) {
 						UID:             "istio://nsa/services/svc1",
 						LabelSelectors:  map[string]string{"app": "prod-app"},
 					},
-					MultiCluster: atomic.NewBool(false),
 				},
 				ServicePort: &model.Port{Name: "tcp-port", Port: 8080, Protocol: protocol.TCP},
 				Endpoint: &model.IstioEndpoint{
@@ -489,7 +487,6 @@ func TestGetProxyServiceInstances(t *testing.T) {
 						UID:             "istio://nsa/services/svc1",
 						LabelSelectors:  map[string]string{"app": "prod-app"},
 					},
-					MultiCluster: atomic.NewBool(false),
 				},
 				ServicePort: &model.Port{Name: "tcp-port", Port: 8080, Protocol: protocol.TCP},
 				Endpoint: &model.IstioEndpoint{
@@ -555,7 +552,6 @@ func TestGetProxyServiceInstances(t *testing.T) {
 						UID:             "istio://nsa/services/svc1",
 						LabelSelectors:  map[string]string{"app": "prod-app"},
 					},
-					MultiCluster: atomic.NewBool(false),
 				},
 				ServicePort: &model.Port{Name: "tcp-port", Port: 8080, Protocol: protocol.TCP},
 				Endpoint: &model.IstioEndpoint{