Skip to content

Split wait-for-ipsec-connect into two services to avoid ipsec restart - #6440

Open
pperiyasamy wants to merge 1 commit into
openshift:mainfrom
pperiyasamy:configure_ipsec_connect_service
Open

Split wait-for-ipsec-connect into two services to avoid ipsec restart#6440
pperiyasamy wants to merge 1 commit into
openshift:mainfrom
pperiyasamy:configure_ipsec_connect_service

Conversation

@pperiyasamy

@pperiyasamy pperiyasamy commented Aug 22, 2026

Copy link
Copy Markdown
Member

Move the openshift.conf config update (adding auto=start to outbound connections) into a new configure-ipsec-connect.service that runs before ipsec.service. This eliminates the unnecessary ipsec restart that was previously done in wait-for-ipsec-connect.service after ipsec had already started.

Summary by CodeRabbit

  • New Features

    • Added automatic configuration for OVN outbound IPsec connections to start when the IPsec service launches.
    • Added a startup service to apply this configuration before IPsec begins.
  • Bug Fixes

    • Removed legacy fallback behavior that modified configuration files and restarted IPsec when Open vSwitch IPsec was not configured.

Move the openshift.conf config update (adding auto=start to outbound
connections) into a new configure-ipsec-connect.service that runs before
ipsec.service. This eliminates the unnecessary ipsec restart that was
previously done in wait-for-ipsec-connect.service after ipsec had
already started.

Signed-off-by: Periyasamy Palanisamy <pepalani@redhat.com>
@openshift-merge-bot

Copy link
Copy Markdown
Contributor

Pipeline controller notification
This repo is configured to use the pipeline controller. Second-stage tests will be triggered either automatically or after lgtm label is added, depending on the repository configuration. The pipeline controller will automatically detect which contexts are required and will utilize /test Prow commands to trigger the second stage.

For optional jobs, comment /test ? to see a list of all defined jobs. To trigger manually all jobs from second stage use /pipeline required command.

This repository is configured in: LGTM mode

@coderabbitai

coderabbitai Bot commented Aug 22, 2026

Copy link
Copy Markdown

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 23ad8ecc-2bbc-4c96-af12-6605903b4f70

📥 Commits

Reviewing files that changed from the base of the PR and between 7ff337a and 62e50ea.

📒 Files selected for processing (3)
  • templates/common/_base/files/configure-ipsec-connect.yaml
  • templates/common/_base/files/wait-for-ipsec-connect.yaml
  • templates/common/_base/units/configure-ipsec-connect.service.yaml
💤 Files with no reviewable changes (1)
  • templates/common/_base/files/wait-for-ipsec-connect.yaml

Included review availability: Your plan provides up to 12 included reviews per hour; 11 remain after this review.


Walkthrough

The change adds a startup script and systemd service for OVN IPsec connections. The script conditionally adds auto=start before ipsec.service starts. The previous openshift.conf fallback path is removed.

Changes

IPsec startup configuration

Layer / File(s) Summary
Configure OVN connections before IPsec startup
templates/common/_base/files/configure-ipsec-connect.yaml, templates/common/_base/units/configure-ipsec-connect.service.yaml, templates/common/_base/files/wait-for-ipsec-connect.yaml
The new script skips missing or Open vSwitch-managed configurations. Otherwise, it adds auto=start to OVN outbound connections. The enabled oneshot service runs before ipsec.service, after ovs-configuration.service, and logs to the journal and console. The previous openshift.conf fallback and service restart are removed.

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: ⚪ Minimal · up to 62e50

This change separates IPsec configuration from connection waiting so the service no longer needs to restart IPsec after startup; no actionable merge-blocking risk remains after normal checks and review.

Sequence Diagram(s)

sequenceDiagram
  participant systemd
  participant configure-ipsec-connect.service
  participant configure-ipsec-connect.sh
  participant ipsec.conf
  participant ipsec.service
  systemd->>configure-ipsec-connect.service: Start before ipsec.service
  configure-ipsec-connect.service->>configure-ipsec-connect.sh: Run configuration script
  configure-ipsec-connect.sh->>ipsec.conf: Check configuration ownership
  configure-ipsec-connect.sh->>ipsec.conf: Add auto=start when required
  systemd->>ipsec.service: Start after configuration completes
Loading
🚥 Pre-merge checks | ✅ 15
✅ Passed checks (15 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes splitting the service to eliminate the unnecessary IPsec restart.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0 files. (2 skipped: 2 unsupported.)
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Stable And Deterministic Test Names ✅ Passed The PR changes only IPsec shell and systemd YAML templates. The exact diff contains no Ginkgo test files or It/Describe/Context/When test titles.
Test Structure And Quality ✅ Passed The PR changes only three IPsec YAML templates and adds no Ginkgo test code, so the listed test-quality requirements are not applicable.
Microshift Test Compatibility ✅ Passed The parent-to-HEAD diff changes only two YAML file templates and one systemd unit; it adds no Ginkgo tests or MicroShift-incompatible test usage.
Single Node Openshift (Sno) Test Compatibility ✅ Passed The pull request changes only IPsec YAML templates and contains no new Ginkgo tests or multi-node test assumptions.
Topology-Aware Scheduling Compatibility ✅ Passed The commit only adds host IPsec scripts and a systemd unit, and removes restart logic; it adds no pod scheduling constraints, topology logic, or workload placement changes.
Ote Binary Stdout Contract ✅ Passed The commit changes only IPsec YAML templates and a systemd shell service; it does not change any OTE Go binary, main(), or suite setup stdout behavior.
Ipv6 And Disconnected Network Test Compatibility ✅ Passed The commit changes only three IPsec YAML files and adds no Ginkgo e2e tests or test constructs, so this check is not applicable.
No-Weak-Crypto ✅ Passed The diff adds only IPsec configuration and service ordering; it introduces no MD5, SHA1, DES, RC4, 3DES, Blowfish, ECB, custom crypto, or secret/token comparisons.
Container-Privileges ✅ Passed The diff adds no privileged, host namespace, SYS_ADMIN, or allowPrivilegeEscalation setting. The root-level systemd script is justified by its documented update of /etc/ipsec.d/openshift.conf.
No-Sensitive-Data-In-Logs ✅ Passed The new service traces only fixed commands and paths; it emits no secrets or variable data. Existing wait-service connection logging and journal+console routing were unchanged.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands.

@openshift-ci

openshift-ci Bot commented Aug 22, 2026

Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: pperiyasamy
Once this PR has been reviewed and has the lgtm label, please assign cheesesashimi for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details 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

@pperiyasamy

Copy link
Copy Markdown
Member Author

/test ?

@pperiyasamy

Copy link
Copy Markdown
Member Author

/test e2e-aws-ovn-upgrade-ipsec

@pperiyasamy

Copy link
Copy Markdown
Member Author

/pipeline required

@openshift-merge-bot

Copy link
Copy Markdown
Contributor

Scheduling tests matching the pipeline_run_if_changed or not excluded by pipeline_skip_if_only_changed parameters:
/test e2e-aws-ovn
/test e2e-aws-ovn-upgrade
/test e2e-gcp-op-ocl-part1
/test e2e-gcp-op-ocl-part2
/test e2e-gcp-op-part1
/test e2e-gcp-op-part2
/test e2e-gcp-op-single-node
/test e2e-hypershift
/test tls-pqc-readiness

@pperiyasamy

Copy link
Copy Markdown
Member Author

/test e2e-aws-ovn-upgrade-ipsec

@openshift-ci

openshift-ci Bot commented Aug 23, 2026

Copy link
Copy Markdown
Contributor

@pperiyasamy: 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-aws-ovn-upgrade-ipsec 62e50ea link false /test e2e-aws-ovn-upgrade-ipsec
ci/prow/tls-pqc-readiness 62e50ea link true /test tls-pqc-readiness
ci/prow/e2e-aws-ovn-upgrade 62e50ea link true /test e2e-aws-ovn-upgrade
ci/prow/perfscale-control-plane-6nodes 62e50ea link false /test perfscale-control-plane-6nodes
ci/prow/e2e-gcp-op-ocl-part1 62e50ea link true /test e2e-gcp-op-ocl-part1
ci/prow/e2e-gcp-op-part2 62e50ea link true /test e2e-gcp-op-part2
ci/prow/e2e-gcp-op-part1 62e50ea link true /test e2e-gcp-op-part1
ci/prow/e2e-aws-ovn 62e50ea link true /test e2e-aws-ovn

Full PR test history. Your PR dashboard.

Details

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

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant