Skip to content

created ingress for kubernetes v1.22 compatible#36

Closed
prashant637 wants to merge 3 commits intotryretool:mainfrom
prashant637:ingress-updation
Closed

created ingress for kubernetes v1.22 compatible#36
prashant637 wants to merge 3 commits intotryretool:mainfrom
prashant637:ingress-updation

Conversation

@prashant637
Copy link

Copy link

@mukul-c2fo mukul-c2fo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@prashant637 - Just wondering if this change would let the chart fail on a lower k8s version. What do you think? If so, we can add some conditionals to apply the ingress based on the k8s version it's going to be deployed to.

@prashant637
Copy link
Author

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

@mukul-c2fo
Copy link

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

@anna-yn
Copy link
Contributor

anna-yn commented Jan 19, 2022

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
Copy link
Contributor

anna-yn commented Jan 20, 2022

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
Copy link
Author

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

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

Successfully merging this pull request may close these issues.

3 participants