Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Knative operator looks for "istio-system" namespace when custom istio gateway is set #565

Closed
jakoberpf opened this issue Apr 28, 2021 · 9 comments · Fixed by #590
Closed
Labels
kind/bug Categorizes issue or PR as related to a bug.

Comments

@jakoberpf
Copy link

jakoberpf commented Apr 28, 2021

Describe the bug
When we deploy the knative operator and set the custom knative-ingress-gateway to another gateway and namespace, the knative serving controller still looks for the istio-system namespace and fails to get ready.

operator log: failed to apply non rbac manifest: namespaces \"istio-system\" not found

describe of KnativeServing: Install failed with message: namespaces "istio-system" not found

Expected behavior
When setting the custom gateway, knative doesn't look for the gateway in istio-system, but the defined namespace for example servicemesh.

To Reproduce
Deploy istio in custom namespace and knative with the following configs.

Knative release version
Knative Operator: v0.21.0 - v0.22.0

Additional context
Serving Configuration:

apiVersion: operator.knative.dev/v1alpha1
kind: KnativeServing
metadata:
  name: platform-knative-serving
spec:
  version: 0.22.0
  config:
    domain:
    istio:
      gateway.knative-serving.knative-ingress-gateway: "app-gateway.servicemesh.svc.cluster.local"
  ingress:
    istio:
      enabled: true
      knative-ingress-gateway:
        selector:
          istio: custom-gateway

If we have an obviously misconfiguration please point me out...

@jakoberpf jakoberpf added the kind/bug Categorizes issue or PR as related to a bug. label Apr 28, 2021
@jakoberpf jakoberpf changed the title Knative operator looks for "istio-system" namespace when custom istion gateway is set Knative operator looks for "istio-system" namespace when custom istio gateway is set Apr 28, 2021
@nak3
Copy link
Contributor

nak3 commented Apr 30, 2021

I think it is bug in https://github.com/knative/operator/blob/main/pkg/reconciler/knativeserving/common/ingress_service.go#L28.
Perhaps creating a istio-system namespace could be a workaround? (operator will deploy knative-local-gateway svc though.)

@jakoberpf
Copy link
Author

jakoberpf commented May 3, 2021

@nak3 Creating the istio-system namespace created removed the error and created the knative-local-gateway as you predicted. The code you provided also tells me that the istio-system namespace is hardcoded and therefore can be changed?

Furthermore, when I read the documentation right, we can set the local cluster gateway, but this then also just works for the istio-namespace if I understood that right.

Although the original error is now gone the routes created by the operator for the "functions" now show

    Last Transition Time:  2021-05-03T06:33:41Z
    Message:               Ingress reconciliation failed
    Reason:                ReconcileIngressFailed
    Status:                Unknown
    Type:                  IngressReady
    Last Transition Time:  2021-05-03T06:33:41Z
    Message:               Ingress reconciliation failed
    Reason:                ReconcileIngressFailed
    Status:                Unknown
    Type:                  Ready

which looks similar to Issue 9727

The error ReconcileIngressFailed can be here found in the operator code.

@houshengbo
Copy link
Contributor

houshengbo commented May 3, 2021

@nak3
The line https://github.com/knative/operator/blob/main/pkg/reconciler/knativeserving/common/ingress_service.go#L28 may NOT be the reason leading to the issue, because this line only makes sure the Service named knative-local-gateway is installed under the default ns istio-system.

If I remember well, the Service should look for the ns for the istio, but the gateways are installed under the ns of the knative-serving CR.

However, if istio is installed under the non-default ns, knative operator will fail to configure, because this hardcoded istio-system.

Maybe, net-istio should remove this service?

apiVersion: v1
kind: Service
metadata:
  name: knative-local-gateway
  namespace: istio-system
  labels:
    serving.knative.dev/release: "v0.22.1"
    networking.knative.dev/ingress-provider: istio
spec:
  type: ClusterIP
  selector:
    istio: ingressgateway
  ports:
    - name: http2
      port: 80
      targetPort: 8081

@jakoberpf Your understanding of the doc is correct, would you do me a favor by verifying if the custom gateway works with the istio installed under "istio-system"? Let's first see if it works with the default ns.

@jakoberpf
Copy link
Author

jakoberpf commented May 3, 2021

@houshengbo
Right, but it means that there must be the default istio-system ns in the cluster for the operator work? So the operator is not build to be used with a non default istio-system ns?

We began testing our operator configuration with a default istio install, which it did work with. We only then encountered these network issues when deploying on a running cluster with a custom istio namespace. So to my current knowledge the custom namespace induced this issue.

@houshengbo
Copy link
Contributor

houshengbo commented May 3, 2021

@jakoberpf It should work with non-default ns of istio. We really need to fix this issue.
I suspect it is caused by this resource in net-istio:

apiVersion: v1
kind: Service
metadata:
  name: knative-local-gateway
  namespace: istio-system
  labels:
    serving.knative.dev/release: "v0.22.1"
    networking.knative.dev/ingress-provider: istio
spec:
  type: ClusterIP
  selector:
    istio: ingressgateway
  ports:
    - name: http2
      port: 80
      targetPort: 8081

We used to install this resource also under the ns of the knative-serving CR, but later we have to pin it to the istio-system ns(cannot remember the reason, but I will take a look). However, if istio installs under a random ns, how can knative operator know it? Maybe the knative serving CR need a field to get the istio ns? I am also thinking of whether the above resource can stay out of the net-istio.

@jakoberpf
Copy link
Author

jakoberpf commented May 3, 2021

@houshengbo I actually was looking for a field on the serving cr to point the operator to the namespace where we istio is running. So the idea to add this seems natural to me.

Should I provide additional logs to further define the cause?

@houshengbo
Copy link
Contributor

@jakoberpf
A few things to confirm:

Could you help me verify these? Thx.

@jakoberpf
Copy link
Author

jakoberpf commented May 3, 2021

@houshengbo

  • correct we do not have a knative-local-gateway within our normal istio install. knative-local-gateway is only created when the operator is deployed and istio-system ns is created/present. If istio-system is not present we see the described error. The service/knative-local-gateway is also the only ressource created in the istio-system ns when present.
  • Yes I should be able to test this tomorrow (along this doc) ...

@houshengbo
Copy link
Contributor

@jakoberpf
There is another way to do this so far. If you install the istio under a namspace, for example, istio-test, you need to change the configmap config-istio. Suppose you install knative serving under ns knative-serving. To change it with the operator, you can use the serving CR like:

apiVersion: operator.knative.dev/v1alpha1
kind: KnativeServing
metadata:
  name: knative-serving
  namespace: knative-serving
spec:
  config:
    istio:
      gateway.knative-serving.knative-ingress-gateway: "istio-ingressgateway.istio-test.svc.cluster.local"
      local-gateway.knative-serving.knative-local-gateway: "knative-local-gateway.istio-test.svc.cluster.local"

Without changing the schema of the serving CR, operator is able to look up the istio ns by checking knative-local-gateway.istio-test.svc.cluster.local. Then set the service knative-local-gateway with the correct ns.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Categorizes issue or PR as related to a bug.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants