-
Notifications
You must be signed in to change notification settings - Fork 173
SDN-4930: [DownstreamMerge] 1-29-25 #2429
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
Conversation
Fixes NPE seen at: openshift#2427 (comment) Certain network types may not have a pod handler or retry framework for cluster manager. Signed-off-by: Tim Rozet <trozet@redhat.com>
Compare annotations directly if possible. For network specific map entries only compare raw json entries without parsing the map in full. Co-authored-by: Tim Rozet <trozet@redhat.com> Signed-off-by: Patryk Diak <pdiak@redhat.com>
Instead of always parsing all node/join subnets parse the raw json map and only compute the results for the affected network. Signed-off-by: Patryk Diak <pdiak@redhat.com>
Signed-off-by: Patryk Diak <pdiak@redhat.com>
Secondary network controllers should ingore resources that do not belong to the current network. Signed-off-by: Patryk Diak <pdiak@redhat.com>
UserDefinedNetworks Performance improvements
GetActiveNetworkForNamespace was being called in controllers that are secondary UDN only. There is no reason to do this, and can cause problems when a secondary NAD is being used by several namespaces, networks. Additionally, GetNetworkRole function is flawed with a note that says do not call this function if a pod is not related to the network controller. If a primary controller (not default) was to call GetNetworkRole on a pod that doesn't belong to it, it would unintentionally return the role as Secondary. This is not possible and we should panic in this case. This will enforce the comment that GetNetworkRole should not be called on a pod that has no network on this controller. Signed-off-by: Tim Rozet <trozet@redhat.com>
The code for default controller was checking if the role of the controller on the pod was not primary. Really it should check if the role is infrastructure-locked. That means there is a primary UDN and the default controller should open ports. Signed-off-by: Tim Rozet <trozet@redhat.com>
Services should not be started on a secondary UDN/NAD controller. Multinetpol should not be started on a primary UDN controller. Signed-off-by: Tim Rozet <trozet@redhat.com>
Order of status may not always be predictable. Allow for it. Signed-off-by: Tim Rozet <trozet@redhat.com>
|
@trozet: This pull request references SDN-4930 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the epic to target either version "4.19." or "openshift-4.19.", but it targets "openshift-4.18" instead. DetailsIn response to this:
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 openshift-eng/jira-lifecycle-plugin repository. |
|
/hold |
|
extra tests... /test e2e-metal-ipi-ovn-ipv6-techpreview also, let's get a look at payload jobs: /payload 4.19 ci blocking |
|
@trozet: trigger 4 job(s) of type blocking for the ci release of OCP 4.19
See details on https://pr-payload-tests.ci.openshift.org/runs/ci/f84f4eb0-dea2-11ef-90c9-85ca78663186-0 trigger 14 job(s) of type blocking for the nightly release of OCP 4.19
See details on https://pr-payload-tests.ci.openshift.org/runs/ci/f84f4eb0-dea2-11ef-90c9-85ca78663186-1 |
|
/lgtm |
|
I checked all the passing techpreview jobs and there were no UDN flakes :) the payload jobs look good too. retesting these two failed jobs because I think we can get them to pass with a re-run or two the bgp-techpreview job is a mess (109 failures), but nothing UDN related and I think it's expected to fail like that for now. the ipv6-techpreview job is failing as expected with the insights-operator tests not being v6 ready. I think this is fine to go in when folks are ready. |
|
@trozet: The following tests failed, say
Full PR test history. Your PR dashboard. DetailsInstructions 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-sigs/prow repository. I understand the commands that are listed here. |
|
/hold cancel upstream PR was merged as is |
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: knobunc, trozet The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/cherry-pick release-4.18 |
|
@jluhrsen: once the present PR merges, I will cherry-pick it on top of DetailsIn response to this:
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-sigs/prow repository. |
9e16d94
into
openshift:master
|
@jluhrsen: new pull request created: #2430 DetailsIn response to this:
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-sigs/prow repository. |
|
[ART PR BUILD NOTIFIER] Distgit: ovn-kubernetes-base |
|
[ART PR BUILD NOTIFIER] Distgit: ovn-kubernetes-microshift |
|
[ART PR BUILD NOTIFIER] Distgit: ose-ovn-kubernetes |
Includes latest performance and other fixes merged upstream.
Includes unmerged upstream PR: ovn-kubernetes/ovn-kubernetes#5013
do not remove the hold on this PR unless the upstream PR has been approved by @jcaamano or @kyrtapz