AGENT-1522: Graduate IRI object from v1alpha1 to V1#2863
Conversation
|
Pipeline controller notification For optional jobs, comment This repository is configured in: LGTM mode |
openshift-ci-robot
added
the
jira/valid-reference
|
@sadasu: This pull request references AGENT-1522 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 "5.0.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 @sadasu! Some important instructions when contributing to openshift/api: |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository YAML (base), Central YAML (inherited) Review profile: CHILL Plan: Enterprise Run ID: ⛔ Files ignored due to path filters (6)
📒 Files selected for processing (6)
💤 Files with no reviewable changes (1)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthroughThis PR introduces the InternalReleaseImage custom resource as a stable v1 API type. The change defines a cluster-scoped singleton resource that manages references to release bundles, with spec validation constraining up to 16 named references following an 🚥 Pre-merge checks | ✅ 14 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (14 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.12.2)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 |
- Created machineconfiguration/v1/types_internalreleaseimage.go with v1 API - Updated compatibility level from 4 (alpha) to 1 (stable/GA) - Registered InternalReleaseImage types in v1 scheme - Created v1 integration test suite - Updated payload CRD script to use v1 instead of v1alpha1 - Generated all CRD manifests, deepcopy, and swagger docs Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
machineconfiguration/v1/tests/internalreleaseimages.machineconfiguration.openshift.io/NoRegistryClusterInstall.yaml (1)
6-88: ⚡ Quick winAdd a negative create test for singleton name enforcement.
Please add an
onCreatecase wheremetadata.nameis notclusterand assert the XValidation rejection. This guards the singleton contract introduced on the type.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@machineconfiguration/v1/tests/internalreleaseimages.machineconfiguration.openshift.io/NoRegistryClusterInstall.yaml` around lines 6 - 88, Add a new onCreate test case that attempts to create an InternalReleaseImage whose metadata.name is not "cluster" to ensure the singleton name check rejects it; create an entry (e.g., name: "Reject non-cluster singleton name") with an initial manifest using kind: InternalReleaseImage and metadata.name: "not-cluster" (include a minimal spec.releases item), and set expectedError to assert the XValidation rejection (include "XValidation" in the expectedError string) so the test verifies the singleton name enforcement.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@machineconfiguration/v1/types_internalreleaseimage.go`:
- Around line 37-39: Update the field comments to document omission behavior and
any list-size constraints: for the top-level field Status (type
InternalReleaseImageStatus) add that if omitted the controller will treat it as
empty/unknown and will not expose any observed state; for Status.Conditions add
what omitting the slice means (no conditions are recorded) and explicitly state
any +optional semantics; for Status.Releases[].Conditions document that omitting
the conditions slice implies no recorded per-release conditions and add the
exact MinItems/MaxItems constraints from the kubebuilder markers (e.g.,
“MinItems=N” or “MaxItems=M”) to the comment so consumers know list-size limits;
update the comments adjacent to the fields Status,
InternalReleaseImageStatus.Conditions, and the Releases[].Conditions field to
include both omission behavior and the MinItems/MaxItems text.
---
Nitpick comments:
In
`@machineconfiguration/v1/tests/internalreleaseimages.machineconfiguration.openshift.io/NoRegistryClusterInstall.yaml`:
- Around line 6-88: Add a new onCreate test case that attempts to create an
InternalReleaseImage whose metadata.name is not "cluster" to ensure the
singleton name check rejects it; create an entry (e.g., name: "Reject
non-cluster singleton name") with an initial manifest using kind:
InternalReleaseImage and metadata.name: "not-cluster" (include a minimal
spec.releases item), and set expectedError to assert the XValidation rejection
(include "XValidation" in the expectedError string) so the test verifies the
singleton name enforcement.
🪄 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), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: 2f0a2a5c-6ed0-4ec5-82b7-5b2132b9d70d
⛔ Files ignored due to path filters (12)
machineconfiguration/v1/zz_generated.crd-manifests/0000_80_machine-config_01_internalreleaseimages-Hypershift.crd.yamlis excluded by!**/zz_generated.crd-manifests/*machineconfiguration/v1/zz_generated.crd-manifests/0000_80_machine-config_01_internalreleaseimages-SelfManagedHA.crd.yamlis excluded by!**/zz_generated.crd-manifests/*machineconfiguration/v1/zz_generated.crd-manifests/0000_80_machine-config_01_machineconfignodes-Hypershift-Default.crd.yamlis excluded by!**/zz_generated.crd-manifests/*machineconfiguration/v1/zz_generated.crd-manifests/0000_80_machine-config_01_machineconfignodes-Hypershift-OKD.crd.yamlis excluded by!**/zz_generated.crd-manifests/*machineconfiguration/v1/zz_generated.crd-manifests/0000_80_machine-config_01_machineconfignodes-SelfManagedHA-Default.crd.yamlis excluded by!**/zz_generated.crd-manifests/*machineconfiguration/v1/zz_generated.crd-manifests/0000_80_machine-config_01_machineconfignodes-SelfManagedHA-OKD.crd.yamlis excluded by!**/zz_generated.crd-manifests/*machineconfiguration/v1/zz_generated.deepcopy.gois excluded by!**/zz_generated*machineconfiguration/v1/zz_generated.featuregated-crd-manifests.yamlis excluded by!**/zz_generated*machineconfiguration/v1/zz_generated.featuregated-crd-manifests/internalreleaseimages.machineconfiguration.openshift.io/NoRegistryClusterInstall.yamlis excluded by!**/zz_generated.featuregated-crd-manifests/**machineconfiguration/v1/zz_generated.swagger_doc_generated.gois excluded by!**/zz_generated*machineconfiguration/v1alpha1/zz_generated.crd-manifests/0000_80_machine-config_01_internalreleaseimages-SelfManagedHA.crd.yamlis excluded by!**/zz_generated.crd-manifests/*openapi/openapi.jsonis excluded by!openapi/**
📒 Files selected for processing (14)
features.mdfeatures/features.gohack/update-payload-crds.shmachineconfiguration/v1/register.gomachineconfiguration/v1/tests/internalreleaseimages.machineconfiguration.openshift.io/NoRegistryClusterInstall.yamlmachineconfiguration/v1/types_internalreleaseimage.gopayload-manifests/crds/0000_80_machine-config_01_internalreleaseimages-Hypershift.crd.yamlpayload-manifests/crds/0000_80_machine-config_01_internalreleaseimages-SelfManagedHA.crd.yamlpayload-manifests/crds/0000_80_machine-config_01_machineconfignodes-Hypershift-Default.crd.yamlpayload-manifests/crds/0000_80_machine-config_01_machineconfignodes-Hypershift-OKD.crd.yamlpayload-manifests/crds/0000_80_machine-config_01_machineconfignodes-SelfManagedHA-Default.crd.yamlpayload-manifests/crds/0000_80_machine-config_01_machineconfignodes-SelfManagedHA-OKD.crd.yamlpayload-manifests/featuregates/featureGate-4-10-SelfManagedHA-Default.yamlpayload-manifests/featuregates/featureGate-4-10-SelfManagedHA-OKD.yaml
💤 Files with no reviewable changes (3)
- hack/update-payload-crds.sh
- payload-manifests/crds/0000_80_machine-config_01_machineconfignodes-Hypershift-Default.crd.yaml
- payload-manifests/crds/0000_80_machine-config_01_machineconfignodes-Hypershift-OKD.crd.yaml
| // status describes the last observed state of this internal release image. | ||
| // +optional | ||
| Status InternalReleaseImageStatus `json:"status,omitempty,omitzero"` |
There was a problem hiding this comment.
Document omitted behavior and list-size constraints in field comments.
The comments for status, status.conditions, and status.releases[].conditions don’t currently document (a) what happens when omitted, and (b) the MinItems/MaxItems constraints where declared. Please add those details directly to each field comment to satisfy API doc requirements.
As per coding guidelines, “For each field with +optional marker, document the behavior when the field is omitted” and “For each field with +kubebuilder:validation:MinItems/MaxItems markers, document the constraints in the field comment.”
Also applies to: 70-78, 103-120
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@machineconfiguration/v1/types_internalreleaseimage.go` around lines 37 - 39,
Update the field comments to document omission behavior and any list-size
constraints: for the top-level field Status (type InternalReleaseImageStatus)
add that if omitted the controller will treat it as empty/unknown and will not
expose any observed state; for Status.Conditions add what omitting the slice
means (no conditions are recorded) and explicitly state any +optional semantics;
for Status.Releases[].Conditions document that omitting the conditions slice
implies no recorded per-release conditions and add the exact MinItems/MaxItems
constraints from the kubebuilder markers (e.g., “MinItems=N” or “MaxItems=M”) to
the comment so consumers know list-size limits; update the comments adjacent to
the fields Status, InternalReleaseImageStatus.Conditions, and the
Releases[].Conditions field to include both omission behavior and the
MinItems/MaxItems text.
|
@sadasu: 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. |
2 participants
No description provided.