diff --git a/agent/cache-types/resolved_service_config_test.go b/agent/cache-types/resolved_service_config_test.go index f89002393..f52d65e42 100644 --- a/agent/cache-types/resolved_service_config_test.go +++ b/agent/cache-types/resolved_service_config_test.go @@ -28,9 +28,13 @@ func TestResolvedServiceConfig(t *testing.T) { require.True(req.AllowStale) reply := args.Get(2).(*structs.ServiceConfigResponse) - reply.Definition = structs.ServiceDefinition{ - ID: "1234", - Name: "foo", + reply.ProxyConfig = map[string]interface{}{ + "protocol": "http", + } + reply.UpstreamConfigs = map[string]map[string]interface{}{ + "s2": map[string]interface{}{ + "protocol": "http", + }, } reply.QueryMeta.Index = 48 diff --git a/agent/consul/config_endpoint.go b/agent/consul/config_endpoint.go index a93251547..348c4c9a3 100644 --- a/agent/consul/config_endpoint.go +++ b/agent/consul/config_endpoint.go @@ -258,22 +258,45 @@ func (c *ConfigEntry) ResolveServiceConfig(args *structs.ServiceConfigRequest, r } } - // Resolve the service definition by overlaying the service config onto the global - // proxy config. - definition := structs.ServiceDefinition{ - Name: args.Name, - } - if proxyConf != nil { - definition.Proxy = &structs.ConnectProxyConfig{ - Config: proxyConf.Config, + reply.Index = index + // Apply the proxy defaults to the sidecar's proxy config + reply.ProxyConfig = proxyConf.Config + + if serviceConf != nil && serviceConf.Protocol != "" { + if reply.ProxyConfig == nil { + reply.ProxyConfig = make(map[string]interface{}) + } + reply.ProxyConfig["protocol"] = serviceConf.Protocol + } + + // Apply the upstream protocols to the upstream configs + for _, upstream := range args.Upstreams { + _, upstreamEntry, err := state.ConfigEntry(ws, structs.ServiceDefaults, upstream) + if err != nil { + return err + } + var upstreamConf *structs.ServiceConfigEntry + var ok bool + if upstreamEntry != nil { + upstreamConf, ok = upstreamEntry.(*structs.ServiceConfigEntry) + if !ok { + return fmt.Errorf("invalid service config type %T", upstreamEntry) + } + } + + // Nothing to configure if a protocol hasn't been set. + if upstreamConf == nil || upstreamConf.Protocol == "" { + continue + } + + if reply.UpstreamConfigs == nil { + reply.UpstreamConfigs = make(map[string]map[string]interface{}) + } + reply.UpstreamConfigs[upstream] = map[string]interface{}{ + "protocol": upstreamConf.Protocol, } - } - if serviceConf != nil { - definition.Name = serviceConf.Name } - reply.Index = index - reply.Definition = definition return nil }) } diff --git a/agent/consul/config_endpoint_test.go b/agent/consul/config_endpoint_test.go index 0f1514745..7dde5061d 100644 --- a/agent/consul/config_endpoint_test.go +++ b/agent/consul/config_endpoint_test.go @@ -6,6 +6,7 @@ import ( "github.com/hashicorp/consul/acl" "github.com/hashicorp/consul/agent/structs" + "github.com/hashicorp/consul/lib" "github.com/hashicorp/consul/testrpc" msgpackrpc "github.com/hashicorp/net-rpc-msgpackrpc" "github.com/stretchr/testify/require" @@ -659,31 +660,51 @@ func TestConfigEntry_ResolveServiceConfig(t *testing.T) { Kind: structs.ProxyDefaults, Name: structs.ProxyConfigGlobal, Config: map[string]interface{}{ - "foo": "bar", + "foo": 1, }, })) require.NoError(state.EnsureConfigEntry(2, &structs.ServiceConfigEntry{ - Kind: structs.ServiceDefaults, - Name: "foo", + Kind: structs.ServiceDefaults, + Name: "foo", + Protocol: "http", + })) + require.NoError(state.EnsureConfigEntry(2, &structs.ServiceConfigEntry{ + Kind: structs.ServiceDefaults, + Name: "bar", + Protocol: "grpc", })) args := structs.ServiceConfigRequest{ Name: "foo", Datacenter: s1.config.Datacenter, + Upstreams: []string{"bar", "baz"}, } var out structs.ServiceConfigResponse require.NoError(msgpackrpc.CallWithCodec(codec, "ConfigEntry.ResolveServiceConfig", &args, &out)) + // Hack to fix up the string encoding in the map[string]interface{}. + // msgpackRPC's codec doesn't use RawToString. + var err error + out.ProxyConfig, err = lib.MapWalk(out.ProxyConfig) + require.NoError(err) + for k := range out.UpstreamConfigs { + out.UpstreamConfigs[k], err = lib.MapWalk(out.UpstreamConfigs[k]) + require.NoError(err) + } - expected := structs.ServiceDefinition{ - Name: "foo", - Proxy: &structs.ConnectProxyConfig{ - Config: map[string]interface{}{ - "foo": "bar", + expected := structs.ServiceConfigResponse{ + ProxyConfig: map[string]interface{}{ + "foo": int64(1), + "protocol": "http", + }, + UpstreamConfigs: map[string]map[string]interface{}{ + "bar": map[string]interface{}{ + "protocol": "grpc", }, }, + // Don't know what this is deterministically + QueryMeta: out.QueryMeta, } - out.Definition.Proxy.Config["foo"] = structs.Uint8ToString(out.Definition.Proxy.Config["foo"].([]uint8)) - require.Equal(expected, out.Definition) + require.Equal(expected, out) } func TestConfigEntry_ResolveServiceConfig_ACLDeny(t *testing.T) { diff --git a/agent/consul/config_replication_test.go b/agent/consul/config_replication_test.go index 8fa2c4f59..06fcf30b5 100644 --- a/agent/consul/config_replication_test.go +++ b/agent/consul/config_replication_test.go @@ -93,7 +93,6 @@ func TestReplication_ConfigEntries(t *testing.T) { require.True(t, ok) require.Equal(t, remoteSvc.Protocol, localSvc.Protocol) - require.Equal(t, remoteSvc.Connect, localSvc.Connect) case structs.ProxyDefaults: localProxy, ok := local[i].(*structs.ProxyConfigEntry) require.True(t, ok) diff --git a/agent/service_manager.go b/agent/service_manager.go index a227ff158..64a58a6ab 100644 --- a/agent/service_manager.go +++ b/agent/service_manager.go @@ -7,6 +7,8 @@ import ( "github.com/hashicorp/consul/agent/cache" cachetype "github.com/hashicorp/consul/agent/cache-types" "github.com/hashicorp/consul/agent/structs" + "github.com/imdario/mergo" + "github.com/mitchellh/copystructure" "golang.org/x/net/context" ) @@ -34,6 +36,12 @@ func NewServiceManager(agent *Agent) *ServiceManager { // to fetch the merged global defaults that apply to the service in order to compose the // initial registration. func (s *ServiceManager) AddService(service *structs.NodeService, chkTypes []*structs.CheckType, persist bool, token string, source configSource) error { + // For now only sidecar proxies have anything that can be configured + // centrally. So bypass the whole manager for regular services. + if !service.IsSidecarProxy() { + return s.agent.addServiceInternal(service, chkTypes, persist, token, source) + } + s.lock.Lock() defer s.lock.Unlock() @@ -111,7 +119,7 @@ type serviceRegistration struct { // service/proxy defaults. type serviceConfigWatch struct { registration *serviceRegistration - config *structs.ServiceDefinition + defaults *structs.ServiceConfigResponse agent *Agent @@ -119,10 +127,24 @@ type serviceConfigWatch struct { // for the resolved service config is received from the cache. readyCh chan error - updateCh chan cache.UpdateEvent + // ctx and cancelFunc store the overall context that lives as long as the + // Watch instance is needed, possibly spanning multiple cache.Notify + // lifetimes. ctx context.Context cancelFunc func() + // cacheKey stores the key of the current request, when registration changes + // we check to see if a new cache watch is needed. + cacheKey string + + // updateCh receives changes from cache watchers or registration changes. + updateCh chan cache.UpdateEvent + + // notifyCancel, if non-nil it the cancel func that will stop the currently + // active Notify loop. It does not cancel ctx and is used when we need to + // switch to a new Notify call because cache key changed. + notifyCancel func() + lock sync.Mutex } @@ -130,7 +152,7 @@ type serviceConfigWatch struct { // the updateCh. This is not safe to call more than once. func (s *serviceConfigWatch) Start() error { s.ctx, s.cancelFunc = context.WithCancel(context.Background()) - if err := s.startConfigWatch(); err != nil { + if err := s.ensureConfigWatch(); err != nil { return err } go s.runWatch() @@ -194,20 +216,34 @@ func (s *serviceConfigWatch) handleUpdate(event cache.UpdateEvent, locked, first return fmt.Errorf("error watching service config: %v", event.Err) } } else { - switch event.Result.(type) { + switch res := event.Result.(type) { case *serviceRegistration: - s.registration = event.Result.(*serviceRegistration) + s.registration = res + // We may need to restart watch if upstreams changed + if err := s.ensureConfigWatch(); err != nil { + return err + } case *structs.ServiceConfigResponse: - resp := event.Result.(*structs.ServiceConfigResponse) - s.config = &resp.Definition + // Sanity check this even came from the currently active watch to ignore + // rare races when switching cache keys + if event.CorrelationID != s.cacheKey { + // It's a no-op. The new watcher will deliver (or may have already + // delivered) the correct config so just ignore this old message. + return nil + } + s.defaults = res default: return fmt.Errorf("unknown update event type: %T", event) } } - service := s.mergeServiceConfig() - err := s.agent.addServiceInternal(service, s.registration.chkTypes, s.registration.persist, s.registration.token, s.registration.source) + // Merge the local registration with the central defaults and update this service + // in the local state. + service, err := s.mergeServiceConfig() if err != nil { + return err + } + if err := s.updateAgentRegistration(service); err != nil { // If this is the initial registration, return the error through the readyCh // so it can be passed back to the original caller. if firstRun { @@ -224,20 +260,75 @@ func (s *serviceConfigWatch) handleUpdate(event cache.UpdateEvent, locked, first return nil } -// startConfigWatch starts a cache.Notify goroutine to run a continuous blocking query -// on the resolved service config for this service. -func (s *serviceConfigWatch) startConfigWatch() error { - name := s.registration.service.Service +// updateAgentRegistration updates the service (and its sidecar, if applicable) in the +// local state. +func (s *serviceConfigWatch) updateAgentRegistration(ns *structs.NodeService) error { + return s.agent.addServiceInternal(ns, s.registration.chkTypes, s.registration.persist, s.registration.token, s.registration.source) +} + +// ensureConfigWatch starts a cache.Notify goroutine to run a continuous +// blocking query on the resolved service config for this service. If the +// registration has changed in a way that requires a new blocking query, it will +// cancel any current watch and start a new one. It is a no-op if there is an +// existing watch that is sufficient for the current registration. It is not +// thread-safe and must only be called from the Start method (which is only safe +// to call once as documented) or from inside the run loop. +func (s *serviceConfigWatch) ensureConfigWatch() error { + ns := s.registration.service + name := ns.Service + var upstreams []string + + // Note that only sidecar proxies should even make it here for now although + // later that will change to add the condition. + if ns.IsSidecarProxy() { + // This is a sidecar proxy, ignore the proxy service's config since we are + // managed by the target service config. + name = ns.Proxy.DestinationServiceName + + // Also if we have any upstreams defined, add them to the request so we can + // learn about their configs. + for _, us := range ns.Proxy.Upstreams { + if us.DestinationType == "" || us.DestinationType == structs.UpstreamDestTypeService { + upstreams = append(upstreams, us.DestinationName) + } + } + } req := &structs.ServiceConfigRequest{ Name: name, Datacenter: s.agent.config.Datacenter, QueryOptions: structs.QueryOptions{Token: s.agent.config.ACLAgentToken}, + Upstreams: upstreams, } if s.registration.token != "" { req.QueryOptions.Token = s.registration.token } - err := s.agent.cache.Notify(s.ctx, cachetype.ResolvedServiceConfigName, req, fmt.Sprintf("service-config:%s", name), s.updateCh) + + // See if this request is different from the current one + cacheKey := req.CacheInfo().Key + if cacheKey == s.cacheKey { + return nil + } + + // If there is an existing notify running, stop it first. This may leave a + // blocking query running in the background but the Notify loop will swallow + // the response and exit when it next unblocks so we can consider it stopped. + if s.notifyCancel != nil { + s.notifyCancel() + } + + // Make a new context just for this Notify call + ctx, cancel := context.WithCancel(s.ctx) + s.notifyCancel = cancel + s.cacheKey = cacheKey + // We use the cache key as the correlationID here. Notify in general will not + // respond on the updateCh after the context is cancelled however there could + // possible be a race where it has only just got an update and checked the + // context before we cancel and so might still deliver the old event. Using + // the cacheKey allows us to ignore updates from the old cache watch and makes + // even this rare edge case safe. + err := s.agent.cache.Notify(ctx, cachetype.ResolvedServiceConfigName, req, + s.cacheKey, s.updateCh) return err } @@ -252,13 +343,40 @@ func (s *serviceConfigWatch) updateRegistration(registration *serviceRegistratio // mergeServiceConfig returns the final effective config for the watched service, // including the latest known global defaults from the servers. -func (s *serviceConfigWatch) mergeServiceConfig() *structs.NodeService { - if s.config == nil { - return s.registration.service +func (s *serviceConfigWatch) mergeServiceConfig() (*structs.NodeService, error) { + if s.defaults == nil || !s.registration.service.IsSidecarProxy() { + return s.registration.service, nil } - svc := s.config.NodeService() - svc.Merge(s.registration.service) + // We don't want to change s.registration in place since it is our source of + // truth about what was actually registered before defaults applied. So copy + // it first. + nsRaw, err := copystructure.Copy(s.registration.service) + if err != nil { + return nil, err + } - return svc + // Merge proxy defaults + ns := nsRaw.(*structs.NodeService) + + if err := mergo.Merge(&ns.Proxy.Config, s.defaults.ProxyConfig); err != nil { + return nil, err + } + // Merge upstream defaults if there were any returned + for i := range ns.Proxy.Upstreams { + // Get a pointer not a value copy of the upstream struct + us := &ns.Proxy.Upstreams[i] + if us.DestinationType != "" && us.DestinationType != structs.UpstreamDestTypeService { + continue + } + usCfg, ok := s.defaults.UpstreamConfigs[us.DestinationName] + if !ok { + // No config defaults to merge + continue + } + if err := mergo.Merge(&us.Config, usCfg); err != nil { + return nil, err + } + } + return ns, err } diff --git a/agent/service_manager_test.go b/agent/service_manager_test.go index 53d05b8e2..04474d19e 100644 --- a/agent/service_manager_test.go +++ b/agent/service_manager_test.go @@ -16,42 +16,155 @@ func TestServiceManager_RegisterService(t *testing.T) { testrpc.WaitForLeader(t, a.RPC, "dc1") - // Register some global proxy config - args := &structs.ConfigEntryRequest{ - Datacenter: "dc1", - Entry: &structs.ProxyConfigEntry{ - Config: map[string]interface{}{ - "foo": 1, + // Register a global proxy and service config + { + args := &structs.ConfigEntryRequest{ + Datacenter: "dc1", + Entry: &structs.ProxyConfigEntry{ + Config: map[string]interface{}{ + "foo": 1, + }, }, - }, + } + var out bool + require.NoError(a.RPC("ConfigEntry.Apply", args, &out)) + } + { + args := &structs.ConfigEntryRequest{ + Datacenter: "dc1", + Entry: &structs.ServiceConfigEntry{ + Kind: structs.ServiceDefaults, + Name: "redis", + Protocol: "tcp", + }, + } + var out bool + require.NoError(a.RPC("ConfigEntry.Apply", args, &out)) } - out := false - require.NoError(a.RPC("ConfigEntry.Apply", args, &out)) - // Now register a service locally and make sure the resulting State entry - // has the global config in it. + // Now register a service locally with no sidecar, it should be a no-op. svc := &structs.NodeService{ ID: "redis", Service: "redis", Port: 8000, } require.NoError(a.AddService(svc, nil, false, "", ConfigSourceLocal)) - mergedService := a.State.Service("redis") - require.NotNil(mergedService) + + // Verify both the service and sidecar. + redisService := a.State.Service("redis") + require.NotNil(redisService) require.Equal(&structs.NodeService{ ID: "redis", Service: "redis", Port: 8000, + Weights: &structs.Weights{ + Passing: 1, + Warning: 1, + }, + }, redisService) +} + +func TestServiceManager_RegisterSidecar(t *testing.T) { + require := require.New(t) + + a := NewTestAgent(t, t.Name(), "enable_central_service_config = true") + defer a.Shutdown() + + testrpc.WaitForLeader(t, a.RPC, "dc1") + + // Register a global proxy and service config + { + args := &structs.ConfigEntryRequest{ + Datacenter: "dc1", + Entry: &structs.ProxyConfigEntry{ + Config: map[string]interface{}{ + "foo": 1, + }, + }, + } + var out bool + require.NoError(a.RPC("ConfigEntry.Apply", args, &out)) + } + { + args := &structs.ConfigEntryRequest{ + Datacenter: "dc1", + Entry: &structs.ServiceConfigEntry{ + Kind: structs.ServiceDefaults, + Name: "web", + Protocol: "http", + }, + } + var out bool + require.NoError(a.RPC("ConfigEntry.Apply", args, &out)) + } + { + args := &structs.ConfigEntryRequest{ + Datacenter: "dc1", + Entry: &structs.ServiceConfigEntry{ + Kind: structs.ServiceDefaults, + Name: "redis", + Protocol: "tcp", + }, + } + var out bool + require.NoError(a.RPC("ConfigEntry.Apply", args, &out)) + } + + // Now register a sidecar proxy. Note we don't use SidecarService here because + // that gets resolved earlier in config handling than the AddService call + // here. + svc := &structs.NodeService{ + Kind: structs.ServiceKindConnectProxy, + ID: "web-sidecar-proxy", + Service: "web-sidecar-proxy", + Port: 21000, Proxy: structs.ConnectProxyConfig{ + DestinationServiceName: "web", + DestinationServiceID: "web", + LocalServiceAddress: "127.0.0.1", + LocalServicePort: 8000, + Upstreams: structs.Upstreams{ + { + DestinationName: "redis", + LocalBindPort: 5000, + }, + }, + }, + } + require.NoError(a.AddService(svc, nil, false, "", ConfigSourceLocal)) + + // Verify sidecar got global config loaded + sidecarService := a.State.Service("web-sidecar-proxy") + require.NotNil(sidecarService) + require.Equal(&structs.NodeService{ + Kind: structs.ServiceKindConnectProxy, + ID: "web-sidecar-proxy", + Service: "web-sidecar-proxy", + Port: 21000, + Proxy: structs.ConnectProxyConfig{ + DestinationServiceName: "web", + DestinationServiceID: "web", + LocalServiceAddress: "127.0.0.1", + LocalServicePort: 8000, Config: map[string]interface{}{ - "foo": int64(1), + "foo": int64(1), + "protocol": "http", + }, + Upstreams: structs.Upstreams{ + { + DestinationName: "redis", + LocalBindPort: 5000, + Config: map[string]interface{}{ + "protocol": "tcp", + }, + }, }, }, Weights: &structs.Weights{ Passing: 1, Warning: 1, }, - }, mergedService) + }, sidecarService) } func TestServiceManager_Disabled(t *testing.T) { @@ -62,37 +175,92 @@ func TestServiceManager_Disabled(t *testing.T) { testrpc.WaitForLeader(t, a.RPC, "dc1") - // Register some global proxy config - args := &structs.ConfigEntryRequest{ - Datacenter: "dc1", - Entry: &structs.ProxyConfigEntry{ - Config: map[string]interface{}{ - "foo": 1, + // Register a global proxy and service config + { + args := &structs.ConfigEntryRequest{ + Datacenter: "dc1", + Entry: &structs.ProxyConfigEntry{ + Config: map[string]interface{}{ + "foo": 1, + }, + }, + } + var out bool + require.NoError(a.RPC("ConfigEntry.Apply", args, &out)) + } + { + args := &structs.ConfigEntryRequest{ + Datacenter: "dc1", + Entry: &structs.ServiceConfigEntry{ + Kind: structs.ServiceDefaults, + Name: "web", + Protocol: "http", + }, + } + var out bool + require.NoError(a.RPC("ConfigEntry.Apply", args, &out)) + } + { + args := &structs.ConfigEntryRequest{ + Datacenter: "dc1", + Entry: &structs.ServiceConfigEntry{ + Kind: structs.ServiceDefaults, + Name: "redis", + Protocol: "tcp", + }, + } + var out bool + require.NoError(a.RPC("ConfigEntry.Apply", args, &out)) + } + + // Now register a sidecar proxy. Note we don't use SidecarService here because + // that gets resolved earlier in config handling than the AddService call + // here. + svc := &structs.NodeService{ + Kind: structs.ServiceKindConnectProxy, + ID: "web-sidecar-proxy", + Service: "web-sidecar-proxy", + Port: 21000, + Proxy: structs.ConnectProxyConfig{ + DestinationServiceName: "web", + DestinationServiceID: "web", + LocalServiceAddress: "127.0.0.1", + LocalServicePort: 8000, + Upstreams: structs.Upstreams{ + { + DestinationName: "redis", + LocalBindPort: 5000, + }, }, }, } - out := false - require.NoError(a.RPC("ConfigEntry.Apply", args, &out)) - - // Now register a service locally and make sure the resulting State entry - // has the global config in it. - svc := &structs.NodeService{ - ID: "redis", - Service: "redis", - Port: 8000, - } require.NoError(a.AddService(svc, nil, false, "", ConfigSourceLocal)) - mergedService := a.State.Service("redis") - require.NotNil(mergedService) - // The proxy config map shouldn't be present; the agent should ignore global - // defaults here. + + // Verify sidecar got global config loaded + sidecarService := a.State.Service("web-sidecar-proxy") + require.NotNil(sidecarService) require.Equal(&structs.NodeService{ - ID: "redis", - Service: "redis", - Port: 8000, + Kind: structs.ServiceKindConnectProxy, + ID: "web-sidecar-proxy", + Service: "web-sidecar-proxy", + Port: 21000, + Proxy: structs.ConnectProxyConfig{ + DestinationServiceName: "web", + DestinationServiceID: "web", + LocalServiceAddress: "127.0.0.1", + LocalServicePort: 8000, + // No config added + Upstreams: structs.Upstreams{ + { + DestinationName: "redis", + LocalBindPort: 5000, + // No config added + }, + }, + }, Weights: &structs.Weights{ Passing: 1, Warning: 1, }, - }, mergedService) + }, sidecarService) } diff --git a/agent/structs/config_entry.go b/agent/structs/config_entry.go index 059ba35c4..1ab8c2b08 100644 --- a/agent/structs/config_entry.go +++ b/agent/structs/config_entry.go @@ -46,7 +46,11 @@ type ServiceConfigEntry struct { Kind string Name string Protocol string - Connect ConnectConfiguration + // TODO(banks): enable this once we have upstreams supported too. Enabling + // sidecars actually makes no sense and adds complications when you don't + // allow upstreams to be specified centrally too. + // + // Connect ConnectConfiguration RaftIndex } @@ -368,6 +372,7 @@ func (c *ConfigEntryQuery) RequestDatacenter() string { type ServiceConfigRequest struct { Name string Datacenter string + Upstreams []string QueryOptions } @@ -386,10 +391,18 @@ func (r *ServiceConfigRequest) CacheInfo() cache.RequestInfo { MustRevalidate: r.MustRevalidate, } - // To calculate the cache key we only hash the service name. The - // datacenter is handled by the cache framework. The other fields are - // not, but should not be used in any cache types. - v, err := hashstructure.Hash(r.Name, nil) + // To calculate the cache key we only hash the service name and upstream set. + // We don't want ordering of the upstreams to affect the outcome so use an + // anonymous struct field with hash:set behavior. Note the order of fields in + // the slice would affect cache keys if we ever persist between agent restarts + // and change it. + v, err := hashstructure.Hash(struct { + Name string + Upstreams []string `hash:"set"` + }{ + Name: r.Name, + Upstreams: r.Upstreams, + }, nil) if err == nil { // If there is an error, we don't set the key. A blank key forces // no cache for this request so the request is forwarded directly @@ -401,8 +414,8 @@ func (r *ServiceConfigRequest) CacheInfo() cache.RequestInfo { } type ServiceConfigResponse struct { - Definition ServiceDefinition - + ProxyConfig map[string]interface{} + UpstreamConfigs map[string]map[string]interface{} QueryMeta } diff --git a/agent/structs/config_entry_test.go b/agent/structs/config_entry_test.go index 0ffafe223..3fd7ea955 100644 --- a/agent/structs/config_entry_test.go +++ b/agent/structs/config_entry_test.go @@ -62,7 +62,7 @@ func TestDecodeConfigEntry(t *testing.T) { Kind: ServiceDefaults, Name: "foo", Protocol: "tcp", - Connect: ConnectConfiguration{SidecarProxy: true}, + //Connect: ConnectConfiguration{SidecarProxy: true}, }, }, "service-defaults translations": tcase{ @@ -78,7 +78,7 @@ func TestDecodeConfigEntry(t *testing.T) { Kind: ServiceDefaults, Name: "foo", Protocol: "tcp", - Connect: ConnectConfiguration{SidecarProxy: true}, + //Connect: ConnectConfiguration{SidecarProxy: true}, }, }, } diff --git a/agent/structs/structs.go b/agent/structs/structs.go index 56a19fc4e..dd2cea214 100644 --- a/agent/structs/structs.go +++ b/agent/structs/structs.go @@ -12,13 +12,14 @@ import ( "strings" "time" - "github.com/hashicorp/consul/agent/cache" - "github.com/hashicorp/consul/api" - "github.com/hashicorp/consul/types" "github.com/hashicorp/go-msgpack/codec" multierror "github.com/hashicorp/go-multierror" "github.com/hashicorp/serf/coordinate" "github.com/mitchellh/hashstructure" + + "github.com/hashicorp/consul/agent/cache" + "github.com/hashicorp/consul/api" + "github.com/hashicorp/consul/types" ) type MessageType uint8 @@ -771,76 +772,9 @@ type ServiceConnect struct { SidecarService *ServiceDefinition `json:",omitempty" bexpr:"-"` } -// Merge overlays any non-empty fields of other onto s. Tags, metadata and proxy -// config are unioned together instead of overwritten. The Connect field and the -// non-config proxy fields are taken from other. -func (s *NodeService) Merge(other *NodeService) { - if other.Kind != "" { - s.Kind = other.Kind - } - if other.ID != "" { - s.ID = other.ID - } - if other.Service != "" { - s.Service = other.Service - } - - if s.Tags == nil { - s.Tags = other.Tags - } else if other.Tags != nil { - // Both nodes have tags, so deduplicate and merge them. - tagSet := make(map[string]struct{}) - for _, tag := range s.Tags { - tagSet[tag] = struct{}{} - } - for _, tag := range other.Tags { - tagSet[tag] = struct{}{} - } - tags := make([]string, 0, len(tagSet)) - for tag, _ := range tagSet { - tags = append(tags, tag) - } - sort.Strings(tags) - s.Tags = tags - } - - if other.Address != "" { - s.Address = other.Address - } - if s.Meta == nil { - s.Meta = other.Meta - } else { - for k, v := range other.Meta { - s.Meta[k] = v - } - } - if other.Port != 0 { - s.Port = other.Port - } - if other.Weights != nil { - s.Weights = other.Weights - } - s.EnableTagOverride = other.EnableTagOverride - if other.ProxyDestination != "" { - s.ProxyDestination = other.ProxyDestination - } - - // Take the incoming service's proxy fields and merge the config map. - proxyConf := s.Proxy.Config - s.Proxy = other.Proxy - if proxyConf == nil { - proxyConf = other.Proxy.Config - } else { - for k, v := range other.Proxy.Config { - proxyConf[k] = v - } - } - s.Proxy.Config = proxyConf - - // Just take the entire Connect block from the other node. - // We can revisit this when adding more fields to centralized config. - s.Connect = other.Connect - s.LocallyRegisteredAsSidecar = other.LocallyRegisteredAsSidecar +// IsSidecarProxy returns true if the NodeService is a sidecar proxy. +func (s *NodeService) IsSidecarProxy() bool { + return s.Kind == ServiceKindConnectProxy && s.Proxy.DestinationServiceID != "" } // Validate validates the node service configuration. diff --git a/agent/structs/structs_test.go b/agent/structs/structs_test.go index 37167e780..e407f758c 100644 --- a/agent/structs/structs_test.go +++ b/agent/structs/structs_test.go @@ -561,103 +561,6 @@ func TestStructs_NodeService_IsSame(t *testing.T) { } } -func TestStructs_NodeService_Merge(t *testing.T) { - a := &NodeService{ - Kind: "service", - ID: "foo:1", - Service: "foo", - Tags: []string{"a", "b"}, - Address: "127.0.0.1", - Meta: map[string]string{"a": "b"}, - Port: 1234, - Weights: &Weights{ - Passing: 1, - Warning: 1, - }, - EnableTagOverride: false, - ProxyDestination: "asdf", - Proxy: ConnectProxyConfig{ - DestinationServiceName: "baz", - DestinationServiceID: "baz:1", - LocalServiceAddress: "127.0.0.1", - LocalServicePort: 2345, - Config: map[string]interface{}{ - "foo": 1, - }, - }, - Connect: ServiceConnect{ - Native: false, - }, - LocallyRegisteredAsSidecar: false, - } - - b := &NodeService{ - Kind: "other", - ID: "bar:1", - Service: "bar", - Tags: []string{"c", "d"}, - Address: "127.0.0.2", - Meta: map[string]string{"c": "d"}, - Port: 4567, - Weights: &Weights{ - Passing: 2, - Warning: 2, - }, - EnableTagOverride: true, - ProxyDestination: "qwer", - Proxy: ConnectProxyConfig{ - DestinationServiceName: "zoo", - DestinationServiceID: "zoo:1", - LocalServiceAddress: "127.0.0.2", - LocalServicePort: 6789, - Config: map[string]interface{}{ - "bar": 2, - }, - }, - Connect: ServiceConnect{ - Native: true, - }, - LocallyRegisteredAsSidecar: true, - } - - expected := &NodeService{ - Kind: "other", - ID: "bar:1", - Service: "bar", - Tags: []string{"a", "b", "c", "d"}, - Address: "127.0.0.2", - Meta: map[string]string{ - "a": "b", - "c": "d", - }, - Port: 4567, - Weights: &Weights{ - Passing: 2, - Warning: 2, - }, - EnableTagOverride: true, - ProxyDestination: "qwer", - Proxy: ConnectProxyConfig{ - DestinationServiceName: "zoo", - DestinationServiceID: "zoo:1", - LocalServiceAddress: "127.0.0.2", - LocalServicePort: 6789, - Config: map[string]interface{}{ - "foo": 1, - "bar": 2, - }, - }, - Connect: ServiceConnect{ - Native: true, - }, - LocallyRegisteredAsSidecar: true, - } - - a.Merge(b) - - require.Equal(t, expected, a) -} - func TestStructs_HealthCheck_IsSame(t *testing.T) { hc := &HealthCheck{ Node: "node1", diff --git a/api/config_entry.go b/api/config_entry.go index e6f3a6ada..0c18963fd 100644 --- a/api/config_entry.go +++ b/api/config_entry.go @@ -24,15 +24,10 @@ type ConfigEntry interface { GetModifyIndex() uint64 } -type ConnectConfiguration struct { - SidecarProxy bool -} - type ServiceConfigEntry struct { Kind string Name string Protocol string - Connect ConnectConfiguration CreateIndex uint64 ModifyIndex uint64 } diff --git a/api/config_entry_test.go b/api/config_entry_test.go index a2102d574..98e72d1d0 100644 --- a/api/config_entry_test.go +++ b/api/config_entry_test.go @@ -139,7 +139,7 @@ func TestAPI_ConfigEntries(t *testing.T) { require.True(t, written) // update no cas - service.Connect.SidecarProxy = true + service.Protocol = "http" _, wm, err = config_entries.Set(service, nil) require.NoError(t, err) @@ -156,14 +156,13 @@ func TestAPI_ConfigEntries(t *testing.T) { for _, entry = range entries { switch entry.GetName() { case "foo": - // this also verfies that the update value was persisted and + // this also verifies that the update value was persisted and // the updated values are seen readService, ok = entry.(*ServiceConfigEntry) require.True(t, ok) require.Equal(t, service.Kind, readService.Kind) require.Equal(t, service.Name, readService.Name) require.Equal(t, service.Protocol, readService.Protocol) - require.Equal(t, service.Connect.SidecarProxy, readService.Connect.SidecarProxy) case "bar": readService, ok = entry.(*ServiceConfigEntry) require.True(t, ok) diff --git a/command/commands_oss.go b/command/commands_oss.go index 79f31e083..c33ffc427 100644 --- a/command/commands_oss.go +++ b/command/commands_oss.go @@ -50,7 +50,6 @@ import ( "github.com/hashicorp/consul/command/connect/ca" caget "github.com/hashicorp/consul/command/connect/ca/get" caset "github.com/hashicorp/consul/command/connect/ca/set" - connectenable "github.com/hashicorp/consul/command/connect/enable" "github.com/hashicorp/consul/command/connect/envoy" "github.com/hashicorp/consul/command/connect/proxy" "github.com/hashicorp/consul/command/debug" @@ -166,7 +165,6 @@ func init() { Register("connect ca", func(ui cli.Ui) (cli.Command, error) { return ca.New(), nil }) Register("connect ca get-config", func(ui cli.Ui) (cli.Command, error) { return caget.New(ui), nil }) Register("connect ca set-config", func(ui cli.Ui) (cli.Command, error) { return caset.New(ui), nil }) - Register("connect enable", func(ui cli.Ui) (cli.Command, error) { return connectenable.New(ui), nil }) Register("connect proxy", func(ui cli.Ui) (cli.Command, error) { return proxy.New(ui, MakeShutdownCh()), nil }) Register("connect envoy", func(ui cli.Ui) (cli.Command, error) { return envoy.New(ui), nil }) Register("debug", func(ui cli.Ui) (cli.Command, error) { return debug.New(ui, MakeShutdownCh()), nil }) diff --git a/command/connect/enable/connect_enable.go b/command/connect/enable/connect_enable.go deleted file mode 100644 index 3f4ed591d..000000000 --- a/command/connect/enable/connect_enable.go +++ /dev/null @@ -1,101 +0,0 @@ -package enable - -import ( - "flag" - "fmt" - "io" - - "github.com/hashicorp/consul/api" - "github.com/hashicorp/consul/command/flags" - "github.com/mitchellh/cli" -) - -func New(ui cli.Ui) *cmd { - c := &cmd{UI: ui} - c.init() - return c -} - -type cmd struct { - UI cli.Ui - flags *flag.FlagSet - http *flags.HTTPFlags - help string - - service string - protocol string - sidecarProxy bool - - testStdin io.Reader -} - -func (c *cmd) init() { - c.flags = flag.NewFlagSet("", flag.ContinueOnError) - c.http = &flags.HTTPFlags{} - c.flags.BoolVar(&c.sidecarProxy, "sidecar-proxy", false, "Whether the service should have a Sidecar Proxy by default") - c.flags.StringVar(&c.service, "service", "", "The service to enable connect for") - c.flags.StringVar(&c.protocol, "protocol", "", "The protocol spoken by the service") - flags.Merge(c.flags, c.http.ClientFlags()) - flags.Merge(c.flags, c.http.ServerFlags()) - c.help = flags.Usage(help, c.flags) -} - -func (c *cmd) Run(args []string) int { - if err := c.flags.Parse(args); err != nil { - return 1 - } - - if c.service == "" { - c.UI.Error("Must specify the -service parameter") - return 1 - } - - entry := &api.ServiceConfigEntry{ - Kind: api.ServiceDefaults, - Name: c.service, - Protocol: c.protocol, - Connect: api.ConnectConfiguration{ - SidecarProxy: c.sidecarProxy, - }, - } - - client, err := c.http.APIClient() - if err != nil { - c.UI.Error(fmt.Sprintf("Error connect to Consul agent: %s", err)) - return 1 - } - - written, _, err := client.ConfigEntries().Set(entry, nil) - if err != nil { - c.UI.Error(fmt.Sprintf("Error writing config entry %q / %q: %v", entry.GetKind(), entry.GetName(), err)) - return 1 - } - - if !written { - c.UI.Error(fmt.Sprintf("Config entry %q / %q not updated", entry.GetKind(), entry.GetName())) - return 1 - } - - // TODO (mkeeler) should we output anything when successful - return 0 - -} - -func (c *cmd) Synopsis() string { - return synopsis -} - -func (c *cmd) Help() string { - return flags.Usage(c.help, nil) -} - -const synopsis = "Sets some simple Connect related configuration for a service" -const help = ` -Usage: consul connect enable -service [options] - - Sets up some Connect related service defaults. - - Example: - - $ consul connect enable -service web -protocol http -sidecar-proxy true -` diff --git a/command/connect/enable/connect_enable_test.go b/command/connect/enable/connect_enable_test.go deleted file mode 100644 index bd822c6fd..000000000 --- a/command/connect/enable/connect_enable_test.go +++ /dev/null @@ -1,64 +0,0 @@ -package enable - -import ( - "testing" - - "github.com/hashicorp/consul/agent" - "github.com/hashicorp/consul/api" - "github.com/mitchellh/cli" - "github.com/stretchr/testify/require" -) - -func TestConnectEnable_noTabs(t *testing.T) { - t.Parallel() - - require.NotContains(t, New(cli.NewMockUi()).Help(), "\t") -} - -func TestConnectEnable(t *testing.T) { - t.Parallel() - - a := agent.NewTestAgent(t, t.Name(), ``) - defer a.Shutdown() - client := a.Client() - - ui := cli.NewMockUi() - c := New(ui) - - args := []string{ - "-http-addr=" + a.HTTPAddr(), - "-service=web", - "-protocol=tcp", - "-sidecar-proxy=true", - } - - code := c.Run(args) - require.Equal(t, 0, code) - - entry, _, err := client.ConfigEntries().Get(api.ServiceDefaults, "web", nil) - require.NoError(t, err) - svc, ok := entry.(*api.ServiceConfigEntry) - require.True(t, ok) - require.Equal(t, api.ServiceDefaults, svc.Kind) - require.Equal(t, "web", svc.Name) - require.Equal(t, "tcp", svc.Protocol) - require.True(t, svc.Connect.SidecarProxy) -} - -func TestConnectEnable_InvalidArgs(t *testing.T) { - t.Parallel() - - cases := map[string][]string{ - "no service": []string{}, - } - - for name, tcase := range cases { - t.Run(name, func(t *testing.T) { - ui := cli.NewMockUi() - c := New(ui) - - require.NotEqual(t, 0, c.Run(tcase)) - require.NotEmpty(t, ui.ErrorWriter.String()) - }) - } -} diff --git a/go.mod b/go.mod index 4bd357ae0..21d65112a 100644 --- a/go.mod +++ b/go.mod @@ -81,6 +81,7 @@ require ( github.com/hashicorp/vault v0.10.3 github.com/hashicorp/vault-plugin-secrets-kv v0.0.0-20190318174639-195e0e9d07f1 // indirect github.com/hashicorp/yamux v0.0.0-20180604194846-3520598351bb + github.com/imdario/mergo v0.3.6 github.com/jefferai/jsonx v0.0.0-20160721235117-9cc31c3135ee // indirect github.com/keybase/go-crypto v0.0.0-20180614160407-5114a9a81e1b // indirect github.com/kr/pretty v0.1.0 // indirect diff --git a/test/integration/connect/envoy/case-badauthz/setup.sh b/test/integration/connect/envoy/case-badauthz/setup.sh index 2e7944922..daa2b19e0 100644 --- a/test/integration/connect/envoy/case-badauthz/setup.sh +++ b/test/integration/connect/envoy/case-badauthz/setup.sh @@ -5,13 +5,7 @@ set -euo pipefail # Setup deny intention docker_consul intention create -deny s1 s2 -docker_consul connect envoy -bootstrap \ - -proxy-id s1-sidecar-proxy \ - > workdir/envoy/s1-bootstrap.json - -docker_consul connect envoy -bootstrap \ - -proxy-id s2-sidecar-proxy \ - -admin-bind 127.0.0.1:19001 \ - > workdir/envoy/s2-bootstrap.json +gen_envoy_bootstrap s1 19000 +gen_envoy_bootstrap s2 19001 export REQUIRED_SERVICES="s1 s1-sidecar-proxy s2 s2-sidecar-proxy" \ No newline at end of file diff --git a/test/integration/connect/envoy/case-basic/setup.sh b/test/integration/connect/envoy/case-basic/setup.sh index aa3030d43..bf0b444b2 100644 --- a/test/integration/connect/envoy/case-basic/setup.sh +++ b/test/integration/connect/envoy/case-basic/setup.sh @@ -2,13 +2,7 @@ set -euo pipefail -docker_consul connect envoy -bootstrap \ - -proxy-id s1-sidecar-proxy \ - > workdir/envoy/s1-bootstrap.json - -docker_consul connect envoy -bootstrap \ - -proxy-id s2-sidecar-proxy \ - -admin-bind 127.0.0.1:19001 \ - > workdir/envoy/s2-bootstrap.json +gen_envoy_bootstrap s1 19000 +gen_envoy_bootstrap s2 19001 export REQUIRED_SERVICES="s1 s1-sidecar-proxy s2 s2-sidecar-proxy" \ No newline at end of file diff --git a/test/integration/connect/envoy/case-centralconf/config_entries.hcl b/test/integration/connect/envoy/case-centralconf/config_entries.hcl new file mode 100644 index 000000000..8abf17236 --- /dev/null +++ b/test/integration/connect/envoy/case-centralconf/config_entries.hcl @@ -0,0 +1,26 @@ +enable_central_service_config = true +config_entries { + bootstrap { + kind = "proxy-defaults" + name = "global" + config { + envoy_prometheus_bind_addr = "0.0.0.0:1234" + } + } + bootstrap { + kind = "service-defaults" + name = "s1" + protocol = "http" + connect { + sidecar_proxy = true + } + } + bootstrap { + kind = "service-defaults" + name = "s2" + protocol = "http" + connect { + sidecar_proxy = true + } + } +} \ No newline at end of file diff --git a/test/integration/connect/envoy/case-centralconf/s1.hcl b/test/integration/connect/envoy/case-centralconf/s1.hcl new file mode 100644 index 000000000..7f48b1c51 --- /dev/null +++ b/test/integration/connect/envoy/case-centralconf/s1.hcl @@ -0,0 +1,16 @@ +services { + name = "s1" + port = 8080 + connect { + sidecar_service { + proxy { + upstreams = [ + { + destination_name = "s2" + local_bind_port = 5000 + } + ] + } + } + } +} \ No newline at end of file diff --git a/test/integration/connect/envoy/case-centralconf/s2.hcl b/test/integration/connect/envoy/case-centralconf/s2.hcl new file mode 100644 index 000000000..da36f6d06 --- /dev/null +++ b/test/integration/connect/envoy/case-centralconf/s2.hcl @@ -0,0 +1,17 @@ +services { + name = "s2" + port = 8181 + connect { + sidecar_service { + proxy { + config { + # We need to override this because both proxies run in same network + # namespace and so it's non-deterministic which one manages to bind + # the 1234 port first. This forces the issue here while still testing + # that s1's proxy is configured from global config. + envoy_prometheus_bind_addr = "0.0.0.0:2345" + } + } + } + } +} \ No newline at end of file diff --git a/test/integration/connect/envoy/case-centralconf/setup.sh b/test/integration/connect/envoy/case-centralconf/setup.sh new file mode 100644 index 000000000..12fa68c1d --- /dev/null +++ b/test/integration/connect/envoy/case-centralconf/setup.sh @@ -0,0 +1,9 @@ +#!/bin/bash + +set -euo pipefail + +# retry because resolving the central config might race +retry_default gen_envoy_bootstrap s1 19000 +retry_default gen_envoy_bootstrap s2 19001 + +export REQUIRED_SERVICES="s1 s1-sidecar-proxy s2 s2-sidecar-proxy" \ No newline at end of file diff --git a/test/integration/connect/envoy/case-centralconf/verify.bats b/test/integration/connect/envoy/case-centralconf/verify.bats new file mode 100644 index 000000000..ca2f2f274 --- /dev/null +++ b/test/integration/connect/envoy/case-centralconf/verify.bats @@ -0,0 +1,49 @@ +#!/usr/bin/env bats + +load helpers + +@test "s1 proxy admin is up on :19000" { + retry_default curl -f -s localhost:19000/stats -o /dev/null +} + +@test "s2 proxy admin is up on :19001" { + retry_default curl -f -s localhost:19001/stats -o /dev/null +} + +@test "s1 proxy listener should be up and have right cert" { + assert_proxy_presents_cert_uri localhost:21000 s1 + +} + +@test "s2 proxy listener should be up and have right cert" { + assert_proxy_presents_cert_uri localhost:21001 s2 +} + +@test "s1 upstream should be able to connect to s2 with http/1.1" { + run retry_default curl --http1.1 -s -f -d hello localhost:5000 + [ "$status" -eq 0 ] + [ "$output" = "hello" ] +} + +@test "s1 proxy should be exposing metrics to prometheus from central config" { + # Should have http metrics. This is just a sample one. Require the metric to + # be present not just found in a comment (anchor the regexp). + retry_default \ + must_match_in_prometheus_response localhost:1234 \ + '^envoy_http_downstream_rq_active' + + # Should be labelling with local_cluster. + retry_default \ + must_match_in_prometheus_response localhost:1234 \ + '[\{,]local_cluster="s1"[,}] ' + + # Ensure we have http metrics for public listener + retry_default \ + must_match_in_prometheus_response localhost:1234 \ + '[\{,]envoy_http_conn_manager_prefix="public_listener_http"[,}]' + + # Ensure we have http metrics for s2 upstream + retry_default \ + must_match_in_prometheus_response localhost:1234 \ + '[\{,]envoy_http_conn_manager_prefix="upstream_s2_http"[,}]' +} \ No newline at end of file diff --git a/test/integration/connect/envoy/case-dogstatsd-udp/setup.sh b/test/integration/connect/envoy/case-dogstatsd-udp/setup.sh index 414f941b2..6cb2f8306 100644 --- a/test/integration/connect/envoy/case-dogstatsd-udp/setup.sh +++ b/test/integration/connect/envoy/case-dogstatsd-udp/setup.sh @@ -2,13 +2,7 @@ set -euo pipefail -docker_consul connect envoy -bootstrap \ - -proxy-id s1-sidecar-proxy \ - > workdir/envoy/s1-bootstrap.json - -docker_consul connect envoy -bootstrap \ - -proxy-id s2-sidecar-proxy \ - -admin-bind 127.0.0.1:19001 \ - > workdir/envoy/s2-bootstrap.json +gen_envoy_bootstrap s1 19000 +gen_envoy_bootstrap s2 19001 export REQUIRED_SERVICES="s1 s1-sidecar-proxy s2 s2-sidecar-proxy fake-statsd" \ No newline at end of file diff --git a/test/integration/connect/envoy/case-grpc/setup.sh b/test/integration/connect/envoy/case-grpc/setup.sh index 414f941b2..6cb2f8306 100644 --- a/test/integration/connect/envoy/case-grpc/setup.sh +++ b/test/integration/connect/envoy/case-grpc/setup.sh @@ -2,13 +2,7 @@ set -euo pipefail -docker_consul connect envoy -bootstrap \ - -proxy-id s1-sidecar-proxy \ - > workdir/envoy/s1-bootstrap.json - -docker_consul connect envoy -bootstrap \ - -proxy-id s2-sidecar-proxy \ - -admin-bind 127.0.0.1:19001 \ - > workdir/envoy/s2-bootstrap.json +gen_envoy_bootstrap s1 19000 +gen_envoy_bootstrap s2 19001 export REQUIRED_SERVICES="s1 s1-sidecar-proxy s2 s2-sidecar-proxy fake-statsd" \ No newline at end of file diff --git a/test/integration/connect/envoy/case-http-badauthz/setup.sh b/test/integration/connect/envoy/case-http-badauthz/setup.sh index 2e7944922..daa2b19e0 100644 --- a/test/integration/connect/envoy/case-http-badauthz/setup.sh +++ b/test/integration/connect/envoy/case-http-badauthz/setup.sh @@ -5,13 +5,7 @@ set -euo pipefail # Setup deny intention docker_consul intention create -deny s1 s2 -docker_consul connect envoy -bootstrap \ - -proxy-id s1-sidecar-proxy \ - > workdir/envoy/s1-bootstrap.json - -docker_consul connect envoy -bootstrap \ - -proxy-id s2-sidecar-proxy \ - -admin-bind 127.0.0.1:19001 \ - > workdir/envoy/s2-bootstrap.json +gen_envoy_bootstrap s1 19000 +gen_envoy_bootstrap s2 19001 export REQUIRED_SERVICES="s1 s1-sidecar-proxy s2 s2-sidecar-proxy" \ No newline at end of file diff --git a/test/integration/connect/envoy/case-http/setup.sh b/test/integration/connect/envoy/case-http/setup.sh index aa3030d43..bf0b444b2 100644 --- a/test/integration/connect/envoy/case-http/setup.sh +++ b/test/integration/connect/envoy/case-http/setup.sh @@ -2,13 +2,7 @@ set -euo pipefail -docker_consul connect envoy -bootstrap \ - -proxy-id s1-sidecar-proxy \ - > workdir/envoy/s1-bootstrap.json - -docker_consul connect envoy -bootstrap \ - -proxy-id s2-sidecar-proxy \ - -admin-bind 127.0.0.1:19001 \ - > workdir/envoy/s2-bootstrap.json +gen_envoy_bootstrap s1 19000 +gen_envoy_bootstrap s2 19001 export REQUIRED_SERVICES="s1 s1-sidecar-proxy s2 s2-sidecar-proxy" \ No newline at end of file diff --git a/test/integration/connect/envoy/case-http2/setup.sh b/test/integration/connect/envoy/case-http2/setup.sh index aa3030d43..bf0b444b2 100644 --- a/test/integration/connect/envoy/case-http2/setup.sh +++ b/test/integration/connect/envoy/case-http2/setup.sh @@ -2,13 +2,7 @@ set -euo pipefail -docker_consul connect envoy -bootstrap \ - -proxy-id s1-sidecar-proxy \ - > workdir/envoy/s1-bootstrap.json - -docker_consul connect envoy -bootstrap \ - -proxy-id s2-sidecar-proxy \ - -admin-bind 127.0.0.1:19001 \ - > workdir/envoy/s2-bootstrap.json +gen_envoy_bootstrap s1 19000 +gen_envoy_bootstrap s2 19001 export REQUIRED_SERVICES="s1 s1-sidecar-proxy s2 s2-sidecar-proxy" \ No newline at end of file diff --git a/test/integration/connect/envoy/case-prometheus/setup.sh b/test/integration/connect/envoy/case-prometheus/setup.sh index aa3030d43..bf0b444b2 100644 --- a/test/integration/connect/envoy/case-prometheus/setup.sh +++ b/test/integration/connect/envoy/case-prometheus/setup.sh @@ -2,13 +2,7 @@ set -euo pipefail -docker_consul connect envoy -bootstrap \ - -proxy-id s1-sidecar-proxy \ - > workdir/envoy/s1-bootstrap.json - -docker_consul connect envoy -bootstrap \ - -proxy-id s2-sidecar-proxy \ - -admin-bind 127.0.0.1:19001 \ - > workdir/envoy/s2-bootstrap.json +gen_envoy_bootstrap s1 19000 +gen_envoy_bootstrap s2 19001 export REQUIRED_SERVICES="s1 s1-sidecar-proxy s2 s2-sidecar-proxy" \ No newline at end of file diff --git a/test/integration/connect/envoy/case-prometheus/verify.bats b/test/integration/connect/envoy/case-prometheus/verify.bats index 5e38592f7..9d8d8665f 100644 --- a/test/integration/connect/envoy/case-prometheus/verify.bats +++ b/test/integration/connect/envoy/case-prometheus/verify.bats @@ -28,17 +28,17 @@ load helpers @test "s1 proxy should be exposing metrics to prometheus" { # Should have http metrics. This is just a sample one. Require the metric to # be present not just found in a comment (anchor the regexp). - run retry_defaults \ + retry_default \ must_match_in_prometheus_response localhost:1234 \ '^envoy_http_downstream_rq_active' # Should be labelling with local_cluster. - run retry_defaults \ + retry_default \ must_match_in_prometheus_response localhost:1234 \ '[\{,]local_cluster="s1"[,}] ' # Should be labelling with http listener prefix. - run retry_defaults \ + retry_default \ must_match_in_prometheus_response localhost:1234 \ '[\{,]envoy_http_conn_manager_prefix="public_listener_http"[,}]' } \ No newline at end of file diff --git a/test/integration/connect/envoy/case-statsd-udp/setup.sh b/test/integration/connect/envoy/case-statsd-udp/setup.sh index 414f941b2..6cb2f8306 100644 --- a/test/integration/connect/envoy/case-statsd-udp/setup.sh +++ b/test/integration/connect/envoy/case-statsd-udp/setup.sh @@ -2,13 +2,7 @@ set -euo pipefail -docker_consul connect envoy -bootstrap \ - -proxy-id s1-sidecar-proxy \ - > workdir/envoy/s1-bootstrap.json - -docker_consul connect envoy -bootstrap \ - -proxy-id s2-sidecar-proxy \ - -admin-bind 127.0.0.1:19001 \ - > workdir/envoy/s2-bootstrap.json +gen_envoy_bootstrap s1 19000 +gen_envoy_bootstrap s2 19001 export REQUIRED_SERVICES="s1 s1-sidecar-proxy s2 s2-sidecar-proxy fake-statsd" \ No newline at end of file diff --git a/test/integration/connect/envoy/case-zipkin/setup.sh b/test/integration/connect/envoy/case-zipkin/setup.sh index 541bedf37..8c174c256 100644 --- a/test/integration/connect/envoy/case-zipkin/setup.sh +++ b/test/integration/connect/envoy/case-zipkin/setup.sh @@ -2,13 +2,7 @@ set -euo pipefail -docker_consul connect envoy -bootstrap \ - -proxy-id s1-sidecar-proxy \ - > workdir/envoy/s1-bootstrap.json - -docker_consul connect envoy -bootstrap \ - -proxy-id s2-sidecar-proxy \ - -admin-bind 127.0.0.1:19001 \ - > workdir/envoy/s2-bootstrap.json +gen_envoy_bootstrap s1 19000 +gen_envoy_bootstrap s2 19001 export REQUIRED_SERVICES="s1 s1-sidecar-proxy s2 s2-sidecar-proxy jaeger" \ No newline at end of file diff --git a/test/integration/connect/envoy/helpers.bash b/test/integration/connect/envoy/helpers.bash index 018cb1602..2365b1abc 100755 --- a/test/integration/connect/envoy/helpers.bash +++ b/test/integration/connect/envoy/helpers.bash @@ -95,6 +95,8 @@ function must_match_in_prometheus_response { run curl -f -s $1/metrics COUNT=$( echo "$output" | grep -Ec $2 ) + echo "OUTPUT head -n 10" + echo "$output" | head -n 10 echo "COUNT of '$2' matches: $COUNT" [ "$status" == 0 ] @@ -129,4 +131,23 @@ function must_fail_http_connection { # Should fail request with 503 echo "$output" | grep '503 Service Unavailable' +} + +function gen_envoy_bootstrap { + SERVICE=$1 + ADMIN_PORT=$2 + + if output=$(docker_consul connect envoy -bootstrap \ + -proxy-id $SERVICE-sidecar-proxy \ + -admin-bind 0.0.0.0:$ADMIN_PORT 2>&1); then + + # All OK, write config to file + echo "$output" > workdir/envoy/$SERVICE-bootstrap.json + else + status=$? + # Command failed, instead of swallowing error (printed on stdout by docker + # it seems) by writing it to file, echo it + echo "$output" + return $status + fi } \ No newline at end of file diff --git a/test/integration/connect/envoy/run-tests.sh b/test/integration/connect/envoy/run-tests.sh index 1f515d314..50e27ccd6 100755 --- a/test/integration/connect/envoy/run-tests.sh +++ b/test/integration/connect/envoy/run-tests.sh @@ -36,24 +36,36 @@ FILTER_TESTS=${FILTER_TESTS:-} LEAVE_CONSUL_UP=${LEAVE_CONSUL_UP:-} PROXY_LOGS_ON_FAIL=${PROXY_LOGS_ON_FAIL:-} -mkdir -p workdir/{consul,envoy,bats,statsd,logs} - source helpers.bash RESULT=1 CLEANED_UP=0 +PREV_CMD="" +THIS_CMD="" + function cleanup { + local STATUS="$?" + local CMD="$THIS_CMD" + if [ "$CLEANED_UP" != 0 ] ; then return fi CLEANED_UP=1 + # We failed due to set -e catching an error, output some useful info about + # that error. + echo "ERR: command exited with $STATUS" + echo " command: $CMD" + if [ -z "$LEAVE_CONSUL_UP" ] ; then docker-compose down fi } trap cleanup EXIT +# Magic to capture commands and statuses so we can show them when we exit due to +# set -e This is useful for debugging setup.sh failures. +trap 'PREV_CMD=$THIS_CMD; THIS_CMD=$BASH_COMMAND' DEBUG # Start the volume container docker-compose up -d workdir @@ -73,6 +85,8 @@ for c in ./case-*/ ; do # Wipe state docker-compose up wipe-volumes + rm -rf workdir/* + mkdir -p workdir/{consul,envoy,bats,statsd,logs} # Reload consul config from defaults cp consul-base-cfg/* workdir/consul diff --git a/vendor/modules.txt b/vendor/modules.txt index 054987c0e..0e10ee7e2 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -120,8 +120,8 @@ github.com/envoyproxy/go-control-plane/envoy/config/filter/network/http_connecti github.com/envoyproxy/go-control-plane/envoy/config/filter/network/tcp_proxy/v2 github.com/envoyproxy/go-control-plane/envoy/service/auth/v2alpha github.com/envoyproxy/go-control-plane/envoy/service/discovery/v2 -github.com/envoyproxy/go-control-plane/pkg/util github.com/envoyproxy/go-control-plane/envoy/type +github.com/envoyproxy/go-control-plane/pkg/util github.com/envoyproxy/go-control-plane/envoy/config/filter/accesslog/v2 # github.com/fatih/color v1.7.0 github.com/fatih/color diff --git a/version/version.go b/version/version.go index 654851d20..a9986e086 100644 --- a/version/version.go +++ b/version/version.go @@ -15,7 +15,7 @@ var ( // // Version must conform to the format expected by github.com/hashicorp/go-version // for tests to work. - Version = "1.4.4" + Version = "1.5.0" // A pre-release marker for the version. If this is "" (empty string) // then it means that it is a final release. Otherwise, this is a pre-release