From 33bc3a6b29b0f77c0b4d3e53a4fb55c930084812 Mon Sep 17 00:00:00 2001 From: John Howard Date: Mon, 13 May 2019 14:18:51 -0700 Subject: [PATCH 1/2] Add application ports for gateways --- .../charts/gateways/templates/deployment.yaml | 4 ++++ .../helm/istio/charts/gateways/values.yaml | 9 +++++++++ pilot/cmd/pilot-agent/main.go | 1 + pilot/cmd/pilot-agent/status/ready/probe.go | 5 ++++- pilot/cmd/pilot-agent/status/server.go | 4 ++++ pilot/cmd/pilot-agent/status/util/listeners.go | 17 ++++++++++++++--- 6 files changed, 36 insertions(+), 4 deletions(-) diff --git a/install/kubernetes/helm/istio/charts/gateways/templates/deployment.yaml b/install/kubernetes/helm/istio/charts/gateways/templates/deployment.yaml index bb33c316eaf..b51e6aed1d0 100644 --- a/install/kubernetes/helm/istio/charts/gateways/templates/deployment.yaml +++ b/install/kubernetes/helm/istio/charts/gateways/templates/deployment.yaml @@ -153,6 +153,10 @@ spec: {{- else }} - istio-pilot:15010 {{- end }} + {{- if $spec.applicationPorts }} + - --applicationPorts + - "{{ $spec.applicationPorts }}" + {{- end }} {{- end }} {{- if $.Values.global.trustDomain }} - --trust-domain={{ $.Values.global.trustDomain }} diff --git a/install/kubernetes/helm/istio/charts/gateways/values.yaml b/install/kubernetes/helm/istio/charts/gateways/values.yaml index 1284f1c02be..34460b79a39 100644 --- a/install/kubernetes/helm/istio/charts/gateways/values.yaml +++ b/install/kubernetes/helm/istio/charts/gateways/values.yaml @@ -108,6 +108,15 @@ istio-ingressgateway: secretName: istio-ingressgateway-ca-certs mountPath: /etc/istio/ingressgateway-ca-certs ### Advanced options ############ + + # Ports to explicitly check for readiness. If configured, the readiness check will expect a + # listener on these ports. A comma separated list is expected, such as "80,443". + # + # Warning: If you do not have a gateway configured for the ports provided, this check will always + # fail. This is intended for use cases where you always expect to have a listener on the port, + # such as 80 or 443 in typical setups. + applicationPorts: "" + env: # A gateway with this mode ensures that pilot generates an additional # set of clusters for internal services but without Istio mTLS, to diff --git a/pilot/cmd/pilot-agent/main.go b/pilot/cmd/pilot-agent/main.go index 3b74d8bb228..f175a22299b 100644 --- a/pilot/cmd/pilot-agent/main.go +++ b/pilot/cmd/pilot-agent/main.go @@ -339,6 +339,7 @@ var ( StatusPort: statusPort, ApplicationPorts: parsedPorts, KubeAppHTTPProbers: prober, + NodeType: role.Type, }) if err != nil { return err diff --git a/pilot/cmd/pilot-agent/status/ready/probe.go b/pilot/cmd/pilot-agent/status/ready/probe.go index 07bdd09afe7..a662652e01b 100644 --- a/pilot/cmd/pilot-agent/status/ready/probe.go +++ b/pilot/cmd/pilot-agent/status/ready/probe.go @@ -19,6 +19,8 @@ import ( multierror "github.com/hashicorp/go-multierror" + "istio.io/istio/pilot/pkg/model" + "istio.io/istio/pilot/cmd/pilot-agent/status/util" ) @@ -26,6 +28,7 @@ import ( type Probe struct { AdminPort uint16 ApplicationPorts []uint16 + NodeType model.NodeType } // Check executes the probe and returns an error is the probe fails. @@ -43,7 +46,7 @@ func (p *Probe) Check() error { // checkApplicationPorts verifies that Envoy has received configuration for all ports exposed by the application container. func (p *Probe) checkInboundConfigured() error { if len(p.ApplicationPorts) > 0 { - listeningPorts, listeners, err := util.GetInboundListeningPorts(p.AdminPort) + listeningPorts, listeners, err := util.GetInboundListeningPorts(p.AdminPort, p.NodeType) if err != nil { return err } diff --git a/pilot/cmd/pilot-agent/status/server.go b/pilot/cmd/pilot-agent/status/server.go index 8ec2ee9cd45..c0e5a31274d 100644 --- a/pilot/cmd/pilot-agent/status/server.go +++ b/pilot/cmd/pilot-agent/status/server.go @@ -28,6 +28,8 @@ import ( "syscall" "time" + "istio.io/istio/pilot/pkg/model" + "istio.io/istio/pilot/cmd/pilot-agent/status/ready" "istio.io/istio/pkg/log" @@ -63,6 +65,7 @@ type Config struct { ApplicationPorts []uint16 // KubeAppHTTPProbers is a json with Kubernetes application HTTP prober config encoded. KubeAppHTTPProbers string + NodeType model.NodeType } // Server provides an endpoint for handling status probes. @@ -81,6 +84,7 @@ func NewServer(config Config) (*Server, error) { ready: &ready.Probe{ AdminPort: config.AdminPort, ApplicationPorts: config.ApplicationPorts, + NodeType: config.NodeType, }, } if config.KubeAppHTTPProbers == "" { diff --git a/pilot/cmd/pilot-agent/status/util/listeners.go b/pilot/cmd/pilot-agent/status/util/listeners.go index be2d4dcf408..23c8e6ddcf6 100644 --- a/pilot/cmd/pilot-agent/status/util/listeners.go +++ b/pilot/cmd/pilot-agent/status/util/listeners.go @@ -21,6 +21,8 @@ import ( "strings" multierror "github.com/hashicorp/go-multierror" + + "istio.io/istio/pilot/pkg/model" ) var ( @@ -28,7 +30,7 @@ var ( ) // GetInboundListeningPorts returns a map of inbound ports for which Envoy has active listeners. -func GetInboundListeningPorts(adminPort uint16) (map[uint16]bool, string, error) { +func GetInboundListeningPorts(adminPort uint16, nodeType model.NodeType) (map[uint16]bool, string, error) { buf, err := doHTTPGet(fmt.Sprintf("http://127.0.0.1:%d/listeners", adminPort)) if err != nil { return nil, "", multierror.Prefix(err, "failed retrieving Envoy listeners:") @@ -50,8 +52,17 @@ func GetInboundListeningPorts(adminPort uint16) (map[uint16]bool, string, error) } // Before checking if listener is local, removing port portion of the address ipAddr := strings.TrimSuffix(l, ":"+ipAddrParts[len(ipAddrParts)-1]) - if !isLocalListener(ipAddr) { - continue + + switch nodeType { + // For gateways, we will not listen on a local host, instead on 0.0.0.0 + case model.Router: + if ipAddr != "0.0.0.0" { + continue + } + default: + if !isLocalListener(ipAddr) { + continue + } } portStr := ipAddrParts[len(ipAddrParts)-1] From 72e102caaa904a1dbbba1f0946465cd9d76e7a8b Mon Sep 17 00:00:00 2001 From: John Howard Date: Mon, 13 May 2019 15:14:11 -0700 Subject: [PATCH 2/2] Check all ports for readiness --- pilot/cmd/pilot-agent/status/ready/probe.go | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/pilot/cmd/pilot-agent/status/ready/probe.go b/pilot/cmd/pilot-agent/status/ready/probe.go index a662652e01b..53156a24c1a 100644 --- a/pilot/cmd/pilot-agent/status/ready/probe.go +++ b/pilot/cmd/pilot-agent/status/ready/probe.go @@ -56,11 +56,14 @@ func (p *Probe) checkInboundConfigured() error { // confuration in Envoy. The CDS/LDS updates will contain everything, so just ensuring at least one port has // been configured should be sufficient. for _, appPort := range p.ApplicationPorts { - if listeningPorts[appPort] { + if listeningPorts[appPort] && p.NodeType != model.Router { // Success - Envoy is configured. + // For gateways we should check for all ports though, so don't return success yet. return nil } - err = multierror.Append(err, fmt.Errorf("envoy missing listener for inbound application port: %d", appPort)) + if !listeningPorts[appPort] { + err = multierror.Append(err, fmt.Errorf("envoy missing listener for inbound application port: %d", appPort)) + } } if err != nil { return multierror.Append(fmt.Errorf("failed checking application ports. listeners=%s", listeners), err)