Join GitHub today
GitHub is home to over 50 million developers working together to host and review code, manage projects, and build software together.
Sign upMake Ingress generators K8s 1.18 compatible #110
Comments
|
We can't add both. A 1.18 cluster:
a 1.17 gives a validation error if the field exists but we can set We might have to get the k8s version when installing ingress and decide if we are doing annotation or field. Ill have a look at checking the version using the go client or something.. |
|
You can't set an annotation for backwards compatibility? Since when were annotations something the Kubernetes API rejected deployments over. If there's no workaround, then like you say the code will need an additional flag or test to determine the API version. @viveksyngh cc |
|
Please don't add a dependency to the Kubernetes go client to arkade, the size of the binary will balloon out of control. Rob Scott is saying that it is supported to have both the annotation and the spec, despite the hard error. Perhaps the best solution for arkade is to run https://twitter.com/robertjscott/status/1262053313786216449?s=20 For the Ingress Operator (that creates ingress, we have the Kubernetes Go client already there, so it seems like we should either set both there, or set either depending on the API server version. openfaas/ingress-operator#27) |
|
The |
|
https://twitter.com/robertjscott/status/1262053313786216449?s=20 It's unclear whether this is intended or unintended behaviour from the K8s team, it certainly doesn't look like both the spec attribute and the legacy annotation are compatible. Thanks @aledbf for echoing what @Waterdrips said above. We should try this for arkade, and then see what kind of errors we get in the ingress-operator and decide how to move on from there. |
|
|
I was already using
The error is ^ there the yaml: apiVersion: extensions/v1beta1
kind: Ingress
metadata:
name: openfaas-gateway
namespace: openfaas
annotations:
cert-manager.io/cluster-issuer: letsencrypt-staging
kubernetes.io/ingress.class: nginx
spec:
ingressClassName: nginx
rules:
- host: example.heyal.uk
http:
paths:
- backend:
serviceName: gateway
servicePort: 8080
path: /
tls:
- hosts:
- example.heyal.uk
secretName: openfaas-gateway
---
apiVersion: cert-manager.io/v1alpha2
kind: ClusterIssuer
metadata:
name: letsencrypt-staging
spec:
acme:
email: alistair@heyal.co.uk
server: https://acme-staging-v02.api.letsencrypt.org/directory
privateKeySecretRef:
name: example-issuer-account-key
solvers:
- http01:
ingress:
class: nginx |
|
To clarify here, the only reason both a class field and annotation should exist on an Ingress resource at the same time would be part of an upgrade from one approach to the other. Only one of these values will be read from at any given time. When both are set, the annotation is given precedence. Why set both on new resources if only one is going to be used? |
|
There's more, apparently the ingressClass has to be defined separately, if it's a miss on a K8s 1.18 cluster, what happens then? @aledbf do you know? |
|
Sorry to mix two issues here, but whilst you have eyes on @robscott What would you recommend we do for both separate projects? I'd be interested in your opinion if you're open to giving it. |

Formed in 2009, the Archive Team (not to be confused with the archive.org Archive-It Team) is a rogue archivist collective dedicated to saving copies of rapidly dying or deleted websites for the sake of history and digital heritage. The group is 100% composed of volunteers and interested parties, and has expanded into a large amount of related projects for saving online and digital history.


In Kubernetes 1.18 the Ingress definitions have changed slightly.
In particular the @paurosello mentioned the change about
ingressClassbecoming part of the spec, instead of being an annotation.If we can add both, perhaps that will help keep backwards compatibility for the time being?
https://kubernetes.io/docs/setup/release/notes/#extending-ingress-with-and-replacing-a-deprecated-annotation-with-ingressclass
Alex