search

tryretool / retool-helm

MIT License
46 stars 57 forks source link

created ingress for kubernetes v1.22 compatible #36

Closed prashant637 closed 2 years ago

prashant637 commented 2 years ago

https://kubernetes.io/docs/reference/using-api/deprecation-guide/#ingress-v122

prashant637 commented 2 years ago

@mukul-c2fo Changes has been done to support the old and new ingress.

mukul-c2fo commented 2 years ago

@theodormarcu - Could we please get some eyes on this PR as this could be a blocker for us to rollout 1.22.

anna-yn commented 2 years ago

Hi @prashant637 @mukul-c2fo , sorry for the late reply and thanks for the PR! Looks like this PR is very similar to #38 , except that this one sets the ingressClassName and the other one doesn't? I'd prefer we either don't set the ingressClassName for now or make it optional by default (only set it if ingress.className has a value, and don't give it a value by default) because otherwise it breaks helm ugprade for people who've been setting the ingress class with the kubernetes.io/ingress.class annotation. What do you think?

anna-yn commented 2 years ago

Hi again! I merged #38 because that one doesn't contain the ingressClassName change and it won't affect people's helm upgrade. It's in the release v4.7.0 now

prashant637 commented 2 years ago

Hi @anna-yn Sure, we can skip the ingressClassName and can run without using the same. I ran the release v4.7.0, it's working fine. Thanks for merging the PR #38