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

upgrade ingress nginx version #4551

Merged
merged 1 commit into from Dec 28, 2021
Merged

Conversation

chaunceyjiang
Copy link
Contributor

What type of PR is this?

/kind bug

What this PR does / why we need it:

ingress nginx controller deployment failure due to K8s v1.22+ api removal

Which issue(s) this PR fixes:

Fixes #4548 #4486

Special notes for reviewers:

Does this PR introduced a user-facing change?

None

Additional documentation, usage docs, etc.:

upgrade ingress nginx version
https://github.com/kubernetes/ingress-nginx/blob/helm-chart-4.0.13/Changelog.md

@ks-ci-bot ks-ci-bot added release-note-none kind/bug Categorizes issue or PR as related to a bug. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Dec 17, 2021
@ks-ci-bot
Copy link
Collaborator

Hi @chaunceyjiang. Thanks for your PR.

I'm waiting for a kubesphere member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@pixiake
Copy link
Contributor

pixiake commented Dec 20, 2021

/cc @RolandMa1986 @zryfish

@RolandMa1986
Copy link
Member

@chaunceyjiang It seems like the nginx-ingress 1.1.0 don't compatible with the old nginx 0.48.x. We can't simply change the downloaded helm chart version.

@RolandMa1986
Copy link
Member

@chaunceyjiang Please check the doc. in the v1.x release. we need add the following flags to let ingress controller monitoring all ingress.

In this case, you need to make your controller aware of the objects. If you have any Ingress objects that don't yet have either the .spec.ingressClassName field set in their manifest, or the ingress annotation (kubernetes.io/ingress.class), then you should start your Ingress-NGINX controller with the flag --watch-ingress-without-class=true.

You can configure your Helm chart installation's values file with .controller.watchIngressWithoutClass: true.

@chaunceyjiang
Copy link
Contributor Author

Nginx Ingress - breaking change, Ingress.class now required
image

@chaunceyjiang
Copy link
Contributor Author

@chaunceyjiang Please check the doc. in the v1.x release. we need add the following flags to let ingress controller monitoring all ingress.

In this case, you need to make your controller aware of the objects. If you have any Ingress objects that don't yet have either the .spec.ingressClassName field set in their manifest, or the ingress annotation (kubernetes.io/ingress.class), then you should start your Ingress-NGINX controller with the flag --watch-ingress-without-class=true.

You can configure your Helm chart installation's values file with .controller.watchIngressWithoutClass: true.

ok

@chaunceyjiang
Copy link
Contributor Author

Nginx Ingress - breaking change, Ingress.class now required image

Turns out the fix was very simple, you just needed to ensure that your ingress annotations included the line kubernetes.io/ingress.class: "nginx" to specify the IngressClass.

image
image

@RolandMa1986
Copy link
Member

@chaunceyjiang if users are upgradding from an older version, all the ingresses needed to be updated manually. So it would be better to add ".controller.watchIngressWithoutClass: true" to let it works like the old way.

@chaunceyjiang
Copy link
Contributor Author

chaunceyjiang commented Dec 21, 2021

@chaunceyjiang if users are upgradding from an older version, all the ingresses needed to be updated manually. So it would be better to add ".controller.watchIngressWithoutClass: true" to let it works like the old way.

ok
you are right . I will add the parameter flag --watch-ingress-without-class=true

@ks-ci-bot ks-ci-bot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Dec 21, 2021
@chaunceyjiang
Copy link
Contributor Author

@chaunceyjiang if users are upgradding from an older version, all the ingresses needed to be updated manually. So it would be better to add ".controller.watchIngressWithoutClass: true" to let it works like the old way.

ok you are right . I will add the parameter flag --watch-ingress-without-class=true

image

image

@chaunceyjiang
Copy link
Contributor Author

so kubesphere/nginx-ingress-controller:v1.1.0 is required

config/watches.yaml Outdated Show resolved Hide resolved
@chaunceyjiang
Copy link
Contributor Author

image
image

@RolandMa1986
Copy link
Member

/retest

@RolandMa1986
Copy link
Member

/retest

@RolandMa1986
Copy link
Member

/lgtm
@chaunceyjiang btw, The docker image version will be overridden by the ks-installer:
https://github.com/kubesphere/ks-installer/blob/master/roles/download/defaults/main.yml

@ks-ci-bot ks-ci-bot added the lgtm Indicates that a PR is ready to be merged. label Dec 24, 2021
@ks-ci-bot
Copy link
Collaborator

LGTM label has been added.

Git tree hash: ce4575848ac58f20b87d473f55a327ce56c267ad

@chaunceyjiang
Copy link
Contributor Author

/lgtm @chaunceyjiang btw, The docker image version will be overridden by the ks-installer: https://github.com/kubesphere/ks-installer/blob/master/roles/download/defaults/main.yml

ok

kubesphere/ks-installer#1872

@RolandMa1986
Copy link
Member

/cc @zryfish

@zryfish
Copy link
Member

zryfish commented Dec 27, 2021

/approve

@ks-ci-bot
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: chaunceyjiang, zryfish

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@ks-ci-bot ks-ci-bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Dec 27, 2021
@zryfish
Copy link
Member

zryfish commented Dec 27, 2021

/test pull-kubesphere-unit-test

@ks-ci-bot
Copy link
Collaborator

@chaunceyjiang: The following test failed, say /retest to rerun all failed tests:

Test name Commit Details Rerun command
pull-kubesphere-unit-test a80c94e link /test pull-kubesphere-unit-test

Full PR test history. Your PR dashboard.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

@zryfish
Copy link
Member

zryfish commented Dec 28, 2021

/test pull-kubesphere-unit-test

@zryfish
Copy link
Member

zryfish commented Dec 28, 2021

/ok-to-test

@ks-ci-bot ks-ci-bot merged commit e9a6289 into kubesphere:master Dec 28, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. kind/bug Categorizes issue or PR as related to a bug. lgtm Indicates that a PR is ready to be merged. ok-to-test release-note-none size/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Gateway deployment failure due to K8s v1.22+ api removal
5 participants