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

Empty $service_name and $service_port variables #5198

Closed
praseodym opened this issue Feb 29, 2020 · 15 comments · Fixed by #5226
Closed

Empty $service_name and $service_port variables #5198

praseodym opened this issue Feb 29, 2020 · 15 comments · Fixed by #5226
Labels
kind/bug Categorizes issue or PR as related to a bug.

Comments

@praseodym
Copy link
Contributor

praseodym commented Feb 29, 2020

NGINX Ingress controller version: 0.30.0

Kubernetes version (use kubectl version): v1.17.2 in kind

What happened:

I am trying to get Linkerd to work with ingress-nginx. Linkerd provides instructions for this, the important annotation to add to the Ingress object being:

    nginx.ingress.kubernetes.io/configuration-snippet: |
      proxy_set_header l5d-dst-override $service_name.$namespace.svc.cluster.local:$service_port;
      grpc_set_header l5d-dst-override $service_name.$namespace.svc.cluster.local:$service_port;

This didn't work as expected, and I traced this down to the $service_name and $service_port variables not being filled as expected:

> kubectl ingress-nginx conf | grep -C4 service_name
--
		location / {
			
			set $namespace      "emojivoto";
			set $ingress_name   "emojivoto";
			set $service_name   "";
			set $service_port   "";
			set $location_path  "/";
			
			rewrite_by_lua_block {
--

What you expected to happen:

The $service_name variable being set to web-svc and the $service_port variable being set to 80.

How to reproduce it:

Install the emojivoto sample application and apply the following Ingress object:

apiVersion: networking.k8s.io/v1beta1
kind: Ingress
metadata:
  name: emojivoto
  namespace: emojivoto
  annotations:
    nginx.ingress.kubernetes.io/configuration-snippet: |
      proxy_set_header l5d-dst-override $service_name.$namespace.svc.cluster.local:$service_port;
spec:
  rules:
  - http:
      paths:
      - backend:
          serviceName: web-svc
          servicePort: 80
        path: /

Then use the ingress-nginx kubectl plugin to check the nginx config (kubectl ingress-nginx conf | grep -C4 service_name).

Anything else we need to know:

/kind bug

@praseodym praseodym added the kind/bug Categorizes issue or PR as related to a bug. label Feb 29, 2020
@aledbf
Copy link
Member

aledbf commented Feb 29, 2020

servicePort: 8080

@praseodym please check the ingress controller pod log.
This should not be 80? (from the web.yaml emojivoto yaml)

@praseodym
Copy link
Contributor Author

@praseodym please check the ingress controller pod log.
This should not be 80? (from the web.yaml emojivoto yaml)

You're right, it should be servicePort: 80. Unfortunately that doesn't make a difference: $service_name and $service_port are still empty.

@aledbf
Copy link
Member

aledbf commented Mar 1, 2020

@praseodym please post the pod logs. If that is empty, something should appear there.

@praseodym
Copy link
Contributor Author

Please see the ingress-nginx pod logs with --v=2.

@aledbf
Copy link
Member

aledbf commented Mar 1, 2020

@praseodym are you using the sample ingress definition? (or did you create an ingress without host field?)

@praseodym
Copy link
Contributor Author

I created an ingress object without a host field (the definition in the first post is the full object I applied).

@praseodym
Copy link
Contributor Author

Adding a host field to the ingress fixes the problem:

apiVersion: networking.k8s.io/v1beta1
kind: Ingress
metadata:
  name: emojivoto
  namespace: emojivoto
  annotations:
    nginx.ingress.kubernetes.io/configuration-snippet: |
      proxy_set_header l5d-dst-override $service_name.$namespace.svc.cluster.local:$service_port;
spec:
  rules:
  - host: localhost
    http:
      paths:
      - backend:
          serviceName: web-svc
          servicePort: 80
        path: /

gives:

		location / {
			
			set $namespace      "emojivoto";
			set $ingress_name   "emojivoto";
			set $service_name   "web-svc";
			set $service_port   "80";
			set $location_path  "/";
			
			rewrite_by_lua_block {

While it isn't too common to not have a host field defined, it'd still expect that to work or log a warning message if it is not a supported configuration.

@aledbf
Copy link
Member

aledbf commented Mar 2, 2020

While it isn't too common to not have a host field defined, it'd still expect that to work or log a warning message if it is not a supported configuration.

This is a bug, I am just trying to narrow where is located.

@aledbf
Copy link
Member

aledbf commented Mar 8, 2020

@praseodym please test quay.io/kubernetes-ingress-controller/nginx-ingress-controller-amd64:dev

@praseodym
Copy link
Contributor Author

@praseodym please test quay.io/kubernetes-ingress-controller/nginx-ingress-controller-amd64:dev

This fixes the problem, thank you!

@wmorgan
Copy link
Contributor

wmorgan commented Mar 9, 2020

Thanks from the Linkerd team, @aledbf!

@zaharidichev
Copy link

@aledbf We are still running into this problem. Namely, I have the following resource:

apiVersion: networking.k8s.io/v1beta1
kind: Ingress
metadata:
  name: test
  annotations:
    kubernetes.io/ingress.class: nginx
    nginx.ingress.kubernetes.io/configuration-snippet: |
      proxy_set_header l5d-dst-override $service_name.$namespace.svc.cluster.local:$service_port;
      grpc_set_header l5d-dst-override $service_name.$namespace.svc.cluster.local:$service_port;
spec:
  rules:
  - http:
      paths:
      - backend:
          serviceName: backend-one-svc-west
          servicePort: 8888
        path: /test

I find the l5d-dst-override header to be set to ".default.svc.cluster.local:" We are using 0.33.0

@aledbf
Copy link
Member

aledbf commented Jul 14, 2020

@zaharidichev thank you for the report. I will open a PR to fix this issue.

That said, what's the use case for an ingress without a host field? I asked because, without that field, paths cannot be repeated, and accessing an ingress controller by an IP address is not the "correct" approach.

@gauravkohli
Copy link

gauravkohli commented Aug 11, 2020

@aledbf

We are having a similar problem even if we have specified the host variable like below. So while using below ingress we get "set $service_name ""; " in the nginx.conf ( so a empty service_anme) and if I remove annotation "nginx.ingress.kubernetes.io/rewrite-target: /" then it works fine and I get "set $service_name "API";"

apiVersion: networking.k8s.io/v1beta1
kind: Ingress
metadata:
  name: "ingress-api"
  namespace: test
  annotations:
    nginx.ingress.kubernetes.io/rewrite-target: /
    nginx.ingress.kubernetes.io/configuration-snippet: |
      proxy_set_header l5d-dst-override $service_name.$namespace.svc.cluster.local;
spec:
  tls:
  - hosts:
    - test.io
    secretName: tls-secret
  rules:
    - host: test.io
      http:
        paths:
        - path: /api/v1/
          backend:
            serviceName: api
            servicePort: 80

Is this something you could have a look at?

@mbelang
Copy link

mbelang commented Oct 13, 2020

I stumble upon this while debugging the linkerd annotations for ingress. It seems not that service_port variable is always 80 by default.

I'm using controller version v0.34.1

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.

6 participants