Skip to content

Commit

Permalink
Fix probes for internal addresses when using one contour (#310)
Browse files Browse the repository at this point in the history 
* Use ports from Endpoints when probing, rather than making up constants

* Fix lint / gofmt

* Fix port detection for Istio

* Add notes on running istio e2e kind test
  • Loading branch information
Evan Anderson authored Jun 24, 2022
1 parent dd191c1 commit 207051d
Show file tree
Hide file tree
Showing 3 changed files with 121 additions and 85 deletions.
94 changes: 39 additions & 55 deletions pkg/reconciler/ingress/lister.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"context"
"fmt"
"net/url"
"strconv"

"go.uber.org/zap"
"k8s.io/apimachinery/pkg/util/sets"
Expand All @@ -30,15 +31,6 @@ import (
"knative.dev/net-gateway-api/pkg/reconciler/ingress/config"
)

const (
// HTTPPortExternal is the port for external availability.
HTTPPortExternal = "8080"
// HTTPPortInternal is the port for internal availability.
HTTPPortInternal = "8081"
// HTTPSPortExternal is the port for external HTTPS availability.
HTTPSPortExternal = "8443"
)

func NewProbeTargetLister(logger *zap.SugaredLogger, endpointsLister corev1listers.EndpointsLister) status.ProbeTargetLister {
return &gatewayPodTargetLister{
logger: logger,
Expand All @@ -52,69 +44,61 @@ type gatewayPodTargetLister struct {
}

func (l *gatewayPodTargetLister) ListProbeTargets(ctx context.Context, ing *v1alpha1.Ingress) ([]status.ProbeTarget, error) {
privateIPs, err := l.endpointIPs(ctx, v1alpha1.IngressVisibilityClusterLocal)
if err != nil {
return nil, err
}
publicIPs, err := l.endpointIPs(ctx, v1alpha1.IngressVisibilityExternalIP)
if err != nil {
return nil, err
result := make([]status.ProbeTarget, 0, len(ing.Spec.Rules))
for _, rule := range ing.Spec.Rules {
eps, err := l.getRuleProbes(ctx, rule, ing.Spec.HTTPOption)
if err != nil {
return nil, err
}
result = append(result, eps...)
}

return l.getIngressUrls(ing, privateIPs, publicIPs)
return result, nil
}

func (l *gatewayPodTargetLister) endpointIPs(ctx context.Context, visibility v1alpha1.IngressVisibility) (sets.String, error) {
func (l *gatewayPodTargetLister) getRuleProbes(ctx context.Context, rule v1alpha1.IngressRule, sslOpt v1alpha1.HTTPOption) ([]status.ProbeTarget, error) {
gatewayConfig := config.FromContext(ctx).Gateway
service := gatewayConfig.Gateways[visibility].Service
service := gatewayConfig.Gateways[rule.Visibility].Service

eps, err := l.endpointsLister.Endpoints(service.Namespace).Get(service.Name)
if err != nil {
return nil, fmt.Errorf("failed to get endpoints: %w", err)
}

readyIPs := sets.NewString()
targets := make([]status.ProbeTarget, 0, len(eps.Subsets))
foundTargets := 0
for _, sub := range eps.Subsets {
for _, address := range sub.Addresses {
readyIPs.Insert(address.IP)
}
}
if len(readyIPs) == 0 {
return nil, fmt.Errorf("no gateway pods available")
}
return readyIPs, nil
}

func (l *gatewayPodTargetLister) getIngressUrls(ing *v1alpha1.Ingress, privateIPs, publicIPs sets.String) ([]status.ProbeTarget, error) {

targets := make([]status.ProbeTarget, 0, len(ing.Spec.Rules))
for _, rule := range ing.Spec.Rules {
var target status.ProbeTarget

domains := rule.Hosts
scheme := "http"

if rule.Visibility == v1alpha1.IngressVisibilityExternalIP {
target = status.ProbeTarget{
PodIPs: publicIPs,
}
if ing.Spec.HTTPOption == v1alpha1.HTTPOptionRedirected {
target.PodPort = HTTPSPortExternal
target.URLs = domainsToURL(domains, "https")
} else {
target.PodPort = HTTPPortExternal
target.URLs = domainsToURL(domains, scheme)
// Istio uses "http2" for the http port
matchSchemes := sets.NewString("http", "http2")
if rule.Visibility == v1alpha1.IngressVisibilityExternalIP && sslOpt == v1alpha1.HTTPOptionRedirected {
scheme = "https"
matchSchemes = sets.NewString("https")
}
pt := status.ProbeTarget{PodIPs: sets.NewString()}

portNumber := sub.Ports[0].Port
for _, port := range sub.Ports {
if matchSchemes.Has(port.Name) {
// Prefer to match the name exactly
portNumber = port.Port
break
}
} else {
target = status.ProbeTarget{
PodIPs: privateIPs,
PodPort: HTTPPortInternal,
URLs: domainsToURL(domains, scheme),
if port.AppProtocol != nil && matchSchemes.Has(*port.AppProtocol) {
portNumber = port.Port
}
}
pt.PodPort = strconv.Itoa(int(portNumber))

targets = append(targets, target)
for _, address := range sub.Addresses {
pt.PodIPs.Insert(address.IP)
}
foundTargets += len(pt.PodIPs)

pt.URLs = domainsToURL(rule.Hosts, scheme)
targets = append(targets, pt)
}
if foundTargets == 0 {
return nil, fmt.Errorf("no gateway pods available")
}
return targets, nil
}
Expand Down
105 changes: 75 additions & 30 deletions pkg/reconciler/ingress/lister_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ func TestListProbeTargets(t *testing.T) {
objects: []runtime.Object{
publicEndpointsOneAddr,
},
ing: ing(withBasicSpec, withGatewayAPIClass),
ing: ing(withBasicSpec, withInternalSpec, withGatewayAPIClass),
wantErr: fmt.Errorf("failed to get endpoints: endpoints %q not found", privateName),
}, {
name: "no external endpoint to probe",
Expand All @@ -87,7 +87,7 @@ func TestListProbeTargets(t *testing.T) {
privateEndpointsNoAddr,
publicEndpointsOneAddr,
},
ing: ing(withBasicSpec, withGatewayAPIClass),
ing: ing(withBasicSpec, withInternalSpec, withGatewayAPIClass),
wantErr: fmt.Errorf("no gateway pods available"),
}, {
name: "external endpoint without address to probe",
Expand All @@ -101,7 +101,7 @@ func TestListProbeTargets(t *testing.T) {
name: "endpoint with single address to probe (https redirected)",
objects: []runtime.Object{
privateEndpointsOneAddr,
publicEndpointsOneAddr,
publicSslEndpointsOneAddr,
},
ing: ing(withBasicSpec, withGatewayAPIClass, withHTTPOption(v1alpha1.HTTPOptionRedirected)),
want: []status.ProbeTarget{{
Expand All @@ -120,15 +120,24 @@ func TestListProbeTargets(t *testing.T) {
publicEndpointsMultiAddrMultiSubset,
},
ing: ing(withBasicSpec, withGatewayAPIClass),
want: []status.ProbeTarget{{
PodIPs: sets.NewString("2.3.4.5", "3.4.5.6", "4.3.2.1"),
PodPort: "8080",
URLs: []*url.URL{{
Scheme: "http",
Host: "example.com",
Path: "/",
want: []status.ProbeTarget{
{
PodIPs: sets.NewString("2.3.4.5"),
PodPort: "1234",
URLs: []*url.URL{{
Scheme: "http",
Host: "example.com",
Path: "/",
}},
}, {
PodIPs: sets.NewString("3.4.5.6", "4.3.2.1"),
PodPort: "4321",
URLs: []*url.URL{{
Scheme: "http",
Host: "example.com",
Path: "/",
}},
}},
}},
}}

for _, test := range tests {
Expand Down Expand Up @@ -189,6 +198,22 @@ var (
}},
}

publicSslEndpointsOneAddr = &corev1.Endpoints{
ObjectMeta: metav1.ObjectMeta{
Namespace: testNamespace,
Name: publicName,
},
Subsets: []corev1.EndpointSubset{{
Ports: []corev1.EndpointPort{{
Name: "http",
Port: 8443,
}},
Addresses: []corev1.EndpointAddress{{
IP: "1.2.3.4",
}},
}},
}

privateEndpointsMultiAddrMultiSubset = &corev1.Endpoints{
ObjectMeta: metav1.ObjectMeta{
Namespace: testNamespace,
Expand All @@ -204,8 +229,11 @@ var (
}},
}, {
Ports: []corev1.EndpointPort{{
Name: "asdf",
Name: "http2",
Port: 4321,
}, {
Name: "admin",
Port: 1337,
}},
Addresses: []corev1.EndpointAddress{{
IP: "3.4.5.6",
Expand Down Expand Up @@ -266,25 +294,42 @@ var (
)

func withBasicSpec(i *v1alpha1.Ingress) {
i.Spec = v1alpha1.IngressSpec{
HTTPOption: v1alpha1.HTTPOptionEnabled,
Rules: []v1alpha1.IngressRule{{
Hosts: []string{"example.com"},
Visibility: v1alpha1.IngressVisibilityExternalIP,
HTTP: &v1alpha1.HTTPIngressRuleValue{
Paths: []v1alpha1.HTTPIngressPath{{
Splits: []v1alpha1.IngressBackendSplit{{
IngressBackend: v1alpha1.IngressBackend{
ServiceName: "goo",
ServiceNamespace: i.Namespace,
ServicePort: intstr.FromInt(123),
},
Percent: 100,
}},
i.Spec.HTTPOption = v1alpha1.HTTPOptionEnabled
i.Spec.Rules = append(i.Spec.Rules, v1alpha1.IngressRule{
Hosts: []string{"example.com"},
Visibility: v1alpha1.IngressVisibilityExternalIP,
HTTP: &v1alpha1.HTTPIngressRuleValue{
Paths: []v1alpha1.HTTPIngressPath{{
Splits: []v1alpha1.IngressBackendSplit{{
IngressBackend: v1alpha1.IngressBackend{
ServiceName: "goo",
ServiceNamespace: i.Namespace,
ServicePort: intstr.FromInt(123),
},
Percent: 100,
}},
},
}},
}
}},
},
})
}

func withInternalSpec(i *v1alpha1.Ingress) {
i.Spec.Rules = append(i.Spec.Rules, v1alpha1.IngressRule{
Hosts: []string{"foo.svc", "foo.svc.cluster.local"},
Visibility: v1alpha1.IngressVisibilityClusterLocal,
HTTP: &v1alpha1.HTTPIngressRuleValue{
Paths: []v1alpha1.HTTPIngressPath{{
Splits: []v1alpha1.IngressBackendSplit{{
IngressBackend: v1alpha1.IngressBackend{
ServiceName: "goo",
ServiceNamespace: i.Namespace,
ServicePort: intstr.FromInt(124),
},
Percent: 100,
}},
}},
},
})
}

type IngressOption func(*v1alpha1.Ingress)
Expand Down
7 changes: 7 additions & 0 deletions test/kind-e2e-istio.sh
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,13 @@ set -euo pipefail

source $(dirname $0)/../hack/test-env.sh

# Expected prerequisites:
# * A Kind cluster
# * Knative serving installed
# * Gateway CRDs installed
# * net-gateway-api-controller installed (e.g. `ko apply -f config`)
# * Test images published with `./test/upload-test-images.sh`

CONTROL_NAMESPACE=knative-serving
IPS=( $(kubectl get nodes -lkubernetes.io/hostname!=kind-control-plane -ojsonpath='{.items[*].status.addresses[?(@.type=="InternalIP")].address}') )
CLUSTER_SUFFIX=${CLUSTER_SUFFIX:-cluster.local}
Expand Down

0 comments on commit 207051d

Please sign in to comment.