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

Support request for Helm Chart's ingress section #1220

Closed
blaargh opened this issue Aug 12, 2024 · 6 comments
Closed

Support request for Helm Chart's ingress section #1220

blaargh opened this issue Aug 12, 2024 · 6 comments

Comments

@blaargh
Copy link

blaargh commented Aug 12, 2024

Setting up the ingress values leads to an incomplete (yet not broken) ingress definition. It is completely missing the http path options, rendering the current implementation useless, as every update of the upstream chart requires manual edit of the resulting ingress resource.

Expected Behaviour

The ingress definition should be complete, given the values required.

Current Behaviour

The ingress definition is incomplete.

spec:
  {{- if .Values.ingress.ingressClassName }}
  ingressClassName: {{ .Values.ingress.ingressClassName }}
  {{- end }}
  rules:

{{ toYaml .Values.ingress.hosts | indent 2 }}
  {{- if .Values.ingress.tls }}
  tls:
{{ toYaml .Values.ingress.tls | indent 4 }}
  {{- end -}}

toYaml of the .hosts path in values.yaml is not enough to have a proper ingress definition:

ingress:
  enabled: false
  ## For k8s >= 1.18 you need to specify the pathType
  ## See https://kubernetes.io/blog/2020/04/02/improvements-to-the-ingress-api-in-kubernetes-1.18/#better-path-matching-with-path-types
  #pathType: ImplementationSpecific

  # Used to create Ingress record (should be used with exposeServices: false).
  hosts:
    - host: gateway.openfaas.local  # Replace with gateway.example.com if public-facing
      serviceName: gateway
      servicePort: 8080
      path: /

it is missing:

http:
  paths:
    - path: /
       pathType: Prefix
       backend:
         service:
            name: gateway
            port:
              number: 8080

Why is this needed?

So the ingress definition doesn't need to be updated manually after each install

Who is this for?

Everyone using the chart and wanting to receive the latest updates.

List All Possible Solutions and Workarounds

Proposed solution of the ingress.yaml template (specifying everything after the spec key):

spec:
  {{- if .Values.ingress.ingressClassName }}
  ingressClassName: {{ .Values.ingress.ingressClassName }}
  {{- end }}
  rules:
  {{- range .Values.ingress.hosts }}
    - host: {{ .host }}
      http:
        paths:
          - path: {{ .path | default "/" }}
            pathType: Prefix
            backend:
              service:
                name: {{ .serviceName }}
                port:
                  number: {{ .servicePort }}
  {{- end }}

{{- if .Values.ingress.tls }}
  tls:
{{ toYaml .Values.ingress.tls | indent 4 }}
{{- end }}

Steps to Reproduce (for bugs)

  1. Install the chart with values set to enable and configure the ingress
  2. Observe incomplete ingress resource

Context

Your Environment

  • FaaS-CLI version ( Full output from: faas-cli version ):

  • Docker version docker version (e.g. Docker 17.0.05 ):

  • What version and distriubtion of Kubernetes are you using? kubectl version

  • Operating System and version (e.g. Linux, Windows, MacOS):

  • Link to your project or a code example to reproduce issue:

  • What network driver are you using and what CIDR? i.e. Weave net / Flannel

@alexellis
Copy link
Member

Hi @blaargh

I don't recognise your profile and I can't see your company in your profile, do you work for a company that is a customer?

If like you say, the Ingress definition is working, why do you need to edit it to include the additional http section?

Alex

@alexellis
Copy link
Member

alexellis commented Aug 19, 2024

I took a look into your question. Have you already read the docs for OpenFaaS?

https://docs.openfaas.com/reference/tls-openfaas/#configure-tls-for-the-openfaas-gateway

The http section needs to be inputted via values.yaml, it's all explained above.

Am I missing something else?

image

@blaargh
Copy link
Author

blaargh commented Aug 19, 2024

Hi @alexellis

I am using openfaas for private projects.

The ingress definition is working in the sense that it is syntactically correct, so Kubernetes doesn't decline it. But without the missing http section, it will never map to any service. I didn't see the documentation you mentioned, I just pulled the chart from artifacthub.

I would expect a mention of that within the chart directly, instead of putting that in the documentation. Because in the chart it is misleading/wrong what you would have to put for a correct ingress definition:
image

@alexellis
Copy link
Member

The values.yaml you see is what's required for the previous versions of Kubernetes, you need to refer to the docs for the newer format as I explained in my previous comment.

There is no problem with the Helm chart, it has a minimal configuration, and those wishing to use Ingress and TLS need to follow the documentation: https://docs.openfaas.com/reference/tls-openfaas/#configure-tls-for-the-openfaas-gateway

It generates the "complete" YAML definition you are requesting.

values-custom-ingress.yaml:

ingress:
  enabled: true
  hosts:
  - host: gateway.openfaas.local  # Replace with gateway.example.com if public-facing
    http:
      paths:
      - path: /
        pathType: Prefix
        backend:
          service:
            name: gateway
            port:
              number: 8080

Then:

helm repo update  && helm upgrade openfaas   --install ./chart/openfaas   --namespace openfaas    -f ./chart/openfaas/values.yaml   -f ./chart/openfaas/values-pro.yaml -f ./chart/openfaas/values-custom-ingress.yaml

Output:

apiVersion: networking.k8s.io/v1
kind: Ingress
metadata:
  annotations:
    meta.helm.sh/release-name: openfaas
    meta.helm.sh/release-namespace: openfaas
  creationTimestamp: "2024-08-19T11:34:56Z"
  generation: 1
  labels:
    app: openfaas
    app.kubernetes.io/managed-by: Helm
    chart: openfaas-14.2.65
    heritage: Helm
    release: openfaas
  name: openfaas-ingress
  namespace: openfaas
  resourceVersion: "1352"
  uid: 3ed2e7fb-c42d-4987-9277-5ee53faaeb97
spec:
  rules:
  - host: gateway.openfaas.local
    http:
      paths:
      - backend:
          service:
            name: gateway
            port:
              number: 8080
        path: /
        pathType: Prefix
status:
  loadBalancer: {}

Bear in mind that OpenFaaS CE is only licensed for non-commercial personal use. OpenFaaS Standard is available on a monthly basis and faasd can be used on a single host for commercial purposes if needed.

If it's less confusing for you, we can migrate the empty/default example in values.yaml to follow the newer pattern for K8s ingress, but customers will still need to follow the docs.

@alexellis alexellis changed the title Helm Chart ingress definition incomplete Support request for Helm Chart's ingress section Aug 19, 2024
@bobbiec
Copy link

bobbiec commented Aug 21, 2024

@alexellis , I think the change made in c6427d2 broke arkade install openfaas, which is the installation method recommended by docs.

arkade install openfaas on a fresh cluster now results in the following error:

VALUES values.yaml
Command: /Users/bobbiec/.arkade/bin/helm [upgrade --install openfaas openfaas/openfaas --namespace openfaas --values /var/folders/89/qswnsxyn10z9kzwy6f2fj87h0000gn/T/charts/openfaas/values.yaml --set autoscaler.enabled=false --set dashboard.publicURL=http://127.0.0.1:8080 --set serviceType=LoadBalancer --set openfaasImagePullPolicy=IfNotPresent --set basicAuthPlugin.replicas=1 --set gateway.replicas=1 --set queueWorker.replicas=1 --set queueWorker.maxInflight=1 --set ingressOperator.create=true --set clusterRole=false --set operator.create=false --set faasnetes.imagePullPolicy=Always --set dashboard.enabled=false --set basic_auth=true --set gateway.directFunctions=false]
Error: UPGRADE FAILED: resource mapping not found for name: "openfaas-ingress" namespace: "openfaas" from "": no matches for kind "Ingress" in version "extensions/v1beta1"
ensure CRDs are installed first
Error: exit code 1, stderr: Error: UPGRADE FAILED: resource mapping not found for name: "openfaas-ingress" namespace: "openfaas" from "": no matches for kind "Ingress" in version "extensions/v1beta1"
ensure CRDs are installed first

And, reverting the values.yaml change to the old version (.ingress.enabled = false and related) fixed the issue.

@alexellis
Copy link
Member

Hi thanks for your interest in OpenFaaS CE.

OpenFaaS CE is for personal use. I was curious to learn more about your use-case if you don't mind sharing?

The error no matches for kind "Ingress" in version "extensions/v1beta1" implies that you may be on an old K8s version? What does kubectl version give you?

ingress.enabled was not meant to be commited, but gave no error for me when I ran arkade install openfaas with the below version:

Client Version: v1.30.1
Server Version: v1.30.0

That said, I'll disable it and you should be unaffected going forward. You can also use --set ingress.enabled=false as an argument to arkade install openfaas

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants