Keep service instances sorted by creation/hostname (#19265)

pilot/pkg/model/context.go

@@ -20,6 +20,7 @@ import (
 	"fmt"
 	"net"
 	"regexp"
+	"sort"
 	"strconv"
 	"strings"
 
@@ -473,6 +474,18 @@ func (node *Proxy) SetServiceInstances(env *Environment) error {
 		return err
 	}
 
+	// Keep service instances in order of creation/hostname.
+	sort.SliceStable(instances, func(i, j int) bool {
+		if instances[i].Service != nil && instances[j].Service != nil {
+			if !instances[i].Service.CreationTime.Equal(instances[j].Service.CreationTime) {
+				return instances[i].Service.CreationTime.Before(instances[j].Service.CreationTime)
+			}
+			// Additionally, sort by hostname just in case services created automatically at the same second.
+			return instances[i].Service.Hostname < instances[j].Service.Hostname
+		}
+		return true
+	})
+
 	node.ServiceInstances = instances
 	return nil
 }

pilot/pkg/model/context_test.go

@@ -19,15 +19,17 @@ import (
 	"encoding/json"
 	"reflect"
 	"testing"
+	"time"
 
 	"github.com/golang/protobuf/jsonpb"
 	structpb "github.com/golang/protobuf/ptypes/struct"
 	"github.com/stretchr/testify/assert"
 
-	"istio.io/istio/pkg/config/labels"
-
 	"istio.io/istio/pilot/pkg/model"
+	"istio.io/istio/pilot/pkg/networking/core/v1alpha3/fakes"
 	"istio.io/istio/pilot/pkg/serviceregistry/memory"
+	"istio.io/istio/pkg/config/host"
+	"istio.io/istio/pkg/config/labels"
 )
 
 func TestNodeMetadata(t *testing.T) {
@@ -404,3 +406,44 @@ func Test_parseIstioVersion(t *testing.T) {
 		})
 	}
 }
+
+func TestSetServiceInstances(t *testing.T) {
+	tnow := time.Now()
+	instances := []*model.ServiceInstance{
+		{
+			Service: &model.Service{
+				CreationTime: tnow.Add(1 * time.Second),
+				Hostname:     host.Name("test1.com"),
+			},
+		},
+		{
+			Service: &model.Service{
+				CreationTime: tnow,
+				Hostname:     host.Name("test3.com"),
+			},
+		},
+		{
+			Service: &model.Service{
+				CreationTime: tnow,
+				Hostname:     host.Name("test2.com"),
+			},
+		},
+	}
+
+	serviceDiscovery := new(fakes.ServiceDiscovery)
+	serviceDiscovery.GetProxyServiceInstancesReturns(instances, nil)
+
+	env := &model.Environment{
+		ServiceDiscovery: serviceDiscovery,
+	}
+
+	proxy := &model.Proxy{}
+	if err := proxy.SetServiceInstances(env); err != nil {
+		t.Errorf("SetServiceInstances => Got error %v", err)
+	}
+
+	assert.Equal(t, len(proxy.ServiceInstances), 3)
+	assert.Equal(t, proxy.ServiceInstances[0].Service.Hostname, host.Name("test2.com"))
+	assert.Equal(t, proxy.ServiceInstances[1].Service.Hostname, host.Name("test3.com"))
+	assert.Equal(t, proxy.ServiceInstances[2].Service.Hostname, host.Name("test1.com"))
+}

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

@@ -1541,16 +1541,11 @@ func buildInboundListeners(p plugin.Plugin, proxy *model.Proxy, sidecarConfig *m
 	if err := env.PushContext.InitContext(&env, nil, nil); err != nil {
 		return nil
 	}
-	instances := make([]*model.ServiceInstance, len(services))
-	for i, s := range services {
-		instances[i] = &model.ServiceInstance{
-			Service:  s,
-			Endpoint: buildEndpoint(s),
-		}
+	if err := proxy.SetServiceInstances(&env); err != nil {
+		return nil
 	}
 
 	proxy.IstioVersion = model.ParseIstioVersion(proxy.Metadata.IstioVersion)
-	proxy.ServiceInstances = instances
 	if sidecarConfig == nil {
 		proxy.SidecarScope = model.DefaultSidecarScopeForNamespace(env.PushContext, "not-default")
 	} else {
@@ -1731,6 +1726,15 @@ func buildListenerEnvWithVirtualServices(services []*model.Service, virtualServi
 	serviceDiscovery := new(fakes.ServiceDiscovery)
 	serviceDiscovery.ServicesReturns(services, nil)
 
+	instances := make([]*model.ServiceInstance, len(services))
+	for i, s := range services {
+		instances[i] = &model.ServiceInstance{
+			Service:  s,
+			Endpoint: buildEndpoint(s),
+		}
+	}
+	serviceDiscovery.GetProxyServiceInstancesReturns(instances, nil)
+
 	configStore := &fakes.IstioConfigStore{
 		EnvoyFilterStub: func(workloadLabels labels.Collection) *model.Config {
 			return &model.Config{