-
Notifications
You must be signed in to change notification settings - Fork 45
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
charts,salt,tests: Make Ingress controller watch Ingress without class #3704
charts,salt,tests: Make Ingress controller watch Ingress without class #3704
Conversation
Hello teddyandrieux,My role is to assist you with the merge of this Status report is not available. |
Integration data createdI have created the integration data for the additional destination branches.
The following branches will NOT be impacted:
You can set option
|
Waiting for approvalThe following approvals are needed before I can proceed with the merge:
Peer approvals must include at least 1 approval from the following list:
|
73dab09
to
a5a33c5
Compare
History mismatchMerge commit #73dab09f80c997407e6a6882456f4ab105585b0d on the integration branch It is likely due to a rebase of the branch Please use the |
/reset |
Reset completeI have successfully deleted this pull request's integration branches. |
Integration data createdI have created the integration data for the additional destination branches.
The following branches will NOT be impacted:
You can set option
|
Waiting for approvalThe following approvals are needed before I can proceed with the merge:
Peer approvals must include at least 1 approval from the following list:
|
Could be linked with their "stored apiVersion" in etcd... which relates to #2060 |
I'm not sure, maybe, because the controller can handle the Ingress it's just that apparently the |
You're right, I think the issue is rather that "old" Ingress objects are not updated with an |
The Workload Plane Ingress is the default Ingress controller that should be used for Ingress object, so let's make this Ingress controller watch Ingress object with no class. NOTE: It shouldn't be needed since we define a default IngressClass object, but during upgrade the Ingress object created without Class in MetalK8s 2.10.x are ignored after upgrade with a message that the Ingress object do not have any Class. Re-render the state using: ``` ./charts/render.py ingress-nginx --namespace metalk8s-ingress \ charts/ingress-nginx.yaml charts/ingress-nginx/ \ > salt/metalk8s/addons/nginx-ingress/deployed/chart.sls ```
a5a33c5
to
19a69b3
Compare
History mismatchMerge commit #a5a33c579d53e793add85f69639fa9e512605dfb on the integration branch It is likely due to a rebase of the branch Please use the |
/reset |
Reset completeI have successfully deleted this pull request's integration branches. |
Integration data createdI have created the integration data for the additional destination branches.
The following branches will NOT be impacted:
You can set option
|
Waiting for approvalThe following approvals are needed before I can proceed with the merge:
Peer approvals must include at least 1 approval from the following list:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💯
19a69b3
to
9aabad7
Compare
History mismatchMerge commit #19a69b302de721cd52c4e5ff8580b18eff5ea436 on the integration branch It is likely due to a rebase of the branch Please use the |
/reset |
Reset completeI have successfully deleted this pull request's integration branches. |
Integration data createdI have created the integration data for the additional destination branches.
The following branches will NOT be impacted:
You can set option
|
Waiting for approvalThe following approvals are needed before I can proceed with the merge:
Peer approvals must include at least 1 approval from the following list:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(not sure what changed, you just added a time.sleep(5)
after the Ingress creation?)
Yes and also added missing "teardown" on tests |
/approve |
Build failedThe build for commit did not succeed in branch improvement/watch-ingress-without-class. The following options are set: approve |
In the queueThe changeset has received all authorizations and has been added to the The changeset will be merged in:
The following branches will NOT be impacted:
There is no action required on your side. You will be notified here once IMPORTANT Please do not attempt to modify this pull request.
If you need this pull request to be removed from the queue, please contact a The following options are set: approve |
I have successfully merged the changeset of this pull request
The following branches have NOT changed:
Please check the status of the associated issue None. Goodbye teddyandrieux. |
The Workload Plane Ingress is the default Ingress controller that should
be used for Ingress object, so let's make this Ingress controller watch
Ingress object with no class.
NOTE: It shouldn't be needed since we define a default IngressClass
object, but during upgrade, the Ingress object created without Class
in MetalK8s 2.10.x are ignored after upgrade with a message that the
Ingress objects do not have any Class.