NO-ISSUE: rebase-release-4.22-4.22.0-0.nightly-2026-06-18-194212_amd64-2026-06-18_arm64-2026-06-18#6912
Conversation
|
@pacevedom: This pull request explicitly references no jira issue. 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. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository YAML (base), Central YAML (inherited) Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
WalkthroughAuto-rebase to OCP nightly builds from June 18–19, 2026. Updates OCP version variables, Kubernetes git commit, and image digests across release/Multus/OLM manifests. Refactors OVN DaemonSets by extracting cluster-manager initialization into a dedicated container and removing ChangesAuto-rebase + OVN refactoring
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 13 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (13 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: pacevedom 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 |
|
/test e2e-aws-tests-bootc-el10 |
|
/test ocp-full-conformance-serial-rhel-eus |
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
assets/components/ovn/single-node/master/daemonset.yaml (1)
376-389:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winKeep the startup log aligned with the executed init modes.
Line 376 logs only
--init-node, but the command still runs both--init-ovnkube-controllerand--init-node.Proposed fix
- echo "I$(date "+%m%d %H:%M:%S.%N") - ovnkube-master - start ovnkube --init-node ${K8S_NODE}" + echo "I$(date "+%m%d %H:%M:%S.%N") - ovnkube-master - start ovnkube --init-ovnkube-controller ${K8S_NODE} --init-node ${K8S_NODE}"🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@assets/components/ovn/single-node/master/daemonset.yaml` around lines 376 - 389, The startup log message in the echo statement on line 376 only mentions --init-node, but the actual ovnkube command being executed includes both --init-ovnkube-controller and --init-node flags. Update the echo statement to reflect both initialization modes being performed, so the log message accurately represents what the command is actually executing. Include both flag names in the log output to match the command execution that follows.assets/components/ovn/multi-node/master/daemonset.yaml (1)
415-417:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUpdate the stale startup flag in the log message.
Line 415 still logs
--init-master, but Line 417 now starts--init-ovnkube-controller.Proposed fix
- echo "I$(date "+%m%d %H:%M:%S.%N") - ovnkube-master - start ovnkube --init-master ${K8S_NODE}" + echo "I$(date "+%m%d %H:%M:%S.%N") - ovnkube-master - start ovnkube --init-ovnkube-controller ${K8S_NODE}"🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@assets/components/ovn/multi-node/master/daemonset.yaml` around lines 415 - 417, The echo log message on line 415 references the outdated flag `--init-master` but the actual ovnkube command execution on line 417 uses the new flag `--init-ovnkube-controller`. Update the log message in the echo statement to replace `--init-master` with `--init-ovnkube-controller` to match the actual command being executed.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@assets/components/ovn/multi-node/master/daemonset.yaml`:
- Around line 356-392: The ovnkube-cluster-manager container is running in a pod
with hostNetwork: true but lacks required security hardening. Add a
securityContext block to this container with runAsNonRoot: true,
readOnlyRootFilesystem: true, and allowPrivilegeEscalation: false; add a
capabilities section to drop ALL capabilities; add resource limits (both cpu and
memory limits in addition to the existing requests); and add both liveness and
readiness probes with appropriate initial delays and timeouts. If hostNetwork is
not actually required for this container's operation, consider moving it to a
separate restricted deployment or pod without host networking to minimize the
security surface.
- Around line 383-387: The ovnkube-cluster-manager container is missing a volume
mount for kubeconfig which is referenced in its config file template. Add a
volume mount entry to the volumeMounts list in the ovnkube-cluster-manager
container that mounts the kubeconfig volume (which already exists at the pod
spec level) to the appropriate directory path that matches the KubeconfigDir
variable used in the config template. This mount should be added alongside the
existing ovnkube-config and env-overrides volume mounts.
In `@assets/components/ovn/single-node/master/daemonset.yaml`:
- Around line 298-334: The ovnkube-cluster-manager container is running in a pod
with host namespaces (hostNetwork and hostPID) but lacks required security
hardening. Add a securityContext section to this container that includes
runAsNonRoot set to true, readOnlyRootFilesystem set to true, and
allowPrivilegeEscalation set to false. Additionally, add a capabilities drop for
ALL, define both CPU and memory resource limits (in addition to the existing
requests), and include liveness and readiness probes to meet the coding security
guidelines.
---
Outside diff comments:
In `@assets/components/ovn/multi-node/master/daemonset.yaml`:
- Around line 415-417: The echo log message on line 415 references the outdated
flag `--init-master` but the actual ovnkube command execution on line 417 uses
the new flag `--init-ovnkube-controller`. Update the log message in the echo
statement to replace `--init-master` with `--init-ovnkube-controller` to match
the actual command being executed.
In `@assets/components/ovn/single-node/master/daemonset.yaml`:
- Around line 376-389: The startup log message in the echo statement on line 376
only mentions --init-node, but the actual ovnkube command being executed
includes both --init-ovnkube-controller and --init-node flags. Update the echo
statement to reflect both initialization modes being performed, so the log
message accurately represents what the command is actually executing. Include
both flag names in the log output to match the command execution that follows.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: 986034a4-354d-4865-a070-09f1b5d6db69
📒 Files selected for processing (2)
assets/components/ovn/multi-node/master/daemonset.yamlassets/components/ovn/single-node/master/daemonset.yaml
| # ovnkube cluster-manager: allocates subnets to nodes, handles cluster-wide IPAM | ||
| - name: ovnkube-cluster-manager | ||
| image: {{ .ReleaseImage.ovn_kubernetes_microshift }} | ||
| command: | ||
| - /bin/bash | ||
| - -c | ||
| - | | ||
| set -xe | ||
| if [[ -f "/env/_master" ]]; then | ||
| set -o allexport | ||
| source "/env/_master" | ||
| set +o allexport | ||
| fi | ||
|
|
||
| echo "$(date -Iseconds) - starting ovnkube-cluster-manager, Node: ${K8S_NODE}" | ||
| exec /usr/bin/ovnkube \ | ||
| --init-cluster-manager "${K8S_NODE}" \ | ||
| --config-file=/run/ovnkube-config/ovnkube.conf \ | ||
| --loglevel "${OVN_KUBE_LOG_LEVEL}" \ | ||
| --enable-multicast | ||
| env: | ||
| - name: OVN_KUBE_LOG_LEVEL | ||
| value: "4" | ||
| - name: K8S_NODE | ||
| valueFrom: | ||
| fieldRef: | ||
| fieldPath: spec.nodeName | ||
| volumeMounts: | ||
| - mountPath: /run/ovnkube-config/ | ||
| name: ovnkube-config | ||
| - mountPath: /env | ||
| name: env-overrides | ||
| resources: | ||
| requests: | ||
| cpu: 10m | ||
| memory: 10Mi | ||
| terminationMessagePolicy: FallbackToLogsOnError |
There was a problem hiding this comment.
Avoid inheriting host networking without hardening this new container.
This new container only mounts config/env, but it is added to a pod with hostNetwork: true and has no explicit restricted securityContext, limits, or probes. Split it into a restricted workload if host networking is not required; otherwise document the exception and add the missing hardening.
As per coding guidelines, "No hostPID, hostNetwork, hostIPC, privileged: true", "securityContext: runAsNonRoot, readOnlyRootFilesystem, allowPrivilegeEscalation: false", "Drop ALL capabilities", "Resource limits (cpu, memory) on every container", and "Liveness + readiness probes defined" are required.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@assets/components/ovn/multi-node/master/daemonset.yaml` around lines 356 -
392, The ovnkube-cluster-manager container is running in a pod with hostNetwork:
true but lacks required security hardening. Add a securityContext block to this
container with runAsNonRoot: true, readOnlyRootFilesystem: true, and
allowPrivilegeEscalation: false; add a capabilities section to drop ALL
capabilities; add resource limits (both cpu and memory limits in addition to the
existing requests); and add both liveness and readiness probes with appropriate
initial delays and timeouts. If hostNetwork is not actually required for this
container's operation, consider moving it to a separate restricted deployment or
pod without host networking to minimize the security surface.
Source: Coding guidelines
| # ovnkube cluster-manager: allocates subnets to nodes, handles cluster-wide IPAM | ||
| - name: ovnkube-cluster-manager | ||
| image: {{ .ReleaseImage.ovn_kubernetes_microshift }} | ||
| command: | ||
| - /bin/bash | ||
| - -c | ||
| - | | ||
| set -xe | ||
| if [[ -f "/env/_master" ]]; then | ||
| set -o allexport | ||
| source "/env/_master" | ||
| set +o allexport | ||
| fi | ||
|
|
||
| echo "$(date -Iseconds) - starting ovnkube-cluster-manager, Node: ${K8S_NODE}" | ||
| exec /usr/bin/ovnkube \ | ||
| --init-cluster-manager "${K8S_NODE}" \ | ||
| --config-file=/run/ovnkube-config/ovnkube.conf \ | ||
| --loglevel "${OVN_KUBE_LOG_LEVEL}" \ | ||
| --enable-multicast | ||
| env: | ||
| - name: OVN_KUBE_LOG_LEVEL | ||
| value: "4" | ||
| - name: K8S_NODE | ||
| valueFrom: | ||
| fieldRef: | ||
| fieldPath: spec.nodeName | ||
| volumeMounts: | ||
| - mountPath: /run/ovnkube-config/ | ||
| name: ovnkube-config | ||
| - mountPath: /env | ||
| name: env-overrides | ||
| resources: | ||
| requests: | ||
| cpu: 10m | ||
| memory: 10Mi | ||
| terminationMessagePolicy: FallbackToLogsOnError |
There was a problem hiding this comment.
Avoid inheriting host namespaces without hardening this new container.
This new container only mounts config/env, but it is added to a pod with hostNetwork: true and hostPID: true, and has no explicit restricted securityContext, limits, or probes. Split it into a restricted workload if those host namespaces are not required; otherwise document the exception and add the missing hardening.
As per coding guidelines, "No hostPID, hostNetwork, hostIPC, privileged: true", "securityContext: runAsNonRoot, readOnlyRootFilesystem, allowPrivilegeEscalation: false", "Drop ALL capabilities", "Resource limits (cpu, memory) on every container", and "Liveness + readiness probes defined" are required.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@assets/components/ovn/single-node/master/daemonset.yaml` around lines 298 -
334, The ovnkube-cluster-manager container is running in a pod with host
namespaces (hostNetwork and hostPID) but lacks required security hardening. Add
a securityContext section to this container that includes runAsNonRoot set to
true, readOnlyRootFilesystem set to true, and allowPrivilegeEscalation set to
false. Additionally, add a capabilities drop for ALL, define both CPU and memory
resource limits (in addition to the existing requests), and include liveness and
readiness probes to meet the coding security guidelines.
Source: Coding guidelines
53d6923 to
cd0e279
Compare
|
/test ocp-full-conformance-serial-rhel-eus |
|
1 similar comment
|
/test ocp-full-conformance-rhel-eus |
|
@pacevedom: all tests passed! 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. |
Summary by CodeRabbit