knative-extensions / net-istio

A Knative ingress controller for Istio.
Apache License 2.0
73 stars 88 forks source link

Networking conformance tests fail in mesh mode #549

Open nickjameswebb opened 3 years ago

nickjameswebb commented 3 years ago

Expected Behavior

Networking conformance tests pass for a Knative Serving + Istio + net-istio (mesh mode enabled) cluster.

Actual Behavior

This test fails with an error like:

util.go:535: Attempt 0 creating service failed with: Service "ingress-conformance-0-visibility-psziwmpb" is invalid: spec.externalName: Required value
visibility.go:84: Error creating Service "serving-tests/ingress-conformance-0-visibility-psziwmpb": Service "ingress-conformance-0-visibility-psziwmpb" is invalid: spec.externalName: Required value

Steps to Reproduce the Problem

  1. On a new cluster, install Knative Serving, Istio (with sidecar injection enabled), and net-istio.
  2. Enable mesh mode via this property in configmap/config-istio.
  3. Run the networking conformance tests.

Additional Info

I have seen this fail with two different configurations:

  1. Knative 0.21.0, Istio 1.9.1
  2. Knative 0.20.0, Istio 1.7.3

Let me know if this issue should be filed with the networking conformance tests. It doesn't seem like this test will ever succeed because the generated private hostname will never resolve to anything while in mesh mode (the spec.ExternalName error is just covering up this real issue).

Thanks very much!

JRBANCEL commented 3 years ago

/assign

JRBANCEL commented 3 years ago

The test is trying to create a proxy service to test the visibility and that service proxies points to an ExternalName k8s Service pointing to the local gateway defined in ingress.Status.PrivateLoadBalancer.Ingress[0].DomainInternal. In the case pure mesh, this field doesn't exists so the test tries to create a invalid service (empty ExternalName):

privateLoadBalancer:
  ingress:
  - meshOnly: true

In the case of pure mesh, we don't need the ExternalName Service pointing to the (non-existent) local gateway. The test needs to be tweaked to take this into account.

nickjameswebb commented 3 years ago

That makes sense. I've tried removing the call to create the ExternalName service, and the result is a DNS lookup error for the generated private hostnames in the test like:

lookup ingress-conformance-0-visibility-cpjkeznv.serving-tests.svc.cluster.local on 10.96.0.10:53: no such host

It seems like fixing this test will require some additional DNS configuration or creating an Istio ServiceEntry for the private hostname.

dprotaso commented 3 years ago

Note: I've disabled conformance tests in mesh mode in #583

We'll want to re-enable those once this is fixed

github-actions[bot] commented 3 years ago

This issue is stale because it has been open for 90 days with no activity. It will automatically close after 30 more days of inactivity. Reopen the issue with /reopen. Mark the issue as fresh by adding the comment /remove-lifecycle stale.

nak3 commented 1 year ago

Just note, we still haven't enabled conformance test on mesh mode. However, ambient can pass all conformance test.

github-actions[bot] commented 1 year ago

This issue is stale because it has been open for 90 days with no activity. It will automatically close after 30 more days of inactivity. Reopen the issue with /reopen. Mark the issue as fresh by adding the comment /remove-lifecycle stale.

dprotaso commented 11 months ago

/reopen /lifecycle frozen

knative-prow[bot] commented 11 months ago

@dprotaso: Reopened this issue.

In response to [this](https://github.com/knative-extensions/net-istio/issues/549#issuecomment-1793612016): >/reopen >/lifecycle frozen Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.
dprotaso commented 10 months ago

/unassign @JRBANCEL

skonto commented 5 months ago

@ReToCode @dprotaso is this still valid given that the mesh option was removed in https://github.com/knative-extensions/net-istio/pull/915? Am I missing something here?

dprotaso commented 5 months ago

I think the repro steps are not the same but mesh is still pretty flakey with serving tests

https://testgrid.k8s.io/r/knative-own-testgrid/serving#istio-latest-mesh

We should investigate