ROSAENG-62089: doc: Add client-go vs custom SDK comparison doc - #186
ROSAENG-62089: doc: Add client-go vs custom SDK comparison doc#186cdoan1 wants to merge 1 commit into
Conversation
Explains why client-go works for rosa-hyperfleet-api consumers, compares approaches (controller-runtime, typed clientset, REST gateway), and recommends starting with the existing API types module. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: cdoan1 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 |
WalkthroughAdds documentation comparing Kubernetes client-go/controller-runtime usage with a custom SDK, describing current API support, typed-client generation options, REST-gateway constraints, integration paths, and recommended adoption steps. ChangesClient integration guidance
Estimated code review effort: 1 (Trivial) | ~5 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 10 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (10 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 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 `@docs/api/v2-clientgo-vs-custom-sdk.md`:
- Around line 265-268: Update the Path 2 recommendation in the API client
comparison so it refers to continuing to version and document the existing,
already-importable API types module instead of publishing it. Keep the guidance
about using the module for serialization with a thin HTTP client unchanged.
- Around line 280-284: Update the summary table’s client-go support statements
to distinguish controller-runtime/pkg/client, which works today for direct
Kubernetes API access with zero changes, from generated typed client-go clients,
which are not yet implemented and remain optional future work. Adjust the “What
should we do first?” guidance to preserve this distinction.
- Line 120: Update the client initialization in the example to use a
watch-capable client via client.NewWithWatch or client.WithWatch before calling
c.Watch. Keep the existing Watch invocation and ClusterList argument unchanged.
- Around line 35-49: Update the Go examples in this document to be
copy-pastable: add the runtime import and remove the unused controller-runtime
scheme import in the client setup example; define or import ctx and metav1,
replace the invalid NodePoolSpec placeholder with valid Go, and use the
appropriate watch API instead of client.Client.Watch; add the bytes import where
bytes.NewReader is used. If these snippets are intentionally incomplete,
explicitly label them as pseudocode instead.
- Around line 133-148: Update the API type declarations for Cluster, NodePool,
Placement, ManagementCluster, and Manifest to include +genclient markers, while
preserving the existing package-level generation tags and client generation
workflow.
🪄 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: openshift-online/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 77e95f97-5219-45b9-935f-3b18ce287b4f
📒 Files selected for processing (1)
docs/api/v2-clientgo-vs-custom-sdk.md
| c.Create(ctx, np) | ||
|
|
||
| // Watch for changes | ||
| c.Watch(ctx, &hyperfleetv1.ClusterList{}) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Relevant doc excerpt =="
sed -n '105,130p' docs/api/v2-clientgo-vs-custom-sdk.md | cat -n
echo
echo "== Controller-runtime version =="
if [ -f go.mod ]; then
rg -n 'sigs\.k8s\.io/controller-runtime' go.mod || true
fi
echo
echo "== Any mention of Watch-capable client in repo =="
rg -n 'NewWithWatch|WithWatch|Watch\(' docs go.mod .github -g '!**/vendor/**' || trueRepository: openshift-online/rosa-hyperfleet-api
Length of output: 1430
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== File size =="
wc -l docs/api/v2-clientgo-vs-custom-sdk.md
echo
echo "== Lines around the cited snippet =="
sed -n '115,125p' docs/api/v2-clientgo-vs-custom-sdk.md | cat -n
echo
echo "== Search for any local helper/interface adding Watch to client.Client =="
rg -n 'type .*interface.*Watch|interface .*Watch|func .*Watch\(' -g '*.go' . || trueRepository: openshift-online/rosa-hyperfleet-api
Length of output: 809
Use a watch-capable client here. client.New returns client.Client, which doesn’t expose Watch; switch this example to client.NewWithWatch or wrap it with client.WithWatch so the snippet is valid.
🤖 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 `@docs/api/v2-clientgo-vs-custom-sdk.md` at line 120, Update the client
initialization in the example to use a watch-capable client via
client.NewWithWatch or client.WithWatch before calling c.Watch. Keep the
existing Watch invocation and ClusterList argument unchanged.
| ```bash | ||
| # 1. Add code-generator to hack/tools | ||
| go get k8s.io/code-generator@v0.36.0 | ||
|
|
||
| # 2. Add generation tags to api/v1alpha1/doc.go | ||
| // +k8s:deepcopy-gen=package,register | ||
| // +k8s:defaulter-gen=TypeMeta | ||
| // +k8s:openapi-gen=true | ||
| // +groupName=hyperfleet.io | ||
|
|
||
| # 3. Create hack/update-codegen.sh (mirror hypershift's script) | ||
| # Generates: client/clientset/, client/informers/, client/listers/ | ||
|
|
||
| # 4. Add Makefile target | ||
| make clients | ||
| ``` |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate the referenced doc and inspect the relevant section with line numbers.
doc="docs/api/v2-clientgo-vs-custom-sdk.md"
echo "=== $doc (lines 120-170) ==="
sed -n '120,170p' "$doc" | cat -n
echo
echo "=== Search for Kubernetes dependency/version pins ==="
rg -n --hidden --glob '!**/.git/**' 'k8s.io/(client-go|apimachinery|code-generator)|code-generator@|client-go' -S . | sed -n '1,220p'
echo
echo "=== Search for genclient markers and related codegen notes ==="
rg -n --hidden --glob '!**/.git/**' '\+genclient|\+k8s:deepcopy-gen|\+k8s:defaulter-gen|\+k8s:openapi-gen|hack/update-codegen|make clients' -S . | sed -n '1,220p'Repository: openshift-online/rosa-hyperfleet-api
Length of output: 24495
🏁 Script executed:
#!/bin/bash
set -euo pipefail
doc="docs/api/v2-clientgo-vs-custom-sdk.md"
echo "=== $doc (lines 120-170) ==="
sed -n '120,170p' "$doc" | cat -n
echo
echo "=== Search for Kubernetes dependency/version pins ==="
rg -n --hidden --glob '!**/.git/**' 'k8s.io/(client-go|apimachinery|code-generator)|code-generator@|client-go' -S . | sed -n '1,220p'
echo
echo "=== Search for genclient markers and related codegen notes ==="
rg -n --hidden --glob '!**/.git/**' '\+genclient|\+k8s:deepcopy-gen|\+k8s:defaulter-gen|\+k8s:openapi-gen|hack/update-codegen|make clients' -S . | sed -n '1,220p'Repository: openshift-online/rosa-hyperfleet-api
Length of output: 24495
🏁 Script executed:
#!/bin/bash
set -euo pipefail
doc="docs/api/v2-clientgo-vs-custom-sdk.md"
echo "=== $doc (lines 120-170) ==="
sed -n '120,170p' "$doc" | cat -n
echo
echo "=== Candidate dependency manifest files ==="
git ls-files | rg '(^|/)(go\.mod|go\.sum|Makefile|go\.work|package\.json|buf\.yaml|helmfile\.yaml|\.mod)$|(^|/)(api|apis|pkg)/.*\.go$' | sed -n '1,240p'Repository: openshift-online/rosa-hyperfleet-api
Length of output: 4981
🌐 Web query:
Kubernetes code-generator version must match client-go/apimachinery version genclient markers required typed clients
💡 Result:
When working with Kubernetes code generation, maintaining version synchronization between components is critical for stability, though the specific mechanisms and requirements are well-defined: 1. Version Matching: The k8s.io/code-generator repository explicitly states that its HEAD matches the HEAD of k8s.io/apiserver, k8s.io/apimachinery, and k8s.io/client-go [1][2][3]. When consuming these libraries, it is standard practice to align the versions of k8s.io/code-generator, k8s.io/apimachinery, and k8s.io/client-go within your go.mod file [2][4]. Using mismatched versions often leads to compilation errors or subtle runtime issues due to changes in internal API structures or helper library signatures [5]. 2. Genclient Markers: To generate typed clients for your API types, you must use the // +genclient marker [6][7]. - This marker must be placed above the struct definition for the API type (e.g., in your types.go file) [6]. - It must not be placed above the List type (e.g., PodList) [7]. - The generator uses this marker as an opt-in mechanism; without it, the client-gen tool will ignore the type [7][8]. - Additional tags such as // +genclient:nonNamespaced, // +genclient:noStatus, and various verb-control tags (e.g., // +genclient:onlyVerbs=...) can be used to further configure the behavior of the generated client [6][7]. In summary, ensure that your k8s.io dependencies are pinned to compatible versions—ideally matching the Kubernetes release cycle you are targeting [5]—and ensure that every resource you wish to have a typed client for is explicitly annotated with the // +genclient tag [6][7].
Citations:
- 1: https://github.com/kubernetes/code-generator
- 2: https://pkg.go.dev/k8s.io/code-generator
- 3: https://github.com/kubernetes/code-generator/blob/master/README.md
- 4: https://pkg.go.dev/k8s.io/code-generator@v0.36.1
- 5: https://github.com/kubernetes/client-go/blob/master/README.md
- 6: https://github.com/kubernetes/community/blob/main/contributors/devel/sig-api-machinery/generating-clientset.md
- 7: https://www.redhat.com/en/blog/kubernetes-deep-dive-code-generation-customresources
- 8: https://github.com/kubernetes/code-generator/blob/master/cmd/client-gen/generators/client_generator.go
🌐 Web query:
site:kubernetes.io code-generator genclient marker typed client client-gen README version match client-go/apimachinery
💡 Result:
In the context of the Kubernetes code-generator toolset, the +genclient marker is a directive used to instruct client-gen to generate a typed client for a specific API group or resource [1]. Key points regarding the use of these markers and version matching include: 1. Marker Purpose: The +genclient marker is typically placed in the doc.go file or above the relevant type definition within your API package to signal that the code generator should produce a typed clientset for the associated custom resource [1]. This enables type-safe interactions with your Kubernetes custom resources rather than relying on less efficient or error-prone dynamic clients [1]. 2. Version Matching (client-go and code-generator): It is critical that the version of k8s.io/code-generator matches the version of k8s.io/client-go and the Kubernetes API machinery your project depends on [1]. Mismatches between these dependencies often lead to build failures, incompatible generated code, or runtime errors because the internal structures (such as API schemes and object metadata) may differ between versions [1][2]. 3. Implementation Best Practices: - When setting up your project, ensure that the version (or branch) of code-generator matches the target Kubernetes release [1]. - If using Go modules, the go.mod file should reflect consistent versions across all k8s.io/* repositories. Historically, projects pinned these to specific Kubernetes release tags (e.g., kubernetes-1.9.0) to ensure compatibility [1]. - Beyond genclient, modern Kubernetes development also utilizes various other markers (e.g., +k8s:listType, +k8s:immutable) for declarative validation and server-side apply, which are processed by specialized generators like validation-gen [3][4][5]. Always consult the go.mod file of the specific kubernetes/kubernetes release you are targeting to ensure that your local development dependencies align with the upstream source [6][2].
Citations:
- 1: https://kubernetes.io/blog/2018/01/introducing-client-go-version-6/
- 2: https://kubernetes.io/docs/contribute/generate-ref-docs/quickstart/
- 3: https://kubernetes.io/docs/reference/using-api/server-side-apply/
- 4: https://kubernetes.io/docs/reference/using-api/declarative-validation/
- 5: https://kubernetes.io/blog/2026/05/05/kubernetes-v1-36-declarative-validation-ga/
- 6: https://kubernetes.io/docs/contribute/generate-ref-docs/kubernetes-api/
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== api/v1alpha1 files ==="
git ls-files 'hyperfleet-operator/api/v1alpha1/*' | sed -n '1,200p'
echo
echo "=== api/v1alpha1/doc.go ==="
sed -n '1,220p' hyperfleet-operator/api/v1alpha1/doc.go | cat -n
echo
echo "=== api/v1alpha1/cluster_types.go ==="
sed -n '1,220p' hyperfleet-operator/api/v1alpha1/cluster_types.go | cat -n
echo
echo "=== api/v1alpha1/nodepool_types.go ==="
sed -n '1,220p' hyperfleet-operator/api/v1alpha1/nodepool_types.go | cat -n
echo
echo "=== api/v1alpha1/placement_types.go ==="
sed -n '1,220p' hyperfleet-operator/api/v1alpha1/placement_types.go | cat -n
echo
echo "=== api/v1alpha1/managementcluster_types.go ==="
sed -n '1,220p' hyperfleet-operator/api/v1alpha1/managementcluster_types.go | cat -n
echo
echo "=== api/v1alpha1/manifest_types.go ==="
sed -n '1,220p' hyperfleet-operator/api/v1alpha1/manifest_types.go | cat -nRepository: openshift-online/rosa-hyperfleet-api
Length of output: 689
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== types files with line numbers ==="
for f in \
hyperfleet-operator/api/v1alpha1/cluster_types.go \
hyperfleet-operator/api/v1alpha1/nodepool_types.go \
hyperfleet-operator/api/v1alpha1/placement_types.go \
hyperfleet-operator/api/v1alpha1/managementcluster_types.go \
hyperfleet-operator/api/v1alpha1/manifest_types.go \
hyperfleet-operator/api/v1alpha1/groupversion_info.go
do
echo
echo "--- $f ---"
sed -n '1,220p' "$f" | cat -n
done
echo
echo "=== search for genclient markers in repo ==="
rg -n --hidden --glob '!**/.git/**' '\+genclient|client-gen|customresourcedefinitions|SchemeBuilder|AddToScheme' hyperfleet-operator/api platform-api hyperfleet-db docs -S | sed -n '1,240p'Repository: openshift-online/rosa-hyperfleet-api
Length of output: 28947
Add +genclient on the concrete API types.
Package-level tags alone won’t generate a typed clientset; Cluster, NodePool, Placement, ManagementCluster, and Manifest need +genclient markers.
🤖 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 `@docs/api/v2-clientgo-vs-custom-sdk.md` around lines 133 - 148, Update the API
type declarations for Cluster, NodePool, Placement, ManagementCluster, and
Manifest to include +genclient markers, while preserving the existing
package-level generation tags and client generation workflow.
|
Let me summarize here all the points:
|
|
@cdoan1: This pull request references ROSAENG-62089 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. |
Explains why client-go works for rosa-hyperfleet-api consumers, compares approaches (controller-runtime, typed clientset, REST gateway), and recommends starting with the existing API types module.
Description
Type of Change
Testing
make test)Checklist
Summary by CodeRabbit