Skip to content

Conversation

jcaamano
Copy link
Contributor

cherry-picked

5cc28c623 FRR-K8s webhook: promote to priviledged
7056e67b7 FRRK8s CRDs: align to upstream
0b29885cc FRRK8s webhook: align to upstream
acdd04316 FRRK8s webhook: webhook liveness / readiness from metrics to webhook
0dae0a03e (origin/nad-validation-nameOrSpec, nad-validation-nameOrSpec) Validate NAD name and spec only in multus admission controller
401f7b46b frr-k8s: stop listening for incoming connection in the bgp daemon
bc5f08ed5 Pass '--gateway-mode' flag for ovnkube-cluster-manager

did not cherry-pick

acdd04316 FRRK8s webhook: webhook liveness / readiness from metrics to webhook

as that requires FRR-k8s to be backported as well which is pending.

conflict: acdd043 added webhook arg/port, later on 0b29885 removed the monitoring arg/port. The resolution of the conflict is, logically, neither webhook arg/port nor monitoring arg/port defined.
/cc @fedepaol to croscheck the resolution of the conflict

/hold
waiting for #2714 unless we decide otherwise

pliurh and others added 6 commits July 17, 2025 11:16
Some code (EgressSVC and BGP) in cluster-manager needs to know the
gateway mode.

Signed-off-by: Peng Liu <[email protected]>
(cherry picked from commit bc5f08e)
frr-k8s will not listen to port 179, and BGP peering can only be
established from OCP to external.

Signed-off-by: Konstantinos Karampogias <[email protected]>
(cherry picked from commit 401f7b4)
This should save up calls to the webhook and reduce latency when a NAD
is updated to add labels or annotations.

The root cause of the change is the OVNK BGP feature: when BGP is
enabled for the cluster default network, reconfiguration might cause
temporary disruptions. As part of this reconfiguration and necessary to
complete it, OVNK depends on annotating an internal NAD. We want to
avoid having to reach the webhook for this annotation because the
temporary disruption might prevent it and in that case the
reconfiguration won't complete.

Another possibility would be to filter out from validation the specific
internal NAD but this current approach might be more beneficial overall.

Signed-off-by: Jaime Caamaño Ruiz <[email protected]>
(cherry picked from commit 0dae0a0)
Aligning to upstream and:
- move the webhook deployment to hostnetworked, so that the api can
  still be served if an offending FRRConfiguration is applied
- openshift only: change the webhook port to one in the allowed range
- remove the metrics listening port, as no service monitor was deployed

Signed-off-by: Federico Paolinelli <[email protected]>
(cherry picked from commit 0b29885)
Aligning to upstream and bringing the deprecation of the disableMP flag
and the introduction of a "dualStackAddressFamily" flag to bring the
behavior back to allow backward compatibility.

The default behavior was inconsistent in case of dual stack clusters, as
frr was being configured to advertise both ip families over a single
session, without being instructed properly to what next hop set for the
ip family not corresponding to the ip family of the session.

The dualStackAddressFamily flag is introduced to allow users relying on
that behavior to keep working. Note that both flags are not documented
nor supported d/s.

More details in the upstream metallb issue
metallb/metallb#2704

Signed-off-by: Federico Paolinelli <[email protected]>
(cherry picked from commit 7056e67)
We need to run the webhook as hostnetworked, so it requires the
privileged scc.

Signed-off-by: Federico Paolinelli <[email protected]>
(cherry picked from commit 5cc28c6)
@openshift-merge-robot
Copy link
Contributor

PR needs rebase.

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.

@openshift-merge-robot openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 17, 2025
@openshift-ci openshift-ci bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jul 17, 2025
@openshift-ci openshift-ci bot requested a review from fedepaol July 17, 2025 11:39
@jcaamano
Copy link
Contributor Author

/close filed against wrong branch doh

@jcaamano jcaamano closed this Jul 17, 2025
Copy link
Contributor

openshift-ci bot commented Jul 17, 2025

@jcaamano: GitHub didn't allow me to request PR reviews from the following users: to, croscheck, the, resolution, of, conflict.

Note that only openshift members and repo collaborators can review this PR, and authors cannot review their own PRs.

In response to this:

cherry-picked

5cc28c623 FRR-K8s webhook: promote to priviledged
7056e67b7 FRRK8s CRDs: align to upstream
0b29885cc FRRK8s webhook: align to upstream
acdd04316 FRRK8s webhook: webhook liveness / readiness from metrics to webhook
0dae0a03e (origin/nad-validation-nameOrSpec, nad-validation-nameOrSpec) Validate NAD name and spec only in multus admission controller
401f7b46b frr-k8s: stop listening for incoming connection in the bgp daemon
bc5f08ed5 Pass '--gateway-mode' flag for ovnkube-cluster-manager

did not cherry-pick

acdd04316 FRRK8s webhook: webhook liveness / readiness from metrics to webhook

as that requires FRR-k8s to be backported as well which is pending.

conflict: acdd043 added webhook arg/port, later on 0b29885 removed the monitoring arg/port. The resolution of the conflict is, logically, neither webhook arg/port nor monitoring arg/port defined.
/cc @fedepaol to croscheck the resolution of the conflict

/hold
waiting for #2714 unless we decide otherwise

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.

Copy link
Contributor

openshift-ci bot commented Jul 17, 2025

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jcaamano

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

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jul 17, 2025
Copy link
Contributor

openshift-ci bot commented Jul 17, 2025

@jcaamano: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/e2e-ovn-step-registry d8215b1 link false /test e2e-ovn-step-registry
ci/prow/e2e-network-mtu-migration-ovn-ipv6 d8215b1 link false /test e2e-network-mtu-migration-ovn-ipv6
ci/prow/e2e-vsphere-ovn-dualstack-primaryv6 d8215b1 link false /test e2e-vsphere-ovn-dualstack-primaryv6
ci/prow/e2e-aws-ovn-shared-to-local-gateway-mode-migration d8215b1 link false /test e2e-aws-ovn-shared-to-local-gateway-mode-migration
ci/prow/e2e-ovn-ipsec-step-registry d8215b1 link true /test e2e-ovn-ipsec-step-registry
ci/prow/e2e-metal-ipi-ovn-ipv6 d8215b1 link true /test e2e-metal-ipi-ovn-ipv6
ci/prow/e2e-gcp-ovn-upgrade d8215b1 link true /test e2e-gcp-ovn-upgrade
ci/prow/e2e-metal-ipi-ovn-dualstack-bgp d8215b1 link true /test e2e-metal-ipi-ovn-dualstack-bgp
ci/prow/e2e-metal-ipi-ovn-dualstack-bgp-local-gw d8215b1 link true /test e2e-metal-ipi-ovn-dualstack-bgp-local-gw
ci/prow/hypershift-e2e-aks d8215b1 link true /test hypershift-e2e-aks
ci/prow/e2e-gcp-ovn d8215b1 link true /test e2e-gcp-ovn
ci/prow/e2e-aws-ovn-upgrade-ipsec d8215b1 link true /test e2e-aws-ovn-upgrade-ipsec
ci/prow/security d8215b1 link false /test security
ci/prow/e2e-azure-ovn d8215b1 link false /test e2e-azure-ovn
ci/prow/e2e-aws-ovn-upgrade d8215b1 link true /test e2e-aws-ovn-upgrade
ci/prow/e2e-vsphere-ovn-dualstack d8215b1 link false /test e2e-vsphere-ovn-dualstack
ci/prow/e2e-network-mtu-migration-ovn-ipv4 d8215b1 link false /test e2e-network-mtu-migration-ovn-ipv4
ci/prow/e2e-aws-ovn-single-node d8215b1 link false /test e2e-aws-ovn-single-node
ci/prow/e2e-openstack-ovn d8215b1 link false /test e2e-openstack-ovn
ci/prow/e2e-aws-ovn-serial-ipsec d8215b1 link false /test e2e-aws-ovn-serial-ipsec
ci/prow/e2e-azure-ovn-upgrade d8215b1 link true /test e2e-azure-ovn-upgrade
ci/prow/4.20-upgrade-from-stable-4.19-images d8215b1 link true /test 4.20-upgrade-from-stable-4.19-images
ci/prow/images d8215b1 link true /test images
ci/prow/verify d8215b1 link true /test verify
ci/prow/e2e-aws-ovn-windows d8215b1 link true /test e2e-aws-ovn-windows
ci/prow/e2e-aws-hypershift-ovn-kubevirt d8215b1 link false /test e2e-aws-hypershift-ovn-kubevirt
ci/prow/lint d8215b1 link true /test lint
ci/prow/4.20-upgrade-from-stable-4.19-e2e-gcp-ovn-upgrade d8215b1 link false /test 4.20-upgrade-from-stable-4.19-e2e-gcp-ovn-upgrade
ci/prow/unit d8215b1 link true /test unit
ci/prow/e2e-metal-ipi-ovn-ipv6-ipsec d8215b1 link true /test e2e-metal-ipi-ovn-ipv6-ipsec
ci/prow/e2e-gcp-ovn-techpreview d8215b1 link true /test e2e-gcp-ovn-techpreview
ci/prow/e2e-aws-ovn-local-to-shared-gateway-mode-migration d8215b1 link false /test e2e-aws-ovn-local-to-shared-gateway-mode-migration
ci/prow/e2e-aws-ovn-serial d8215b1 link false /test e2e-aws-ovn-serial
ci/prow/frrk8s-e2e d8215b1 link false /test frrk8s-e2e
ci/prow/e2e-vsphere-ovn d8215b1 link false /test e2e-vsphere-ovn
ci/prow/e2e-aws-ovn-hypershift-conformance d8215b1 link true /test e2e-aws-ovn-hypershift-conformance
ci/prow/4.20-upgrade-from-stable-4.19-e2e-aws-ovn-upgrade d8215b1 link false /test 4.20-upgrade-from-stable-4.19-e2e-aws-ovn-upgrade
ci/prow/4.20-upgrade-from-stable-4.19-e2e-azure-ovn-upgrade d8215b1 link false /test 4.20-upgrade-from-stable-4.19-e2e-azure-ovn-upgrade
ci/prow/e2e-ovn-hybrid-step-registry d8215b1 link false /test e2e-ovn-hybrid-step-registry
ci/prow/verify-deps d8215b1 link true /test verify-deps

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-sigs/prow repository. I understand the commands that are listed here.

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. do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants