Bump ingress-nginx to 3.13.0 #2961

TeddyAndrieux · 2020-12-02T17:39:48Z

Component:

'ingress-controller'

Context:

Bump nginx-ingress version

Summary:

Use the new helm3 chart for ingress-nginx version 3.12.0
Adapt a bit the options file to works with the new charts
Rename from nginx-ingress to ingress-nginx
For upgrade, downgrade purpose: remove old ingress controller at the very end

bert-e · 2020-12-02T17:39:50Z

Hello teddyandrieux,

My role is to assist you with the merge of this
pull request. Please type @bert-e help to get information
on this process, or consult the user documentation.

Status report is not available.

bert-e · 2020-12-02T17:39:54Z

Waiting for approval

The following approvals are needed before I can proceed with the merge:

the author
one peer

Peer approvals must include at least 1 approval from the following list:

TeddyAndrieux · 2020-12-03T17:42:31Z

/approve

bert-e · 2020-12-03T17:42:37Z

Waiting for approval

The following approvals are needed before I can proceed with the merge:

the author
one peer

Peer approvals must include at least 1 approval from the following list:

The following options are set: approve

gdemonet

Changes look good to me 👍

Just a comment / question (if we want to do it, it's fine to tackle in a follow-up PR IMO): in K8s 1.18, there is a new IngressClass resource type, which is supported in the updated chart we now use. Would it make sense to also update the Ingress objects we deploy to use this new API, and also make sure (once we have a 2.8 branch) to update their apiVersion to /v1 (from /v1beta1)?

TeddyAndrieux · 2020-12-08T09:23:32Z

Just a comment / question (if we want to do it, it's fine to tackle in a follow-up PR IMO): in K8s 1.18, there is a new IngressClass resource type, which is supported in the updated chart we now use. Would it make sense to also update the Ingress objects we deploy to use this new API, and also make sure (once we have a 2.8 branch) to update their apiVersion to /v1 (from /v1beta1)?

Sure but I think it's not possible with the current Python-Kubernetes we embed (and even with the last version available 1.17)

bert-e · 2020-12-08T09:43:07Z

Conflict with a changeset in the queue

The changeset in this pull request conflicts with another changeset
already in the queue. Please wait for the current queue to merge into
the development branch. The conflict will then appear in this pull
request and can be sorted on the feature branch directly.

This changeset has not been added to the queue.

The following options are set: approve

bert-e · 2020-12-08T13:14:43Z

Conflict

There is a conflict between your branch improvement/bump-nginx-ingress-version and the
destination branch development/2.7.

Please resolve the conflict on the feature branch (improvement/bump-nginx-ingress-version).

 $ git fetch
 $ git checkout origin/improvement/bump-nginx-ingress-version
 $ git merge origin/development/2.7
 $ # <intense conflict resolution>
 $ git commit
 $ git push origin HEAD:improvement/bump-nginx-ingress-version

The following options are set: approve

Add support for `metadata` key to be nill and `metadata:annotations` as well

Update the image downloaded in the buildchain for nginx-ingress-controller and also the repository used, as it changed in the new nginx-ingress chart. Update nginx-ingress chart using: ``` rm -rf charts/nginx-ingress helm repo add ingress-nginx https://kubernetes.github.io/ingress-nginx helm repo update helm fetch -d charts --untar ingress-nginx/ingress-nginx ``` Since chart get renamed from `nginx-ingress` to `ingress-nginx` rename the 2 option files. Adapt a bit the yaml option files for control-plane and workload-plane ingress to work with the new chart. And then re-rendering control-plane and workload-plane ingress salt states using: ``` ./charts/render.py ingress-nginx --namespace metalk8s-ingress \ charts/ingress-nginx.yaml charts/ingress-nginx/ \ > salt/metalk8s/addons/nginx-ingress/deployed/chart.sls ./charts/render.py ingress-nginx-control-plane --namespace metalk8s-ingress \ charts/ingress-nginx-control-plane.yaml charts/ingress-nginx/ \ > salt/metalk8s/addons/nginx-ingress-control-plane/deployed/chart.sls ``` Change a bit tests as some ingress Pod labels changed in this new chart.

Because a renaming was needed to use the new ingress-nginx chart from upstream, we need to remove old `nginx-ingress` objects during upgrade from 2.6.x and `ingress-nginx` during downgrade to 2.6.x. We remove these object at the really end of the upgrade and downgrade so that during upgrade and downgrade we can still access all workload ressources using the running nginx ingress controller

bert-e · 2020-12-08T13:24:52Z

Waiting for approval

The following approvals are needed before I can proceed with the merge:

the author
one peer

Peer approvals must include at least 1 approval from the following list:

The following options are set: approve

bert-e · 2020-12-08T14:35:46Z

In the queue

The changeset has received all authorizations and has been added to the
relevant queue(s). The queue(s) will be merged in the target development
branch(es) as soon as builds have passed.

The changeset will be merged in:

✔️ development/2.7

The following branches will NOT be impacted:

development/1.0
development/1.1
development/1.2
development/1.3
development/2.0
development/2.1
development/2.2
development/2.3
development/2.4
development/2.5
development/2.6

There is no action required on your side. You will be notified here once
the changeset has been merged. In the unlikely event that the changeset
fails permanently on the queue, a member of the admin team will
contact you to help resolve the matter.

IMPORTANT

Please do not attempt to modify this pull request.

Any commit you add on the source branch will trigger a new cycle after the
current queue is merged.
Any commit you add on one of the integration branches will be lost.

If you need this pull request to be removed from the queue, please contact a
member of the admin team now.

The following options are set: approve

bert-e · 2020-12-08T14:36:01Z

I have successfully merged the changeset of this pull request
into targetted development branches:

✔️ development/2.7

The following branches have NOT changed:

development/1.0
development/1.1
development/1.2
development/1.3
development/2.0
development/2.1
development/2.2
development/2.3
development/2.4
development/2.5
development/2.6

Please check the status of the associated issue None.

Goodbye teddyandrieux.

TeddyAndrieux added topic:deployment Bugs in or enhancements to deployment stages kind:debt Technical debt topic:lifecycle Issues related to upgrade or downgrade of MetalK8s complexity:medium Something that requires one or few days to fix labels Dec 2, 2020

TeddyAndrieux requested a review from a team December 2, 2020 17:39

TeddyAndrieux marked this pull request as draft December 2, 2020 17:39

TeddyAndrieux force-pushed the improvement/bump-nginx-ingress-version branch 2 times, most recently from dd8f79f to fa4b286 Compare December 3, 2020 07:42

TeddyAndrieux marked this pull request as ready for review December 3, 2020 17:28

TeddyAndrieux force-pushed the improvement/bump-nginx-ingress-version branch from fa4b286 to c827b31 Compare December 3, 2020 17:30

TeddyAndrieux force-pushed the improvement/bump-nginx-ingress-version branch from c827b31 to 6529736 Compare December 8, 2020 08:18

gdemonet previously approved these changes Dec 8, 2020

View reviewed changes

TeddyAndrieux added 3 commits December 8, 2020 14:24

charts/render: Add support for empty metadata:annotations

58fda92

Add support for `metadata` key to be nill and `metadata:annotations` as well

TeddyAndrieux dismissed gdemonet’s stale review via 4eb11a6 December 8, 2020 13:24

TeddyAndrieux force-pushed the improvement/bump-nginx-ingress-version branch from 6529736 to 4eb11a6 Compare December 8, 2020 13:24

gdemonet approved these changes Dec 8, 2020

View reviewed changes

bert-e merged commit 4eb11a6 into development/2.7 Dec 8, 2020

bert-e deleted the improvement/bump-nginx-ingress-version branch December 8, 2020 14:36

TeddyAndrieux changed the title ~~Bump ingress-nginx to 3.12.0~~ Bump ingress-nginx to 3.13.0 Dec 8, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Bump ingress-nginx to 3.13.0 #2961

Bump ingress-nginx to 3.13.0 #2961

TeddyAndrieux commented Dec 2, 2020 •

edited

Loading

bert-e commented Dec 2, 2020

bert-e commented Dec 2, 2020

TeddyAndrieux commented Dec 3, 2020

bert-e commented Dec 3, 2020

gdemonet left a comment

TeddyAndrieux commented Dec 8, 2020 •

edited

Loading

bert-e commented Dec 8, 2020

bert-e commented Dec 8, 2020

bert-e commented Dec 8, 2020

bert-e commented Dec 8, 2020

bert-e commented Dec 8, 2020

Bump ingress-nginx to 3.13.0 #2961

Bump ingress-nginx to 3.13.0 #2961

Conversation

TeddyAndrieux commented Dec 2, 2020 • edited Loading

bert-e commented Dec 2, 2020

Hello teddyandrieux,

bert-e commented Dec 2, 2020

Waiting for approval

TeddyAndrieux commented Dec 3, 2020

bert-e commented Dec 3, 2020

Waiting for approval

gdemonet left a comment

Choose a reason for hiding this comment

TeddyAndrieux commented Dec 8, 2020 • edited Loading

bert-e commented Dec 8, 2020

Conflict with a changeset in the queue

bert-e commented Dec 8, 2020

Conflict

bert-e commented Dec 8, 2020

Waiting for approval

bert-e commented Dec 8, 2020

In the queue

bert-e commented Dec 8, 2020

TeddyAndrieux commented Dec 2, 2020 •

edited

Loading

TeddyAndrieux commented Dec 8, 2020 •

edited

Loading