OTA-1967: Take official CRDs from the OpenShift Lightspeed operator#1387
Open
hongkailiu wants to merge 6 commits into
Open
OTA-1967: Take official CRDs from the OpenShift Lightspeed operator#1387hongkailiu wants to merge 6 commits into
hongkailiu wants to merge 6 commits into
Conversation
```console $ git --no-pager -C ../lightspeed-agentic-operator log --pretty=oneline -1 84a97541c3e01c7a89c96ea0acf1afc663bb5a8f (HEAD -> main, origin/main, origin/HEAD) Merge pull request #1 from harche/wt/api-pr $ cp ../lightspeed-agentic-operator/config/crd/bases/agentic.openshift.io_proposals.yaml install/0000_00_cluster-version-operator_45_lightspeed-crd-proposals.yaml $ cp ../lightspeed-agentic-operator/config/crd/bases/agentic.openshift.io_agents.yaml install/0000_00_cluster-version-operator_46_lightspeed-crd-agents.yaml $ cp ../lightspeed-agentic-operator/config/crd/bases/agentic.openshift.io_llmproviders.yaml install/0000_00_cluster-version-operator_47_lightspeed-crd-llmproviders.yaml $ cp ../lightspeed-agentic-operator/config/crd/bases/agentic.openshift.io_analysisresults.yaml install/0000_00_cluster-version-operator_47_lightspeed-crd-analysisresults.yaml $ rm install/0000_00_cluster-version-operator_47_lightspeed-crd-workflows.yaml ```
openshift-ci-robot
added
the
jira/valid-reference
openshift-ci
Bot
added
the
do-not-merge/work-in-progress
Contributor
|
Skipping CI for Draft Pull Request. |
Contributor
|
@hongkailiu: This pull request references OTA-1967 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. |
openshift-ci
Bot
removed
the
do-not-merge/work-in-progress
|
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 selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughExternalize in-repo proposal API types to ChangesAPI Externalization & CRD Migration
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 10 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (10 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Comment |
Contributor
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: hongkailiu The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci
Bot
added
the
approved
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
test/cvo/proposal.go (1)
85-86:⚠️ Potential issue | 🟠 Major | ⚡ Quick winUpdate the asserted CRD names to the new installed set.
Line 85 still expects
workflows.agentic.openshift.io, but this PR removes the workflows CRD and introducesllmproviders.agentic.openshift.ioandanalysisresults.agentic.openshift.io. The current assertion no longer matches the intended manifest set.Suggested fix
- for _, name := range []string{"proposals.agentic.openshift.io", "agents.agentic.openshift.io", "workflows.agentic.openshift.io"} { + for _, name := range []string{ + "proposals.agentic.openshift.io", + "agents.agentic.openshift.io", + "llmproviders.agentic.openshift.io", + "analysisresults.agentic.openshift.io", + } {🤖 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 `@test/cvo/proposal.go` around lines 85 - 86, Update the asserted CRD names in the loop that checks CRD existence: replace the slice used in the for-range (currently containing "proposals.agentic.openshift.io", "agents.agentic.openshift.io", "workflows.agentic.openshift.io") with the new installed set "proposals.agentic.openshift.io", "agents.agentic.openshift.io", "llmproviders.agentic.openshift.io", and "analysisresults.agentic.openshift.io" so the Get calls in apiExtensionsClient.ApiextensionsV1().CustomResourceDefinitions().Get(...) match the current manifest.pkg/proposal/controller.go (1)
113-116:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winUse the
Createreturn error directly (current code reads a staleerr).On Line 113,
c.client.Create(ctx, proposal)drops its returned error, and Line 114 checkserrfrom a previous operation. This can hide create failures and route error handling incorrectly.Suggested fix
- if c.client.Create(ctx, proposal) != nil { - if !kerrors.IsAlreadyExists(err) { + if err := c.client.Create(ctx, proposal); err != nil { + if !kerrors.IsAlreadyExists(err) { klog.V(i.Normal).Infof("Failed to create proposal %s/%s: %v", proposal.Namespace, proposal.Name, err) errs = append(errs, err) } else { klog.V(i.Debug).Infof("The proposal %s/%s existed already", proposal.Namespace, proposal.Name) } } else {🤖 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 `@pkg/proposal/controller.go` around lines 113 - 116, The call to c.client.Create(ctx, proposal) ignores its returned error and the subsequent check uses a stale variable err; update the code to capture the Create return value (e.g., err := c.client.Create(ctx, proposal)) and then use that err in the kerrors.IsAlreadyExists(err) check and in the klog.V(...).Infof/errs = append(errs, err) paths so create failures are handled correctly for the proposal object.
🧹 Nitpick comments (2)
install/0000_00_cluster-version-operator_46_lightspeed-crd-agents.yaml (1)
4-8: 💤 Low valueMissing
kubernetes.io/descriptionannotation.The manifest is missing a
kubernetes.io/descriptionannotation to explain the CRD's purpose.Suggested annotation
metadata: annotations: include.release.openshift.io/self-managed-high-availability: "true" release.openshift.io/feature-set: TechPreviewNoUpgrade + kubernetes.io/description: |- + Agent defines a cluster-scoped agent tier that configures LLM infrastructure and runtime settings. controller-gen.kubebuilder.io/version: v0.20.1 name: agents.agentic.openshift.ioAs per coding guidelines: "Use kubernetes.io/description annotations to explain the resource's purpose."
🤖 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 `@install/0000_00_cluster-version-operator_46_lightspeed-crd-agents.yaml` around lines 4 - 8, Add a kubernetes.io/description annotation to the metadata.annotations block of the lightspeed CRD manifest (the install/0000_00_cluster-version-operator_46_lightspeed-crd-agents.yaml resource) so the CRD purpose is documented; update the metadata.annotations alongside include.release.openshift.io/self-managed-high-availability and release.openshift.io/feature-set with a concise human-readable description of what the Lightspeed CRD (agents) manages and any important behavior or intent.install/0000_00_cluster-version-operator_47_lightspeed-crd-analysisresults.yaml (1)
4-9: 💤 Low valueMissing
kubernetes.io/descriptionannotation; inconsistent controller-gen version.The manifest is missing a
kubernetes.io/descriptionannotation. Also note thatcontroller-genversion here isv0.19.0while the other CRDs usev0.20.1—this inconsistency may be intentional if these come from different upstream sources, but worth verifying.Suggested annotation
metadata: annotations: include.release.openshift.io/self-managed-high-availability: "true" release.openshift.io/feature-set: TechPreviewNoUpgrade + kubernetes.io/description: |- + AnalysisResult records the output of an analysis step execution, owned by the parent Proposal. controller-gen.kubebuilder.io/version: v0.19.0 name: analysisresults.agentic.openshift.ioAs per coding guidelines: "Use kubernetes.io/description annotations to explain the resource's purpose."
🤖 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 `@install/0000_00_cluster-version-operator_47_lightspeed-crd-analysisresults.yaml` around lines 4 - 9, The manifest for the CRD "analysisresults.agentic.openshift.io" is missing a kubernetes.io/description annotation and has an inconsistent controller-gen version; add a kubernetes.io/description entry under metadata.annotations that briefly explains the resource's purpose (e.g., what AnalysisResults represent) and update controller-gen.kubebuilder.io/version from v0.19.0 to v0.20.1 to match other CRDs unless the v0.19.0 value is intentional—if intentional, add a comment or note in repo docs explaining the divergence; locate these changes in the metadata.annotations block that contains name: analysisresults.agentic.openshift.io and controller-gen.kubebuilder.io/version.
🤖 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
`@install/0000_00_cluster-version-operator_47_lightspeed-crd-llmproviders.yaml`:
- Around line 4-9: This file conflicts with another CRD using the same numeric
prefix; rename the file number (e.g., change the prefix from `_47_` to `_48_`)
so it no longer collides with
0000_00_cluster-version-operator_47_lightspeed-crd-analysisresults.yaml, and add
a kubernetes.io/description annotation under metadata.annotations for the CRD
name llmproviders.agentic.openshift.io that succinctly explains the CRD's
purpose (one-line description), ensuring deterministic ordering and compliance
with the coding guidelines.
---
Outside diff comments:
In `@pkg/proposal/controller.go`:
- Around line 113-116: The call to c.client.Create(ctx, proposal) ignores its
returned error and the subsequent check uses a stale variable err; update the
code to capture the Create return value (e.g., err := c.client.Create(ctx,
proposal)) and then use that err in the kerrors.IsAlreadyExists(err) check and
in the klog.V(...).Infof/errs = append(errs, err) paths so create failures are
handled correctly for the proposal object.
In `@test/cvo/proposal.go`:
- Around line 85-86: Update the asserted CRD names in the loop that checks CRD
existence: replace the slice used in the for-range (currently containing
"proposals.agentic.openshift.io", "agents.agentic.openshift.io",
"workflows.agentic.openshift.io") with the new installed set
"proposals.agentic.openshift.io", "agents.agentic.openshift.io",
"llmproviders.agentic.openshift.io", and "analysisresults.agentic.openshift.io"
so the Get calls in
apiExtensionsClient.ApiextensionsV1().CustomResourceDefinitions().Get(...) match
the current manifest.
---
Nitpick comments:
In `@install/0000_00_cluster-version-operator_46_lightspeed-crd-agents.yaml`:
- Around line 4-8: Add a kubernetes.io/description annotation to the
metadata.annotations block of the lightspeed CRD manifest (the
install/0000_00_cluster-version-operator_46_lightspeed-crd-agents.yaml resource)
so the CRD purpose is documented; update the metadata.annotations alongside
include.release.openshift.io/self-managed-high-availability and
release.openshift.io/feature-set with a concise human-readable description of
what the Lightspeed CRD (agents) manages and any important behavior or intent.
In
`@install/0000_00_cluster-version-operator_47_lightspeed-crd-analysisresults.yaml`:
- Around line 4-9: The manifest for the CRD
"analysisresults.agentic.openshift.io" is missing a kubernetes.io/description
annotation and has an inconsistent controller-gen version; add a
kubernetes.io/description entry under metadata.annotations that briefly explains
the resource's purpose (e.g., what AnalysisResults represent) and update
controller-gen.kubebuilder.io/version from v0.19.0 to v0.20.1 to match other
CRDs unless the v0.19.0 value is intentional—if intentional, add a comment or
note in repo docs explaining the divergence; locate these changes in the
metadata.annotations block that contains name:
analysisresults.agentic.openshift.io and controller-gen.kubebuilder.io/version.
🪄 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: 8cfbe721-0d41-4553-9d42-db96aa73e67a
⛔ Files ignored due to path filters (85)
go.sumis excluded by!**/*.sum,!go.sumvendor/github.com/openshift/lightspeed-agentic-operator/api/LICENSEis excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/lightspeed-agentic-operator/api/v1alpha1/agent_types.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/lightspeed-agentic-operator/api/v1alpha1/analysisresult_types.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/lightspeed-agentic-operator/api/v1alpha1/approvalpolicy_types.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/lightspeed-agentic-operator/api/v1alpha1/escalationresult_types.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/lightspeed-agentic-operator/api/v1alpha1/executionresult_types.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/lightspeed-agentic-operator/api/v1alpha1/groupversion_info.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/lightspeed-agentic-operator/api/v1alpha1/llmprovider_types.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/lightspeed-agentic-operator/api/v1alpha1/proposal_analysis_types.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/lightspeed-agentic-operator/api/v1alpha1/proposal_status_types.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/lightspeed-agentic-operator/api/v1alpha1/proposal_types.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/lightspeed-agentic-operator/api/v1alpha1/proposalapproval_types.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/lightspeed-agentic-operator/api/v1alpha1/reference_types.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/lightspeed-agentic-operator/api/v1alpha1/result_types.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/lightspeed-agentic-operator/api/v1alpha1/shared_types.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/lightspeed-agentic-operator/api/v1alpha1/tools_types.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/lightspeed-agentic-operator/api/v1alpha1/verificationresult_types.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/lightspeed-agentic-operator/api/v1alpha1/zz_generated.deepcopy.gois excluded by!vendor/**,!**/vendor/**,!**/zz_generated*vendor/golang.org/x/net/http2/transport.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/net/http2/writesched_priority_rfc9218.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/net/trace/events.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/net/websocket/hybi.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/sys/unix/mkerrors.shis excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/sys/unix/zerrors_linux.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/sys/unix/zerrors_linux_386.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/sys/unix/zerrors_linux_amd64.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/sys/unix/zerrors_linux_arm.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/sys/unix/zerrors_linux_arm64.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/sys/unix/zerrors_linux_loong64.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/sys/unix/zerrors_linux_mips.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/sys/unix/zerrors_linux_mips64.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/sys/unix/zerrors_linux_mips64le.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/sys/unix/zerrors_linux_mipsle.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/sys/unix/zerrors_linux_ppc.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/sys/unix/zerrors_linux_ppc64.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/sys/unix/zerrors_linux_ppc64le.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/sys/unix/zerrors_linux_riscv64.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/sys/unix/zerrors_linux_s390x.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/sys/unix/zerrors_linux_sparc64.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/sys/unix/ztypes_netbsd_arm.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/term/terminal.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/text/encoding/japanese/eucjp.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/text/encoding/japanese/iso2022jp.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/text/encoding/japanese/shiftjis.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/text/encoding/korean/euckr.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/text/encoding/simplifiedchinese/gbk.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/text/encoding/simplifiedchinese/hzgb2312.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/text/encoding/traditionalchinese/big5.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/text/encoding/unicode/unicode.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/tools/go/ast/inspector/cursor.gois excluded by!vendor/**,!**/vendor/**vendor/k8s.io/client-go/kubernetes/fake/clientset_generated.gois excluded by!vendor/**,!**/vendor/**vendor/k8s.io/klog/v2/README.mdis excluded by!vendor/**,!**/vendor/**vendor/k8s.io/klog/v2/internal/serialize/keyvalues.gois excluded by!vendor/**,!**/vendor/**vendor/k8s.io/klog/v2/internal/serialize/keyvalues_no_slog.gois excluded by!vendor/**,!**/vendor/**vendor/k8s.io/klog/v2/internal/serialize/keyvalues_slog.gois excluded by!vendor/**,!**/vendor/**vendor/k8s.io/klog/v2/klog.gois excluded by!vendor/**,!**/vendor/**vendor/k8s.io/klog/v2/klogr.gois excluded by!vendor/**,!**/vendor/**vendor/k8s.io/klog/v2/klogr_slog.gois excluded by!vendor/**,!**/vendor/**vendor/k8s.io/kube-openapi/pkg/validation/spec/ref.gois excluded by!vendor/**,!**/vendor/**vendor/k8s.io/utils/buffer/ring_fixed.gois excluded by!vendor/**,!**/vendor/**vendor/modules.txtis excluded by!vendor/**,!**/vendor/**vendor/sigs.k8s.io/controller-runtime/pkg/client/apiutil/apimachinery.gois excluded by!vendor/**,!**/vendor/**vendor/sigs.k8s.io/controller-runtime/pkg/client/apiutil/errors.gois excluded by!vendor/**,!**/vendor/**vendor/sigs.k8s.io/controller-runtime/pkg/client/client.gois excluded by!vendor/**,!**/vendor/**vendor/sigs.k8s.io/controller-runtime/pkg/client/client_rest_resources.gois excluded by!vendor/**,!**/vendor/**vendor/sigs.k8s.io/controller-runtime/pkg/client/config/config.gois excluded by!vendor/**,!**/vendor/**vendor/sigs.k8s.io/controller-runtime/pkg/client/dryrun.gois excluded by!vendor/**,!**/vendor/**vendor/sigs.k8s.io/controller-runtime/pkg/client/fake/client.gois excluded by!vendor/**,!**/vendor/**vendor/sigs.k8s.io/controller-runtime/pkg/client/fake/versioned_tracker.gois excluded by!vendor/**,!**/vendor/**vendor/sigs.k8s.io/controller-runtime/pkg/client/fieldowner.gois excluded by!vendor/**,!**/vendor/**vendor/sigs.k8s.io/controller-runtime/pkg/client/fieldvalidation.gois excluded by!vendor/**,!**/vendor/**vendor/sigs.k8s.io/controller-runtime/pkg/client/interceptor/intercept.gois excluded by!vendor/**,!**/vendor/**vendor/sigs.k8s.io/controller-runtime/pkg/client/interfaces.gois excluded by!vendor/**,!**/vendor/**vendor/sigs.k8s.io/controller-runtime/pkg/client/namespaced_client.gois excluded by!vendor/**,!**/vendor/**vendor/sigs.k8s.io/controller-runtime/pkg/client/options.gois excluded by!vendor/**,!**/vendor/**vendor/sigs.k8s.io/controller-runtime/pkg/client/patch.gois excluded by!vendor/**,!**/vendor/**vendor/sigs.k8s.io/controller-runtime/pkg/client/typed_client.gois excluded by!vendor/**,!**/vendor/**vendor/sigs.k8s.io/controller-runtime/pkg/client/unstructured_client.gois excluded by!vendor/**,!**/vendor/**vendor/sigs.k8s.io/controller-runtime/pkg/log/deleg.gois excluded by!vendor/**,!**/vendor/**vendor/sigs.k8s.io/controller-runtime/pkg/log/log.gois excluded by!vendor/**,!**/vendor/**vendor/sigs.k8s.io/controller-runtime/pkg/log/null.gois excluded by!vendor/**,!**/vendor/**vendor/sigs.k8s.io/structured-merge-diff/v6/schema/elements.gois excluded by!vendor/**,!**/vendor/**vendor/sigs.k8s.io/structured-merge-diff/v6/typed/remove.gois excluded by!vendor/**,!**/vendor/**vendor/sigs.k8s.io/structured-merge-diff/v6/value/reflectcache.gois excluded by!vendor/**,!**/vendor/**
📒 Files selected for processing (16)
go.modinstall/0000_00_cluster-version-operator_45_lightspeed-crd-proposals.yamlinstall/0000_00_cluster-version-operator_46_lightspeed-crd-agents.yamlinstall/0000_00_cluster-version-operator_47_lightspeed-crd-analysisresults.yamlinstall/0000_00_cluster-version-operator_47_lightspeed-crd-llmproviders.yamlinstall/0000_00_cluster-version-operator_47_lightspeed-crd-workflows.yamlpkg/proposal/api/v1alpha1/agent_types.gopkg/proposal/api/v1alpha1/llmprovider_types.gopkg/proposal/api/v1alpha1/olsconfig_types.gopkg/proposal/api/v1alpha1/outputfield_deepcopy.gopkg/proposal/api/v1alpha1/proposal_types.gopkg/proposal/api/v1alpha1/workflow_types.gopkg/proposal/controller.gopkg/proposal/controller_test.gopkg/start/start.gotest/cvo/proposal.go
💤 Files with no reviewable changes (7)
- pkg/proposal/api/v1alpha1/outputfield_deepcopy.go
- pkg/proposal/api/v1alpha1/workflow_types.go
- pkg/proposal/api/v1alpha1/agent_types.go
- install/0000_00_cluster-version-operator_47_lightspeed-crd-workflows.yaml
- pkg/proposal/api/v1alpha1/olsconfig_types.go
- pkg/proposal/api/v1alpha1/llmprovider_types.go
- pkg/proposal/api/v1alpha1/proposal_types.go
Comment on lines
+4
to
+9
| metadata: | ||
| annotations: | ||
| include.release.openshift.io/self-managed-high-availability: "true" | ||
| release.openshift.io/feature-set: TechPreviewNoUpgrade | ||
| controller-gen.kubebuilder.io/version: v0.20.1 | ||
| name: llmproviders.agentic.openshift.io |
There was a problem hiding this comment.
File numbering collision: both this file and analysisresults use _47_.
This file and 0000_00_cluster-version-operator_47_lightspeed-crd-analysisresults.yaml share the same file number (47). Consider renumbering one of them (e.g., use _48_ for this file) to ensure deterministic ordering.
Additionally, the manifest is missing a kubernetes.io/description annotation to explain the CRD's purpose.
Suggested annotations
metadata:
annotations:
include.release.openshift.io/self-managed-high-availability: "true"
release.openshift.io/feature-set: TechPreviewNoUpgrade
+ kubernetes.io/description: |-
+ LLMProvider defines an LLM provider configuration referenced by Agent resources.
controller-gen.kubebuilder.io/version: v0.20.1
name: llmproviders.agentic.openshift.ioAs per coding guidelines: "Use kubernetes.io/description annotations to explain the resource's purpose" and "File numbering should leave gaps for future additions."
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| metadata: | |
| annotations: | |
| include.release.openshift.io/self-managed-high-availability: "true" | |
| release.openshift.io/feature-set: TechPreviewNoUpgrade | |
| controller-gen.kubebuilder.io/version: v0.20.1 | |
| name: llmproviders.agentic.openshift.io | |
| metadata: | |
| annotations: | |
| include.release.openshift.io/self-managed-high-availability: "true" | |
| release.openshift.io/feature-set: TechPreviewNoUpgrade | |
| kubernetes.io/description: |- | |
| LLMProvider defines an LLM provider configuration referenced by Agent resources. | |
| controller-gen.kubebuilder.io/version: v0.20.1 | |
| name: llmproviders.agentic.openshift.io |
🤖 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 `@install/0000_00_cluster-version-operator_47_lightspeed-crd-llmproviders.yaml`
around lines 4 - 9, This file conflicts with another CRD using the same numeric
prefix; rename the file number (e.g., change the prefix from `_47_` to `_48_`)
so it no longer collides with
0000_00_cluster-version-operator_47_lightspeed-crd-analysisresults.yaml, and add
a kubernetes.io/description annotation under metadata.annotations for the CRD
name llmproviders.agentic.openshift.io that succinctly explains the CRD's
purpose (one-line description), ensuring deterministic ordering and compliance
with the coding guidelines.
Member
Author
|
Take from this job: $ curl -s https://gcsweb-ci.apps.ci.l2s4.p1.openshiftapps.com/gcs/test-platform-results/pr-logs/pull/openshift_cluster-version-operator/1387/pull-ci-openshift-cluster-version-operator-main-e2e-agnostic-ovn-techpreview-serial-2of3/2052748353468370944/artifacts/e2e-agnostic-ovn-techpreview-serial/openshift-e2e-test/artifacts/e2e.log | grep proposals
started: 0/1/28 "[Jira:\"Cluster Version Operator\"] cluster-version-operator should create proposals"
passed: (700ms) 2026-05-08T15:41:23 "[Jira:\"Cluster Version Operator\"] cluster-version-operator should create proposals" |
Contributor
|
@hongkailiu: The following tests 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. |
DavidHurta
reviewed
May 11, 2026
| @@ -1,12 +1,8 @@ | |||
| # Temporary CRD stub — will be removed once the lightspeed-agentic-operator owns this CRD. | |||
| # Generated from github.com/harche/lightspeed-agentic-operator/api/v1alpha1 | |||
| --- | |||
| apiVersion: apiextensions.k8s.io/v1 | |||
| kind: CustomResourceDefinition | |||
Contributor
There was a problem hiding this comment.
Usually, API and promotion changes are being gated by the API folks with their extensive expertise. These manifests will get applied in clusters. Are the APIs being updated in a non-conflicting manner so as not to break clusters updating from various versions? Currently, this is for the main branch, but soon EC and RC will start getting built. TechPreview clusters will be getting updated. And so on. Are the changes stable non-conflicting, or are the CRDs mainly used for rapid development testing with larger changes? If conflicting, can this approach potentially break future updates?
Member
Author
There was a problem hiding this comment.
are the CRDs mainly used for rapid development testing with larger changes?
Yes.
can this approach potentially break future updates?
I believe some changes on CRD itself wont break anything, such as removing a property in a CRD's spec.
The bad thing happens when some CR is stored in ETCD while its CRD is not backward-compatible.
So if 5.0.1 is built and some job upgrades (by --force) a cluster with it to 5.0.2. Yes, the upgrade might be broken if ec.1 is with a totally different CRD.
But upgrade is not supported when TechPriview is enabled. As long as the feature is guarded by TP, we (upgrade) should be good. Do you notice any blocking job doing it?
Let me know if the above relieves your concerns. Better safe than sorry.
Contributor
There was a problem hiding this comment.
But upgrade is not supported when TechPriview is enabled. As long as the feature is guarded by TP, we (upgrade) should be good.
Patch version updates (not minor-level) are possible and encouraged.
Docs:
Warning
Enabling the TechPreviewNoUpgrade feature set on your cluster cannot be undone and prevents minor version updates. You should not enable this feature set on production clusters.
Do you notice any blocking job doing it?
I am not sure if we are testing upgrades for techpreview clusters as extensively as we are testing default clusters; however, customer are using TechPreviewNoUpgrade clusters, and their experience is important. I am not clear on the policy for the EC builds, though.
After a code freeze all backports will be closely reviewed.
I am wondering about updates from EC TechPreviewNoUpgrade clusters. For example, the build02 cluster is such a cluster.
Contributor
There was a problem hiding this comment.
After a code freeze all backports will be closely reviewed.
And by then these CRDs will be removed from the CVO repository.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
lightspeed-agentic-operatoris installed via OLM (not in OCP core) and CVO needs those CRDs to test/demo the Feature. The sync like this will happen once in a while.Summary by CodeRabbit
Breaking Changes
New Features
Removals
Chores