Skip to content

[DNM] Use custom layered image to test nm-libreswan fix #2856

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.

Sign up for GitHub

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

Open
pperiyasamy wants to merge 1 commit into
base: master
Choose a base branch
from
Open

[DNM] Use custom layered image to test nm-libreswan fix #2856

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 4 additions & 3 deletions bindata/network/ovn-kubernetes/common/80-ipsec-master-extensions.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,11 @@ kind: MachineConfig
metadata:
labels:
machineconfiguration.openshift.io/role: master
annotations:
user-ipsec-machine-config: "true"
Comment on lines +7 to +8
Copy link

@coderabbitai coderabbitai bot Dec 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Misleading annotation name for CNO-managed resource.

The annotation user-ipsec-machine-config: "true" is misleading since this MachineConfig is managed by CNO, not user-defined. This could cause confusion about resource ownership and lifecycle management.

Consider renaming to reflect CNO ownership, e.g., cno-ipsec-machine-config: "true" or network.operator.openshift.io/ipsec-machine-config: "true".

🤖 Prompt for AI Agents 
In bindata/network/ovn-kubernetes/common/80-ipsec-master-extensions.yaml around
lines 7-8, the annotation key user-ipsec-machine-config is misleading for a
CNO-managed MachineConfig; replace or rename it to indicate CNO ownership (for
example cno-ipsec-machine-config: "true" or
network.operator.openshift.io/ipsec-machine-config: "true") so the annotation
accurately reflects resource ownership and lifecycle; update any consumers or
documentation that reference the old key to use the new annotation name.

name: 80-ipsec-master-extensions
spec:
osImageURL: quay.io/pepalani/ipsec-rhcos-layered-image:nm-libreswan-fix
Copy link

@coderabbitai coderabbitai bot Dec 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Hardcoded test image URL must not be merged.

The osImageURL points to a personal registry (quay.io/pepalani/...) which is not suitable for production. Since this PR is marked [DNM] for testing the nm-libreswan fix, ensure this reference is replaced with an official image registry before any production use.

🤖 Prompt for AI Agents 
In bindata/network/ovn-kubernetes/common/80-ipsec-master-extensions.yaml around
line 11, the osImageURL is hardcoded to a personal test image
(quay.io/pepalani/ipsec-rhcos-layered-image:nm-libreswan-fix); replace this test
registry reference with the official, approved image registry and tag used by
the project (or parameterize the value via config/CI variables), ensure no
personal quay.io references remain, and update any documentation/tests to use
the official image before merging to production.

config:
ignition:
version: 3.2.0
Expand All @@ -20,11 +23,9 @@ spec:

[Service]
Type=oneshot
ExecStartPre=rm -f /etc/ipsec.d/cno.conf
ExecStartPre=systemd-tmpfiles --create /usr/lib/rpm-ostree/tmpfiles.d/libreswan.conf
ExecStart=systemctl enable --now ipsec.service

[Install]
WantedBy=multi-user.target
extensions:
- ipsec
{{end}}
7 changes: 4 additions & 3 deletions bindata/network/ovn-kubernetes/common/80-ipsec-worker-extensions.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,11 @@ kind: MachineConfig
metadata:
labels:
machineconfiguration.openshift.io/role: worker
annotations:
user-ipsec-machine-config: "true"
Comment on lines +7 to +8
Copy link

@coderabbitai coderabbitai bot Dec 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Misleading annotation name for CNO-managed resource.

The annotation user-ipsec-machine-config: "true" is misleading since this MachineConfig is managed by CNO, not user-defined. This could cause confusion about resource ownership and lifecycle management.

Consider renaming to reflect CNO ownership, e.g., cno-ipsec-machine-config: "true" or network.operator.openshift.io/ipsec-machine-config: "true".

🤖 Prompt for AI Agents 
In bindata/network/ovn-kubernetes/common/80-ipsec-worker-extensions.yaml around
lines 7-8, the annotation key `user-ipsec-machine-config: "true"` is misleading
for a CNO-managed MachineConfig; rename the annotation to indicate CNO ownership
(for example `cno-ipsec-machine-config: "true"` or
`network.operator.openshift.io/ipsec-machine-config: "true"`) and update any
consuming code, documentation, and tests to use the new annotation key so
ownership and lifecycle intent are unambiguous.

name: 80-ipsec-worker-extensions
spec:
osImageURL: quay.io/pepalani/ipsec-rhcos-layered-image:nm-libreswan-fix
Copy link

@coderabbitai coderabbitai bot Dec 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Hardcoded test image URL must not be merged.

The osImageURL points to a personal registry (quay.io/pepalani/...) which is not suitable for production. Since this PR is marked [DNM] for testing the nm-libreswan fix, ensure this reference is replaced with an official image registry before any production use.

🤖 Prompt for AI Agents 
In bindata/network/ovn-kubernetes/common/80-ipsec-worker-extensions.yaml around
line 11 the osImageURL is hardcoded to a personal test registry
(quay.io/pepalani/ipsec-rhcos-layered-image:nm-libreswan-fix); replace this
value with the official, approved production image reference (or a configurable
variable) before merging — update the field to point to the canonical
registry/image tag used by the project (or read from environment/params) and
remove any personal/test registry references.

config:
ignition:
version: 3.2.0
Expand All @@ -20,11 +23,9 @@ spec:

[Service]
Type=oneshot
ExecStartPre=rm -f /etc/ipsec.d/cno.conf
ExecStartPre=systemd-tmpfiles --create /usr/lib/rpm-ostree/tmpfiles.d/libreswan.conf
ExecStart=systemctl enable --now ipsec.service

[Install]
WantedBy=multi-user.target
extensions:
- ipsec
{{end}}
7 changes: 3 additions & 4 deletions pkg/network/ovn_kubernetes.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -648,15 +648,15 @@ func shouldRenderIPsec(conf *operv1.OVNKubernetesConfig, bootstrapResult *bootst
isHypershiftHostedCluster := bootstrapResult.Infra.HostedControlPlane != nil
isOVNIPsecActiveOrRollingOut := bootstrapResult.OVN.IPsecUpdateStatus != nil && bootstrapResult.OVN.IPsecUpdateStatus.IsOVNIPsecActiveOrRollingOut
isCNOIPsecMachineConfigPresent := isCNOIPsecMachineConfigPresent(bootstrapResult.Infra)
isUserDefinedIPsecMachineConfigPresent := isUserDefinedIPsecMachineConfigPresent(bootstrapResult.Infra)
//isUserDefinedIPsecMachineConfigPresent := isUserDefinedIPsecMachineConfigPresent(bootstrapResult.Infra)
isIPsecMachineConfigActive := isIPsecMachineConfigActive(bootstrapResult.Infra)
isMachineConfigClusterOperatorReady := bootstrapResult.Infra.MachineConfigClusterOperatorReady

mode := GetIPsecMode(conf)

// When OVN is rolling out, OVN IPsec might be fully or partially active or inactive.
// If MachineConfigs are not present, we know its inactive since we only stop rendering them once inactive.
isOVNIPsecActive := isOVNIPsecActiveOrRollingOut && (isCNOIPsecMachineConfigPresent || isUserDefinedIPsecMachineConfigPresent || isHypershiftHostedCluster)
isOVNIPsecActive := isOVNIPsecActiveOrRollingOut && (isCNOIPsecMachineConfigPresent || isHypershiftHostedCluster)

// We render the ipsec deployment if IPsec is already active in OVN
// or if EW IPsec config is enabled.
Expand All @@ -667,8 +667,7 @@ func shouldRenderIPsec(conf *operv1.OVNKubernetesConfig, bootstrapResult *bootst
// - not needed for the containerized deployment is used in hypershift
// hosted clusters
// - not needed if the user already created their own
renderCNOIPsecMachineConfig = (mode != operv1.IPsecModeDisabled || renderIPsecDaemonSet) && !isHypershiftHostedCluster &&
!isUserDefinedIPsecMachineConfigPresent
renderCNOIPsecMachineConfig = (mode != operv1.IPsecModeDisabled || renderIPsecDaemonSet) && !isHypershiftHostedCluster
// Wait for MCO to be ready unless we had already rendered the IPsec MachineConfig.
renderCNOIPsecMachineConfig = renderCNOIPsecMachineConfig && (isCNOIPsecMachineConfigPresent || isMachineConfigClusterOperatorReady)

Expand Down