Skip to content

Add configurable OVN controller probes - #690

Open
mtomaska wants to merge 1 commit into
openstack-k8s-operators:mainfrom
mtomaska:osprh-33460
Open

mtomaska wants to merge 1 commit into
openstack-k8s-operators:mainfrom
mtomaska:osprh-33460

Conversation

@mtomaska

Copy link
Copy Markdown
Contributor

This change allows user to configure ovn-controller pod probe intervals (Readiness, liveliness and startup) via CR.
Before this change, these timeout values were hardcoded. There are cases where user need to bump these values when deployment systems are large and ovn-controller take a long time to do initial computations. For example, if pods are restarted

Resolves-Jira: https://redhat.atlassian.net/browse/OSPRH-33460
Assisted-By: GPT-6

Preserve existing defaults and support optional startup probes.

Assisted-By: GPT-6
@openshift-ci

openshift-ci Bot commented Oct 26, 2026

Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: mtomaska
Once this PR has been reviewed and has the lgtm label, please assign averdagu 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

@coderabbitai

coderabbitai Bot commented Oct 26, 2026

Copy link
Copy Markdown

Important

  • 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Central YAML (base), Organization UI (inherited)
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 7e3941c9-298b-4d74-97dd-f215e4f37382
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

@centosinfra-prod-github-app

Copy link
Copy Markdown

Build succeeded (check pipeline).
https://gateway-cloud-softwarefactory.apps.ocp.cloud.ci.centos.org/zuul/t/rdoproject.org/buildset/8c165167778545d6af7358d3ea757a34

✔️ openstack-k8s-operators-content-provider SUCCESS in 3h 23m 34s
✔️ ovn-operator-tempest-multinode SUCCESS in 1h 42m 51s

Warning:
File ansible/tmp/b38808d90d534d619860fa27c88d7d10 found in build home directory during playbook github.com/openstack-k8s-operators/ci-framework/ci/playbooks/collect-logs.yml for job ovn-operator-tempest-multinode. Add to preserve-home-paths if safe, or otherwise remove.
File ansible/tmp/81ef9acc35444271ac863b62aac8060f found in build home directory during playbook github.com/openstack-k8s-operators/ci-framework/ci/playbooks/collect-logs.yml for job ovn-operator-tempest-multinode. Add to preserve-home-paths if safe, or otherwise remove.
File ansible/tmp/b38808d90d534d619860fa27c88d7d10 found in build home directory during playbook github.com/openstack-k8s-operators/ci-framework/ci/playbooks/multinode-autohold.yml for job ovn-operator-tempest-multinode. Add to preserve-home-paths if safe, or otherwise remove.
File ansible/tmp/81ef9acc35444271ac863b62aac8060f found in build home directory during playbook github.com/openstack-k8s-operators/ci-framework/ci/playbooks/multinode-autohold.yml for job ovn-operator-tempest-multinode. Add to preserve-home-paths if safe, or otherwise remove.
File ansible/tmp/b38808d90d534d619860fa27c88d7d10 found in build home directory during playbook review.rdoproject.org/config/playbooks/crc/print-machineconfigpool.yaml for job ovn-operator-tempest-multinode. Add to preserve-home-paths if safe, or otherwise remove.
File .ssh/known_hosts.old found in build home directory during playbook review.rdoproject.org/config/playbooks/base-minimal/post-logs.yaml for job ovn-operator-tempest-multinode. Add to preserve-home-paths if safe, or otherwise remove.

@fernandoroyosanchez fernandoroyosanchez left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Just a non blocking comment, the rest lgtm

errors = append(errors, errs...)
continue
}
config.Merge(*probe.override)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

config.Merge assigns Command by reference (lib-common ProbeConf.Merge: p.Command = overrides.Command), and it flows straight into the final corev1.Probe.Exec.Command. Since probe.override here is spec.Override.Probes.*.Command from the CR spec, a custom command ends up aliasing the cached CR's backing array instead of being copied. The existing test only covers the default-command path, not this one.

Suggest copying the slice before merging (supplied.Command = append([]string(nil), supplied.Command...)), or add a test proving the built probe's Command isn't the same slice as the CR spec's.

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.

2 participants