consul/connect: add initial support for ingress gateways #8709

shoenig · 2020-08-21T21:25:42Z

This PR adds initial support for running Consul Connect Ingress Gateways (CIGs) in Nomad. These gateways are declared as part of a task group level service definition within the connect stanza.

service {
  connect {
    gateway {
      proxy {
        // envoy proxy configuration
      }
      ingress {
        // ingress-gateway configuration entry
      }
    }
  }
}

A gateway can be run in bridge or host networking mode, with the caveat that host networking necessitates manually specifying the Envoy admin listener (which cannot be disabled) via the service port value.

Currently Envoy is the only supported gateway implementation in Consul, and Nomad only supports running Envoy as a gateway using the docker driver.

When the gateway.ingress field is set, Nomad will write/update the Configuration Entry into Consul on job submission. Because Configurations are global in Consul scope and there may be more than one Nomad cluster communicating with Consul, there is an assumption that any ingress gateway defined in Nomad for a particular service will be the same among Nomad clusters. Consul may provide a mechanism for more fine-grained control over Configuration Entries in the future.

Gateways require Consul 1.8.0+, checked by an additional constraint.

Aims to address #8294 and tangentially #8647

This PR adds initial support for running Consul Connect Ingress Gateways (CIGs) in Nomad. These gateways are declared as part of a task group level service definition within the connect stanza. ```hcl service { connect { gateway { proxy { // envoy proxy configuration } ingress { // ingress-gateway configuration entry } } } } ``` A gateway can be run in `bridge` or `host` networking mode, with the caveat that host networking necessitates manually specifying the Envoy admin listener (which cannot be disabled) via the service port value. Currently Envoy is the only supported gateway implementation in Consul, and Nomad only supports running Envoy as a gateway using the docker driver. Aims to address #8294 and tangentially #8647

shoenig · 2020-08-21T21:29:18Z

Still TODO

e2e testing
~~confirm consul ACLs interop~~
~~add examples to nomad-connect-demos~~
- https://github.com/hashicorp/nomad-connect-examples/pull/6/files
changelog

schmichael

Will finish up tomorrow

schmichael · 2020-08-24T21:31:14Z

website/pages/docs/job-specification/gateway.mdx

+!> **Warning:** There is no way to disable the Envoy admin interface, which will be
+accessible to any workload running on the same Nomad client. The admin interface exposes
+information about the proxy, including a Consul Service Identity token if Consul ACLs
+are enabled.


If we can use the "pick a random port and write it to a file" trick then we can mention that filename here in a note instead of this big warning.

schmichael · 2020-08-24T22:09:37Z

client/allocrunner/taskrunner/envoybootstrap_hook.go

+		for _, service := range tg.Services {
+			if service.Name == serviceName {
+				_, port = tg.Networks.Port(service.PortLabel)
+				break
+			}
+		}


Since this is bound on localhost we don't have to reserve a port and instead could bind to 127.0.0.1:0, let the OS pick a free port, and have Envoy write that to a file with -admin-address-path: https://www.envoyproxy.io/docs/envoy/latest/operations/cli#cmdoption-admin-address-path

client/client.go

schmichael · 2020-08-24T22:28:49Z

command/agent/job_endpoint.go

+		return nil
+	}
+
+	var bindAddresses map[string]*structs.ConsulGatewayBindAddress = nil


Suggested change

var bindAddresses map[string]*structs.ConsulGatewayBindAddress = nil

var bindAddresses map[string]*structs.ConsulGatewayBindAddress

schmichael · 2020-08-24T22:35:13Z

nomad/consul.go

+	c.lock.Unlock()
+
+	if stopped {
+		return errors.New("client stopped and may not longer create config entries")


~~Is there a risk of this getting emitted at shutdown?~~

~~I can try to determine that but need to stop for the day.~~

Update: Ah, it will get emitted to the remote caller which is useful! I was worried it would spew error messages to the agent's log at shutdown which gets scary and confusing for operators.

nickethier

I think the implementation here looks solid!

nickethier · 2020-08-24T18:26:19Z

client/allocrunner/taskrunner/envoybootstrap_hook.go

@@ -288,9 +388,16 @@ func (e envoyBootstrapArgs) args() []string {
 		"envoy",
 		"-grpc-addr", e.grpcAddr,
 		"-http-addr", e.consulConfig.HTTPAddr,
-		"-admin-bind", e.envoyAdminBind,
+		"-admin-bind", e.envoyAdminBind, // bleh


Suggested change

"-admin-bind", e.envoyAdminBind, // bleh

"-admin-bind", e.envoyAdminBind,

client/client.go

nickethier · 2020-08-25T14:22:57Z

nomad/job_endpoint.go

+	//
+	// Every job update will re-write the Configuration Entry into Consul.
+	for service, entry := range args.Job.ConfigEntries() {
+		ctx := context.Background()


Should this be a WithTimeout? I guess the consul client may already have a built in timeout, just thinking about blocking job submission forever if Consul is not responding.

Yeah, even if it's high (30 seconds? 1 minute?), dying with a 500 seems preferable to hanging until the user presses Ctrl-C and wonders what state things are in.

nickethier · 2020-08-25T15:01:13Z

nomad/leader_test.go

 	var consulACLsAPI mockConsulACLsAPI
 	s1.consulACLs = &consulACLsAPI

+	// replace consul Config API with a mock for tracking calls in tests
+	// var consulConfigsAPI mock


might be able to remove this

nickethier · 2020-08-25T15:03:50Z

nomad/structs/services.go

@@ -981,6 +1023,15 @@ func (p *ConsulProxy) Copy() *ConsulProxy {
 	return newP
 }

+// opaqueMapsEqual compares map[string]interface{} commonly used for opaque
+//// config blocks. Interprets nil and {} as the same.


Suggested change

//// config blocks. Interprets nil and {} as the same.

// config blocks. Interprets nil and {} as the same.

Required by usage of t.Cleanup introduced in hashicorp/nomad#8709

waszi · 2020-09-23T12:17:41Z

I have tried to setup ingress gateway with wildcard, but I get error:

job "ingress" {
  datacenters = ["dc1"]

  constraint {
    attribute = "${attr.unique.hostname}"
    operator = "regexp"
    value = "^lb0[0-9]$"
  }

  group "ingress-group" {
    network {
      mode = "bridge"
      port "inbound" {
        static = 8080
      }
    }

    service {
      name = "ingress-service"
      port = "8080"

      connect {
        gateway {
          proxy {
            connect_timeout = "500ms"
          }
          ingress {
            tls {
              enabled = false
            }
            listener {
              port = 8080
              protocol = "http"
              service {
                name = "*"
              }
            }
          }
        }
      }
    }
  }
}

Error submitting job: Unexpected response code: 500 (rpc error: rpc error: 1 error occurred:
	* Task group ingress-group validation failed: 1 error occurred:
	* Task group service validation failed: 1 error occurred:
	* Service[0] ingress-service validation failed: 1 error occurred:
	* Consul Ingress Service requires one or more hosts when using HTTP protocol

My goal is to have similar configuration to traefik/fabio.

"Export" everything using wildcard SERVICE.mydomain.com -> SERVICE
or
Configure each service individually but configuration should be in each job separately.

shoenig · 2020-11-25T16:11:24Z

Hi @waszi, sorry I just noticed this. We're you able to resolve the problem? Or if not, could you open a new issue to report it?

github-actions · 2022-12-08T02:18:27Z

I'm going to lock this pull request because it has been closed for 120 days ⏳. This helps our maintainers find and focus on the active contributions.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

consul/connect: update consul version used in circleci

7b3f77d

shoenig force-pushed the f-cc-ingress branch 2 times, most recently from 599f064 to 173fedf Compare August 24, 2020 13:43

consul/connect: remove envoy dns option from gateway proxy config

36a743f

shoenig force-pushed the f-cc-ingress branch from 173fedf to 36a743f Compare August 24, 2020 14:12

shoenig requested review from nickethier and schmichael August 24, 2020 14:42

shoenig marked this pull request as ready for review August 24, 2020 14:42

shoenig mentioned this pull request Aug 24, 2020

Consul v1.7.7 #8715

Merged

consul/connect: update consul to match current

f4ea3a5

shoenig force-pushed the f-cc-ingress branch from e322289 to f4ea3a5 Compare August 24, 2020 15:48

consul/connect: fixup tests to use new consul sdk

db8020f

shoenig force-pushed the f-cc-ingress branch from 83af3b8 to db8020f Compare August 24, 2020 17:02

schmichael reviewed Aug 24, 2020

View reviewed changes

nickethier approved these changes Aug 25, 2020

View reviewed changes

shoenig added 3 commits August 26, 2020 13:17

consul/connect: fixup some comments and context timeout

abd38b3

consul/connect: use context cancel more safely

1dca7eb

Merge branch 'master' into f-cc-ingress

a46ba7f

shoenig merged commit 48ae1de into master Aug 26, 2020

shoenig deleted the f-cc-ingress branch August 26, 2020 20:50

mikemorris added a commit to hashicorp/consul that referenced this pull request Sep 10, 2020

ci: pin Nomad master integration test to golang:1.14

d3eefa3

Required by usage of t.Cleanup introduced in hashicorp/nomad#8709

mikemorris added a commit to hashicorp/consul that referenced this pull request Sep 10, 2020

ci: pin Nomad master integration test to golang:1.14

582d7ef

Required by usage of t.Cleanup introduced in hashicorp/nomad#8709

This was referenced Sep 10, 2020

ci: pin Nomad integration test to Go 1.14 for Consul 1.7.x hashicorp/consul#8652

Merged

ci: pin Nomad integration test to Go 1.14 for Consul 1.6.x hashicorp/consul#8653

Merged

mikemorris added a commit to hashicorp/consul that referenced this pull request Sep 10, 2020

ci: pin Nomad master integration test to golang:1.14

08b3548

Required by usage of t.Cleanup introduced in hashicorp/nomad#8709

mikemorris added a commit to hashicorp/consul that referenced this pull request Sep 10, 2020

ci: pin Nomad master integration test to golang:1.14

e1b4afd

Required by usage of t.Cleanup introduced in hashicorp/nomad#8709

shoenig mentioned this pull request Nov 25, 2020

[feature] Add support (and documentation) for consul ingress gateways #8294

Closed

github-actions bot locked as resolved and limited conversation to collaborators Dec 8, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

consul/connect: add initial support for ingress gateways #8709

consul/connect: add initial support for ingress gateways #8709

shoenig commented Aug 21, 2020 •

edited

Loading

shoenig commented Aug 21, 2020 •

edited

Loading

schmichael left a comment

schmichael Aug 24, 2020

schmichael Aug 24, 2020

schmichael Aug 24, 2020

schmichael Aug 24, 2020 •

edited

Loading

nickethier left a comment

nickethier Aug 24, 2020

nickethier Aug 25, 2020

schmichael Aug 25, 2020

nickethier Aug 25, 2020

nickethier Aug 25, 2020

waszi commented Sep 23, 2020 •

edited

Loading

shoenig commented Nov 25, 2020

github-actions bot commented Dec 8, 2022

	var bindAddresses map[string]*structs.ConsulGatewayBindAddress = nil
	var bindAddresses map[string]*structs.ConsulGatewayBindAddress

	"-admin-bind", e.envoyAdminBind, // bleh
	"-admin-bind", e.envoyAdminBind,

	//// config blocks. Interprets nil and {} as the same.
	// config blocks. Interprets nil and {} as the same.

consul/connect: add initial support for ingress gateways #8709

consul/connect: add initial support for ingress gateways #8709

Conversation

shoenig commented Aug 21, 2020 • edited Loading

shoenig commented Aug 21, 2020 • edited Loading

schmichael left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

schmichael Aug 24, 2020 • edited Loading

Choose a reason for hiding this comment

nickethier left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

waszi commented Sep 23, 2020 • edited Loading

shoenig commented Nov 25, 2020

github-actions bot commented Dec 8, 2022

shoenig commented Aug 21, 2020 •

edited

Loading

shoenig commented Aug 21, 2020 •

edited

Loading

schmichael Aug 24, 2020 •

edited

Loading

waszi commented Sep 23, 2020 •

edited

Loading