OCPEDGE-2197: Promote TNF (Dual Replica) to default#2490
OCPEDGE-2197: Promote TNF (Dual Replica) to default#2490fonta-rh wants to merge 3 commits intoopenshift:masterfrom
Conversation
|
Hello @fonta-rh! Some important instructions when contributing to openshift/api: |
|
Skipping CI for Draft Pull Request. |
|
/test verify-feature-promotion |
1 similar comment
|
/test verify-feature-promotion |
|
/assign |
|
@fonta-rh Let me know when you believe this is ready for a review. I took a quick look at the promotion verification job and it looks like there is some more runs to be gathered. |
|
Hello @everettraven ! Yeah, we're still missing runs and % of success. I removed the draft as I don't expect any more changes on our side and wanted to make sure the rest of the CI was not hiding some trouble. We'll keep looking at this and contact you when it's passing verify-feature-promotion. Thanks! |
|
/test verify-feature-promotion |
4 similar comments
|
/test verify-feature-promotion |
|
/test verify-feature-promotion |
|
/test verify-feature-promotion |
|
/test verify-feature-promotion |
|
PR-Agent: could not fine a component named |
📝 WalkthroughWalkthroughThe change reintroduces and repositions the DualReplica feature row in features.md, expands the FeatureGateDualReplica enablement to include inDefault() and inOKD() in features/features.go, adds 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 golangci-lint (2.11.4)Error: build linters: unable to load custom analyzer "kubeapilinter": tools/_output/bin/kube-api-linter.so, plugin: not implemented Comment |
|
/test verify-feature-promotion |
|
PR-Agent: could not fine a component named |
9b5881e to
3837789
Compare
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@features.md`:
- Line 81: The DualReplica table row in features.md violates MD060 (inconsistent
table pipe spacing) — fix the "DualReplica" row by normalizing pipe spacing so
each cell has a single space after the opening pipe and a single space before
the closing pipe, remove any stray/empty pipes so the row has the same number of
columns as the header, and ensure each cell content (e.g., "Enabled") matches
the surrounding spacing convention used by the other rows.
|
/test verify-feature-promotion |
|
PR-Agent: could not fine a component named |
|
/test verify-feature-promotion |
|
PR-Agent: could not fine a component named |
|
/test verify-feature-promotion |
ⓘ Your monthly quota for Qodo has expired. Upgrade your plan ⓘ Paying users. Check that your Qodo account is linked with this Git user account |
|
PR needs rebase. 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. |
|
/test verify-feature-promotion |
ⓘ Your monthly quota for Qodo has expired. Upgrade your plan ⓘ Paying users. Check that your Qodo account is linked with this Git user account |
103fd50 to
c835c7f
Compare
|
@fonta-rh: This pull request references OCPEDGE-2197 which is a valid 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. |
All features enabled in Default must also be enabled in OKD to maintain feature parity with the community distribution. This fixes the failing TestOKDHasAllDefaultFeatureGates unit test. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
/retest-required |
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
c835c7f to
b6febf6
Compare
| api.openshift.io/merged-by-featuregates: "true" | ||
| include.release.openshift.io/ibm-cloud-managed: "true" | ||
| include.release.openshift.io/self-managed-high-availability: "true" | ||
| release.openshift.io/feature-set: CustomNoUpgrade,DevPreviewNoUpgrade,TechPreviewNoUpgrade |
There was a problem hiding this comment.
nvm, it's tied to the featuregate, we should be good
There was a problem hiding this comment.
🧹 Nitpick comments (1)
payload-manifests/crds/0000_80_machine-config_01_controllerconfigs-Default.crd.yaml (1)
1355-1362: DocumentDualReplicain thecontrolPlaneTopologydescriptionThe enum now includes
DualReplica, but the description block does not explain it. Please add a short description so generated API docs stay accurate.Proposed patch
The 'HighlyAvailableArbiter' mode indicates that the control plane will consist of 2 control-plane nodes that run conventional services and 1 smaller sized arbiter node that runs a bare minimum of services to maintain quorum. + The 'DualReplica' mode indicates that the control plane consists of two control-plane nodes + and relies on fencing to avoid split-brain in two-node deployments. enum: - HighlyAvailable - HighlyAvailableArbiter - SingleReplica - DualReplicaAs per coding guidelines, "Focus on major issues impacting performance, readability, maintainability and security. Avoid nitpicks and avoid verbosity."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@payload-manifests/crds/0000_80_machine-config_01_controllerconfigs-Default.crd.yaml` around lines 1355 - 1362, The controlPlaneTopology description is missing an entry for the new enum value DualReplica; update the description block that explains controlPlaneTopology (the prose above the enum containing HighlyAvailable, HighlyAvailableArbiter, SingleReplica, DualReplica, External) to include a short sentence describing DualReplica — e.g., "DualReplica indicates the control plane consists of two full-sized control-plane replicas (two control-plane nodes providing replicated control-plane services) without an arbiter" — so the generated API docs accurately document the new enum value.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@payload-manifests/crds/0000_80_machine-config_01_controllerconfigs-Default.crd.yaml`:
- Around line 1355-1362: The controlPlaneTopology description is missing an
entry for the new enum value DualReplica; update the description block that
explains controlPlaneTopology (the prose above the enum containing
HighlyAvailable, HighlyAvailableArbiter, SingleReplica, DualReplica, External)
to include a short sentence describing DualReplica — e.g., "DualReplica
indicates the control plane consists of two full-sized control-plane replicas
(two control-plane nodes providing replicated control-plane services) without an
arbiter" — so the generated API docs accurately document the new enum value.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: 9ecc2581-dcbc-4f5c-9775-6a9d5d327c9b
⛔ Files ignored due to path filters (5)
config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_infrastructures-Default.crd.yamlis excluded by!**/zz_generated.crd-manifests/*config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_infrastructures-OKD.crd.yamlis excluded by!**/zz_generated.crd-manifests/*etcd/v1alpha1/zz_generated.crd-manifests/0000_25_etcd_01_pacemakerclusters.crd.yamlis excluded by!**/zz_generated.crd-manifests/*machineconfiguration/v1/zz_generated.crd-manifests/0000_80_machine-config_01_controllerconfigs-Default.crd.yamlis excluded by!**/zz_generated.crd-manifests/*machineconfiguration/v1/zz_generated.crd-manifests/0000_80_machine-config_01_controllerconfigs-OKD.crd.yamlis excluded by!**/zz_generated.crd-manifests/*
📒 Files selected for processing (11)
features.mdfeatures/features.gopayload-manifests/crds/0000_10_config-operator_01_infrastructures-Default.crd.yamlpayload-manifests/crds/0000_10_config-operator_01_infrastructures-OKD.crd.yamlpayload-manifests/crds/0000_25_etcd_01_pacemakerclusters.crd.yamlpayload-manifests/crds/0000_80_machine-config_01_controllerconfigs-Default.crd.yamlpayload-manifests/crds/0000_80_machine-config_01_controllerconfigs-OKD.crd.yamlpayload-manifests/featuregates/featureGate-4-10-Hypershift-Default.yamlpayload-manifests/featuregates/featureGate-4-10-Hypershift-OKD.yamlpayload-manifests/featuregates/featureGate-4-10-SelfManagedHA-Default.yamlpayload-manifests/featuregates/featureGate-4-10-SelfManagedHA-OKD.yaml
💤 Files with no reviewable changes (1)
- payload-manifests/crds/0000_25_etcd_01_pacemakerclusters.crd.yaml
✅ Files skipped from review due to trivial changes (1)
- features.md
🚧 Files skipped from review as they are similar to previous changes (1)
- features/features.go
|
@fonta-rh: The following test failed, say
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. |
Promote DualReplica to default.
Two nodes with Fencing (DualReplica) allows deploying of a two-node OpenShift cluster with the necessary fencing tooling to prevent split-brain.
Enhancement Proposal: openshift/enhancements#1675