Integration native test improvements (#23366)

* Integration native test improvements

* Stop using MCP for native mode in favor of pilot file reader
* Allow pilot file reader to set domain correctly
* Enable a few more tests in native mode

This sets us up to remove the Galley component in a followup PR

* Fix test

* cleanup config

* Fix conformance

pilot/pkg/bootstrap/configcontroller.go

@@ -79,7 +79,7 @@ func (s *Server) initConfigController(args *PilotArgs) error {
 		store := memory.Make(collections.Pilot)
 		configController := memory.NewController(store)
 
-		err := s.makeFileMonitor(args.Config.FileDir, configController)
+		err := s.makeFileMonitor(args.Config.FileDir, args.Config.ControllerOptions.DomainSuffix, configController)
 		if err != nil {
 			return err
 		}
@@ -183,7 +183,7 @@ func (s *Server) initMCPConfigController(args *PilotArgs) (err error) {
 				store := memory.MakeWithLedger(collections.Pilot, buildLedger(args.Config))
 				configController := memory.NewController(store)
 
-				err := s.makeFileMonitor(srcAddress.Path, configController)
+				err := s.makeFileMonitor(srcAddress.Path, args.Config.ControllerOptions.DomainSuffix, configController)
 				if err != nil {
 					return err
 				}
@@ -404,8 +404,8 @@ func (s *Server) makeKubeConfigController(args *PilotArgs) (model.ConfigStoreCac
 	return controller.NewController(configClient, args.Config.ControllerOptions), nil
 }
 
-func (s *Server) makeFileMonitor(fileDir string, configController model.ConfigStore) error {
-	fileSnapshot := configmonitor.NewFileSnapshot(fileDir, collections.Pilot)
+func (s *Server) makeFileMonitor(fileDir string, domainSuffix string, configController model.ConfigStore) error {
+	fileSnapshot := configmonitor.NewFileSnapshot(fileDir, collections.Pilot, domainSuffix)
 	fileMonitor := configmonitor.NewMonitor("file-monitor", configController, fileSnapshot.ReadConfigFiles, fileDir)
 
 	// Defer starting the file monitor until after the service is created.

pilot/pkg/config/monitor/file_snapshot.go

@@ -40,14 +40,16 @@ var (
 // config and filter criteria for which of those configs will be parsed.
 type FileSnapshot struct {
 	root             string
+	domainSuffix     string
 	configTypeFilter map[resource.GroupVersionKind]bool
 }
 
 // NewFileSnapshot returns a snapshotter.
 // If no types are provided in the descriptor, all Istio types will be allowed.
-func NewFileSnapshot(root string, schemas collection.Schemas) *FileSnapshot {
+func NewFileSnapshot(root string, schemas collection.Schemas, domainSuffix string) *FileSnapshot {
 	snapshot := &FileSnapshot{
 		root:             root,
+		domainSuffix:     domainSuffix,
 		configTypeFilter: make(map[resource.GroupVersionKind]bool),
 	}
 
@@ -81,7 +83,7 @@ func (f *FileSnapshot) ReadConfigFiles() ([]*model.Config, error) {
 			log.Warnf("Failed to read %s: %v", path, err)
 			return err
 		}
-		configs, err := parseInputs(data)
+		configs, err := parseInputs(data, f.domainSuffix)
 		if err != nil {
 			log.Warnf("Failed to parse %s: %v", path, err)
 			return err
@@ -106,13 +108,14 @@ func (f *FileSnapshot) ReadConfigFiles() ([]*model.Config, error) {
 }
 
 // parseInputs is identical to crd.ParseInputs, except that it returns an array of config pointers.
-func parseInputs(data []byte) ([]*model.Config, error) {
+func parseInputs(data []byte, domainSuffix string) ([]*model.Config, error) {
 	configs, _, err := crd.ParseInputs(string(data))
 
 	// Convert to an array of pointers.
 	refs := make([]*model.Config, len(configs))
 	for i := range configs {
 		refs[i] = &configs[i]
+		refs[i].Domain = domainSuffix
 	}
 	return refs, err
 }

pilot/pkg/config/monitor/file_snapshot_test.go

@@ -70,10 +70,11 @@ func TestFileSnapshotNoFilter(t *testing.T) {
 	ts.testSetup(t)
 	defer ts.testTeardown(t)
 
-	fileWatcher := monitor.NewFileSnapshot(ts.rootPath, collection.SchemasFor())
+	fileWatcher := monitor.NewFileSnapshot(ts.rootPath, collection.SchemasFor(), "foo")
 	configs, err := fileWatcher.ReadConfigFiles()
 	g.Expect(err).NotTo(gomega.HaveOccurred())
 	g.Expect(configs).To(gomega.HaveLen(1))
+	g.Expect(configs[0].Domain).To(gomega.Equal("foo"))
 
 	gateway := configs[0].Spec.(*networking.Gateway)
 	g.Expect(gateway.Servers[0].Port.Number).To(gomega.Equal(uint32(80)))
@@ -94,7 +95,7 @@ func TestFileSnapshotWithFilter(t *testing.T) {
 	ts.testSetup(t)
 	defer ts.testTeardown(t)
 
-	fileWatcher := monitor.NewFileSnapshot(ts.rootPath, collection.SchemasFor(collections.IstioNetworkingV1Alpha3Virtualservices))
+	fileWatcher := monitor.NewFileSnapshot(ts.rootPath, collection.SchemasFor(collections.IstioNetworkingV1Alpha3Virtualservices), "")
 	configs, err := fileWatcher.ReadConfigFiles()
 	g.Expect(err).NotTo(gomega.HaveOccurred())
 	g.Expect(configs).To(gomega.HaveLen(1))
@@ -116,7 +117,7 @@ func TestFileSnapshotSorting(t *testing.T) {
 	ts.testSetup(t)
 	defer ts.testTeardown(t)
 
-	fileWatcher := monitor.NewFileSnapshot(ts.rootPath, collection.SchemasFor())
+	fileWatcher := monitor.NewFileSnapshot(ts.rootPath, collection.SchemasFor(), "")
 
 	configs, err := fileWatcher.ReadConfigFiles()
 	g.Expect(err).NotTo(gomega.HaveOccurred())

pkg/test/framework/components/galley/galley.go

@@ -43,9 +43,15 @@ type Instance interface {
 	// ApplyConfigDir recursively applies all the config files in the specified directory
 	ApplyConfigDir(ns namespace.Instance, configDir string) error
 
+	// DeleteConfigDir recursively deletes all the config files in the specified directory
+	DeleteConfigDir(ns namespace.Instance, configDir string) error
+
 	// ClearConfig clears all applied config so far.
 	ClearConfig() error
 
+	// GetConfigDir returns the current configuration directory.
+	GetConfigDir() string
+
 	// SetMeshConfig applies the given mesh config.
 	SetMeshConfig(meshCfg string) error
 

pkg/test/framework/components/galley/kube.go

@@ -118,6 +118,10 @@ type kubeComponent struct {
 
 var _ Instance = &kubeComponent{}
 
+func (c *kubeComponent) GetConfigDir() string {
+	return ""
+}
+
 // ID implements resource.Instance
 func (c *kubeComponent) ID() resource.ID {
 	return c.id
@@ -230,6 +234,25 @@ func (c *kubeComponent) ApplyConfigDir(ns namespace.Instance, sourceDir string)
 	})
 }
 
+// ApplyConfigDir implements Galley.ApplyConfigDir.
+func (c *kubeComponent) DeleteConfigDir(ns namespace.Instance, sourceDir string) (err error) {
+	return filepath.Walk(sourceDir, func(path string, info os.FileInfo, err error) error {
+		if err != nil {
+			return err
+		}
+		if info.IsDir() {
+			return nil
+		}
+
+		contents, readerr := ioutil.ReadFile(path)
+		if readerr != nil {
+			return readerr
+		}
+
+		return c.DeleteConfig(ns, string(contents))
+	})
+}
+
 // SetMeshConfig implements Instance
 func (c *kubeComponent) SetMeshConfig(meshCfg string) error {
 	return fmt.Errorf("NYI: K8s Galley.SetMeshConfig")

pkg/test/framework/components/galley/native.go

@@ -15,24 +15,22 @@
 package galley
 
 import (
-	"fmt"
 	"io"
 	"io/ioutil"
 	"os"
 	"path"
 	"path/filepath"
 	"syscall"
-	"time"
+
+	"istio.io/pkg/appsignals"
 
 	"istio.io/istio/galley/pkg/server"
-	"istio.io/istio/galley/pkg/server/settings"
 	"istio.io/istio/pkg/test"
 	"istio.io/istio/pkg/test/framework/components/environment/native"
 	"istio.io/istio/pkg/test/framework/components/namespace"
 	"istio.io/istio/pkg/test/framework/resource"
 	"istio.io/istio/pkg/test/scopes"
 	"istio.io/istio/pkg/test/util/yml"
-	"istio.io/pkg/appsignals"
 )
 
 var (
@@ -95,6 +93,9 @@ func (c *nativeComponent) ID() resource.ID {
 
 // Address of the Galley MCP Server.
 func (c *nativeComponent) Address() string {
+	if c.client == nil {
+		return ""
+	}
 	return c.client.address
 }
 
@@ -197,6 +198,26 @@ func (c *nativeComponent) ApplyConfigDir(ns namespace.Instance, sourceDir string
 	})
 }
 
+// ApplyConfigDir implements Galley.DeleteConfigDir.
+func (c *nativeComponent) DeleteConfigDir(ns namespace.Instance, sourceDir string) (err error) {
+	defer appsignals.Notify("galley.native.DeleteConfigDir", syscall.SIGUSR1)
+	return filepath.Walk(sourceDir, func(path string, info os.FileInfo, err error) error {
+		if err != nil {
+			return err
+		}
+		if info.IsDir() {
+			return nil
+		}
+
+		contents, readerr := ioutil.ReadFile(path)
+		if readerr != nil {
+			return readerr
+		}
+
+		return c.DeleteConfig(ns, string(contents))
+	})
+}
+
 // SetMeshConfig implements Instance
 func (c *nativeComponent) SetMeshConfig(meshCfg string) error {
 	if err := ioutil.WriteFile(c.meshConfigFile, []byte(meshCfg), os.ModePerm); err != nil {
@@ -216,7 +237,7 @@ func (c *nativeComponent) SetMeshConfigOrFail(t test.Failer, meshCfg string) {
 
 // WaitForSnapshot implements Galley.WaitForSnapshot.
 func (c *nativeComponent) WaitForSnapshot(collection string, validator SnapshotValidatorFunc) error {
-	return c.client.waitForSnapshot(collection, validator)
+	return nil
 }
 
 // WaitForSnapshotOrFail implements Galley.WaitForSnapshotOrFail.
@@ -227,6 +248,10 @@ func (c *nativeComponent) WaitForSnapshotOrFail(t test.Failer, collection string
 	}
 }
 
+func (c *nativeComponent) GetConfigDir() string {
+	return c.configDir
+}
+
 func (c *nativeComponent) reset() error {
 	_ = c.Close()
 
@@ -261,55 +286,6 @@ func (c *nativeComponent) reset() error {
 	if err = c.applyAttributeManifest(); err != nil {
 		return err
 	}
-	return c.restart()
-}
-
-func (c *nativeComponent) restart() error {
-	a := settings.DefaultArgs()
-	a.Insecure = true
-	a.EnableServer = true
-	a.DisableResourceReadyCheck = true
-	a.ConfigPath = c.configDir
-	a.MeshConfigFile = c.meshConfigFile
-	// To prevent ctrlZ port collision between galley/pilot&mixer
-	a.IntrospectionOptions.Port = 0
-	a.MonitoringPort = 0
-	a.ExcludedResourceKinds = nil
-	a.EnableServiceDiscovery = true
-	a.EnableValidationServer = false
-	a.EnableValidationController = false
-	a.ValidationWebhookControllerArgs.UnregisterValidationWebhook = false
-	a.Readiness.Path = "/tmp/readinessProbe"
-	a.Liveness.Path = "/tmp/livenessProbe"
-
-	// Bind to an arbitrary port.
-	a.APIAddress = "tcp://0.0.0.0:0"
-
-	if c.cfg.SinkAddress != "" {
-		a.SinkAddress = c.cfg.SinkAddress
-		a.SinkAuthMode = "NONE"
-	}
-
-	s := server.New(a)
-	if err := s.Start(); err != nil {
-		scopes.Framework.Errorf("Error starting Galley: %v", err)
-		return err
-	}
-
-	c.server = s
-
-	// TODO: This is due to Galley start-up being racy. We should go back to the "Start" based model where
-	// return from s.Start() guarantees that all the setup is complete.
-	time.Sleep(time.Second)
-
-	c.client = &client{
-		address: fmt.Sprintf("tcp://%s", s.Address().String()),
-	}
-
-	if err := c.client.waitForStartup(); err != nil {
-		return err
-	}
-
 	return nil
 }
 

pkg/test/framework/components/pilot/native.go

@@ -15,7 +15,6 @@
 package pilot
 
 import (
-	"errors"
 	"io"
 	"net"
 	"os"
@@ -60,10 +59,6 @@ type nativeComponent struct {
 
 // NewNativeComponent factory function for the component
 func newNative(ctx resource.Context, cfg Config) (Instance, error) {
-	if cfg.Galley == nil {
-		return nil, errors.New("galley must be provided")
-	}
-
 	e := ctx.Environment().(*native.Environment)
 	instance := &nativeComponent{
 		environment: ctx.Environment().(*native.Environment),
@@ -85,9 +80,6 @@ func newNative(ctx resource.Context, cfg Config) (Instance, error) {
 		m = cfg.MeshConfig
 	}
 	m.AccessLogFile = "./var/log/istio/access.log"
-	// The local tests will use SDS, so we need to override the mesh to specify the UDS path
-	// TODO(howardjohn) should we make this mesh wide default?
-	m.SdsUdsPath = "unix:./etc/istio/proxy/SDS"
 
 	if cfg.ServiceArgs.Registries == nil {
 		cfg.ServiceArgs = bootstrap.ServiceArgs{
@@ -117,15 +109,8 @@ func newNative(ctx resource.Context, cfg Config) (Instance, error) {
 	if bootstrapArgs.MeshConfig == nil {
 		bootstrapArgs.MeshConfig = &meshapi.MeshConfig{}
 	}
-
-	galleyHostPort := cfg.Galley.Address()[6:]
-	// Set as MCP address, note needs to strip 'tcp://' from the address prefix
-	// Also appending incase if there are existing config sources
-	bootstrapArgs.MeshConfig.ConfigSources = append(bootstrapArgs.MeshConfig.ConfigSources, &meshapi.ConfigSource{
-		Address: galleyHostPort,
-	})
-
-	bootstrapArgs.MCPOptions.MaxMessageSize = 1024 * 1024 * 4
+	// TODO make pilot component (or something other than galley) control this
+	bootstrapArgs.Config.FileDir = cfg.Galley.GetConfigDir()
 
 	// Use testing certs
 	if err := os.Setenv(bootstrap.LocalCertDir.Name, path.Join(env.IstioSrc, "tests/testdata/certs/pilot")); err != nil {