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.

Sign up for GitHub

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

Jump to bottom

Update ingress-nginx module #42

Merged
merged 3 commits into from
Dec 22, 2023
Merged

Update ingress-nginx module #42

merged 3 commits into from
Dec 22, 2023

Conversation

nbucci
Copy link
Contributor

@nbucci nbucci commented Dec 21, 2023

No description provided.

@nbucci nbucci requested a review from Monska85 December 21, 2023 09:52
Monska85
Monska85 requested changes Dec 21, 2023
View reviewed changes
Copy link
Contributor

@Monska85 Monska85 left a comment

Choose a reason for hiding this comment

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

I would add an UPGRADING.md file with the description of the upgrading procedure to this release. Describe the steps to remove and import resources and why it is necessary (e.g.: kubernetes_namespace != kubernetes_namespace_v1)

CHANGELOG.md Outdated Show resolved Hide resolved
@nbucci nbucci force-pushed the task/2564-update-ingress-nginx-module branch from 1056862 to 7d2a6df Compare December 21, 2023 14:39
@nbucci nbucci requested a review from Monska85 December 21, 2023 16:20
Monska85
Monska85 requested changes Dec 21, 2023
View reviewed changes
CHANGELOG.md Outdated Show resolved Hide resolved
ingress-nginx.tf Outdated Show resolved Hide resolved
@nbucci nbucci force-pushed the task/2564-update-ingress-nginx-module branch from 7d2a6df to 8426da2 Compare December 21, 2023 16:37
@nbucci nbucci requested a review from Monska85 December 21, 2023 16:44
Monska85
Monska85 requested changes Dec 21, 2023
View reviewed changes
UPGRADING.md Show resolved Hide resolved
ingress-nginx.tf Outdated Show resolved Hide resolved
@nbucci nbucci force-pushed the task/2564-update-ingress-nginx-module branch from 8426da2 to 7a97d38 Compare December 21, 2023 16:54
@nbucci @Monska85
Update ingress-nginx.tf
9b0d9dd 
Co-authored-by: Daniele Monti <62102073+Monska85@users.noreply.github.com>
Monska85
Monska85 approved these changes Dec 21, 2023
View reviewed changes
Copy link
Contributor

@Monska85 Monska85 left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@nbucci nbucci requested a review from filippolmt December 21, 2023 17:00
@filippolmt
Copy link
Member

I would keep the defaultBackend to false this because by default it was not installed, if we apply this PR it would be activated.

@nbucci nbucci force-pushed the task/2564-update-ingress-nginx-module branch from b9ca758 to 9673b7d Compare December 22, 2023 09:39
@nbucci nbucci requested a review from filippolmt December 22, 2023 09:40
@nbucci nbucci force-pushed the task/2564-update-ingress-nginx-module branch 4 times, most recently from 92bd77d to 4465883 Compare December 22, 2023 09:54
filippolmt
filippolmt approved these changes Dec 22, 2023
View reviewed changes
Copy link
Member

@filippolmt filippolmt left a comment

Choose a reason for hiding this comment

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

LGTM!

@nbucci nbucci force-pushed the task/2564-update-ingress-nginx-module branch from 4465883 to 0118ab4 Compare December 22, 2023 10:07
@nbucci nbucci merged commit c574cac into main Dec 22, 2023
1 check passed
@Monska85 Monska85 deleted the task/2564-update-ingress-nginx-module branch September 10, 2024 08:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Monska85 Monska85 Monska85 approved these changes

@filippolmt filippolmt filippolmt approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@nbucci @filippolmt @Monska85