Skip to content

Preserve user EDPM registry configuration - #2071

Open
rabi wants to merge 1 commit into
openstack-k8s-operators:mainfrom
rabi:OSPRH-36723
Open

rabi wants to merge 1 commit into
openstack-k8s-operators:mainfrom
rabi:OSPRH-36723

Conversation

@rabi

@rabi rabi commented Sep 10, 2026

Copy link
Copy Markdown
Contributor

Sometimes users explicitly set edpm_podman_registries_conf and edpm_podman_disconnected_ocp to false in OpenStackDataPlaneNodeSet spec even if cluster has IDMS objects which we assume as disconnected deployment. Let's give preference to user set ansible vars.

Jira: OSPRH-30371

Sometimes users explicitly set edpm_podman_registries_conf and
edpm_podman_disconnected_ocp to false in OpenStackDataPlaneNodeSet
spec even if cluster has IDMS objects which we assume as disconnected
deployment. Let's give preference to user set ansible vars.

Signed-off-by: rabi <ramishra@redhat.com>
@openshift-ci
openshift-ci Bot requested review from abays and fultonj September 10, 2026 10:28
@openshift-ci

openshift-ci Bot commented Sep 10, 2026

Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: rabi

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

The pull request process is described 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 Sep 10, 2026

Copy link
Copy Markdown

Review Change StackReview Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Central YAML (base), Organization UI (inherited)

Review profile: CHILL

Plan: Advanced

Run ID: 41f05c1b-af93-4da1-ab8e-e8be0c2cf038

📥 Commits

Reviewing files that changed from the base of the PR and between e7a4aaa and 88828b2.

📒 Files selected for processing (2)
  • internal/dataplane/inventory.go
  • internal/dataplane/inventory_test.go

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.


📝 Summary

Summary by CodeRabbit

  • Bug Fixes

    • Preserved user-provided Podman registry configuration values when mirror registries are detected.
    • Applied registry configuration defaults only when corresponding values are not already set.
    • Updated messaging to clarify that OCP registry configuration defaults are being applied.
  • Tests

    • Added coverage for default configuration behavior and independent user overrides.

Walkthrough

Inventory generation now preserves user-provided Podman registry variables. It applies detected mirror registry values only when the variables are absent. Tests cover default assignment and independent overrides.

Changes

Podman registry defaults

Layer / File(s) Summary
Default registry configuration
internal/dataplane/inventory.go, internal/dataplane/inventory_test.go
Inventory generation uses setDefaultPodmanRegistryConfigVars to preserve existing values and set missing registry defaults. Table-driven tests cover both variables independently.

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

Merge Risk: ⚪ Minimal · up to 88828

This change correctly preserves user-specified Podman registry settings while still supplying inferred defaults when they are absent. The covered behavior presents no actionable merge-blocking risk.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 42.86% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 7 functions across 2 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
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.
Title check ✅ Passed The title clearly describes the main change: preserving user-provided EDPM registry configuration.
Description check ✅ Passed The description directly explains the registry configuration precedence change for IDMS-based disconnected deployments.
  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

@github-actions

Copy link
Copy Markdown

OpenStackControlPlane CRD Size Report

Metric Value
CRD JSON size 338280 bytes (330KB)
Base branch size 338314 bytes
Change -0.01%
Status yellow — growing
Threshold reference
Color Range Meaning
🟢 green < 300KB Comfortable
🟡 yellow 300–400KB Growing
🟠 orange 400–750KB Concerning
🔴 red > 750KB Approaching 1.5MB etcd limit (cut in half to allow space for update)

@centosinfra-prod-github-app

Copy link
Copy Markdown

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant