HYPERFLEET-1436 - feat: add desire-transport client (desireclient) #284
There are no files selected for viewing
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
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,173 @@ | ||
| package desireclient | ||
|
|
||
| import ( | ||
| "context" | ||
| "encoding/json" | ||
| "errors" | ||
| "fmt" | ||
|
|
||
| "github.com/openshift-hyperfleet/hyperfleet-adapter/internal/manifest" | ||
| "github.com/openshift-hyperfleet/hyperfleet-adapter/internal/transportclient" | ||
| "github.com/openshift-hyperfleet/hyperfleet-applier/pkg/desire" | ||
| ) | ||
|
|
||
| // ApplyResource implements transportclient.TransportClient. It upserts an | ||
| // apply desire for the rendered manifest and auto-creates its paired read | ||
| // desire so the applied resource becomes visible to discovery. A read-desire | ||
| // pairing failure is returned as an error: an apply desire without its read | ||
| // desire is permanently invisible to discovery. | ||
| func (c *Client) ApplyResource( | ||
| ctx context.Context, | ||
| manifestBytes []byte, | ||
| opts *transportclient.ApplyOptions, | ||
| target transportclient.TransportContext, | ||
| ) (*transportclient.ApplyResult, error) { | ||
| if len(manifestBytes) == 0 { | ||
| return nil, fmt.Errorf("desireclient: manifest bytes cannot be empty") | ||
| } | ||
|
|
||
| tc, err := resolveTransportContext(target) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
|
|
||
| obj, err := parseToUnstructured(manifestBytes) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("desireclient: failed to parse manifest: %w", err) | ||
| } | ||
|
|
||
| // The store's ApplySpec.KubeContent must be valid JSON, but manifestBytes | ||
| // may have been YAML (parseToUnstructured accepts both). Re-marshal the | ||
| // parsed object rather than storing the original bytes verbatim. | ||
| kubeContent, err := json.Marshal(obj.Object) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("desireclient: failed to marshal manifest to JSON: %w", err) | ||
| } | ||
|
|
||
| gvk := obj.GroupVersionKind() | ||
| namespace, name := obj.GetNamespace(), obj.GetName() | ||
|
|
||
| readID, err := buildIdentity(tc, desire.TypeRead, gvk, namespace, name) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
|
|
||
| applyID, err := buildIdentity(tc, desire.TypeApply, gvk, namespace, name) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
|
|
||
| existing, err := c.store.GetApplyDesire(ctx, applyID) | ||
| if err != nil && !errors.Is(err, desire.ErrNotFound) { | ||
| return nil, fmt.Errorf("desireclient: failed to get apply desire for %s/%s: %w", applyID.Namespace, applyID.Name, err) | ||
| } | ||
| exists := err == nil | ||
|
|
||
| newGen := manifest.GetGenerationFromUnstructured(obj) | ||
| var existingGen int64 | ||
| if exists { | ||
| existingGen = generationFromKubeContent(existing.Spec.KubeContent) | ||
| } | ||
|
|
||
| decision := manifest.CompareGenerations(newGen, existingGen, exists) | ||
| result := &transportclient.ApplyResult{Operation: decision.Operation, Reason: decision.Reason} | ||
|
Ruclo marked this conversation as resolved.
|
||
|
|
||
| // 2. Pairing self-heals on every path, including skip: an externally | ||
|
Contributor
There was a problem hiding this comment. Nit: the |
||
| // deleted read desire is re-created on the next event. But the mirror | ||
| // only recreate to a new TargetVersion when this call is actually | ||
| // writing new apply content — on skip, ApplyDesire.Spec.KubeContent | ||
| // stays at the old API version, so the mirror must too. They move | ||
| // together or not at all. | ||
| recreate := decision.Operation != manifest.OperationSkip | ||
| if err = c.ensureReadDesire(ctx, readID, gvk.Version, recreate); err != nil { | ||
|
There was a problem hiding this comment. 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift 🔎 Supported by static analysis🏁 Script executed: #!/bin/bash
set -eu
printf '%s\n' '--- internal/desireclient/apply.go ---'
sed -n '1,145p' internal/desireclient/apply.go
printf '%s\n' '--- bound helpers and desire-store calls ---'
rg -n -A35 -B8 'ensureReadDesire|UpdateApplyDesireSpec|CreateApplyDesire|ErrVersionConflict|ReadDesire|ApplyDesire' internal/desireclientRepository: openshift-hyperfleet/hyperfleet-adapter Length of output: 50395 🏁 Script executed: #!/bin/bash
set -eu
sed -n '1,145p' internal/desireclient/apply.go
rg -n -A35 -B8 'ensureReadDesire|UpdateApplyDesireSpec|CreateApplyDesire|ErrVersionConflict|ReadDesire|ApplyDesire' internal/desireclientRepository: openshift-hyperfleet/hyperfleet-adapter Length of output: 50395 🏁 Script executed: #!/bin/bash
set -eu
printf '%s\n' '--- module binding ---'
rg -n 'hyperfleet-applier|replace ' go.mod go.sum
printf '%s\n' '--- ApplyResource tests and store abstractions ---'
rg -n -A45 -B10 'ApplyResource|VersionConflict|TargetVersion|UpdateApplyDesireSpec' internal/desireclient/*_test.go internal/desireclient/*.go
printf '%s\n' '--- repository references to the store contract ---'
rg -n 'UpdateApplyDesireSpec|DeleteReadDesire|CreateReadDesire' --glob '*.go' --glob '!internal/desireclient/apply.go' --glob '!internal/desireclient/*_test.go' .Repository: openshift-hyperfleet/hyperfleet-adapter Length of output: 50396 🏁 Script executed: #!/bin/bash
set -eu
rg -n 'hyperfleet-applier|replace ' go.mod go.sum
rg -n -A45 -B10 'ApplyResource|VersionConflict|TargetVersion|UpdateApplyDesireSpec' internal/desireclient/*_test.go internal/desireclient/*.go
rg -n 'UpdateApplyDesireSpec|DeleteReadDesire|CreateReadDesire' --glob '*.go' --glob '!internal/desireclient/apply.go' --glob '!internal/desireclient/*_test.go' .Repository: openshift-hyperfleet/hyperfleet-adapter Length of output: 50396 🏁 Script executed (no clone): Length of output: 1873 🏁 Script executed (no clone): Length of output: 263 🏁 Script executed (no clone): Length of output: 15338 Make
🤖 Prompt for AI Agents
Contributor
Author
There was a problem hiding this comment. this race between two apply desires is not possible because of the single writer model. another writer would see ErrOwnerConflict There was a problem hiding this comment. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '%s\n' '--- Client ownership and apply-update flow ---'
ast-grep outline internal/desireclient --items all --type function,method --match 'NewClient|ApplyResource|ensureReadDesire'
sed -n '1,190p' internal/desireclient/apply.go
sed -n '1,120p' internal/desireclient/client.go
printf '%s\n' '--- Pinned applier dependency ---'
rg -n -C2 'hyperfleet-applier|replace ' go.mod go.sum
printf '%s\n' '--- Owner-conflict references in repository and module cache, if present ---'
rg -n -C4 'ErrOwnerConflict|OwnerConflict|owner' internal/desireclient --glob '*.go' || true
modcache="$(go env GOMODCACHE 2>/dev/null || true)"
if [ -n "$modcache" ] && [ -d "$modcache" ]; then
rg -n -C5 'ErrOwnerConflict|UpdateApplyDesireSpec|type SpecStore' \
"$modcache/github.com/openshift-hyperfleet" --glob '*.go' 2>/dev/null || true
fiLength of output: 18636 🏁 Script executed: #!/bin/bash
set -euo pipefail
repo='openshift-hyperfleet/hyperfleet-applier'
rev='94b7a4d56697'
api="https://api.github.com/repos/${repo}/git/trees/${rev}?recursive=1"
paths="$(
curl -fsSL "$api" |
jq -r '.tree[]
| select(.type == "blob")
| select(.path | test("(^|/)(store|memory|desire).+\\.go$|pkg/desire/.+\\.go$"))
| .path'
)"
printf '%s\n' '--- Candidate desire-store source files ---'
printf '%s\n' "$paths"
printf '%s\n' '--- Owner and version checks in apply-update methods ---'
while IFS= read -r path; do
[ -n "$path" ] || continue
content="$(curl -fsSL "https://raw.githubusercontent.com/${repo}/${rev}/${path}")"
if printf '%s\n' "$content" | grep -qE 'UpdateApplyDesireSpec|ErrOwnerConflict'; then
printf '\n--- %s ---\n' "$path"
printf '%s\n' "$content" |
nl -ba |
grep -E -A40 -B12 'UpdateApplyDesireSpec|ErrOwnerConflict|ErrVersionConflict' || true
fi
done <<< "$paths"Length of output: 2194 🏁 Script executed: #!/bin/bash
set -euo pipefail
repo='openshift-hyperfleet/hyperfleet-applier'
rev='94b7a4d56697'
for path in pkg/desire/store.go pkg/desire/store/memory/store.go pkg/desire/store/redis/redis.go; do
printf '\n--- %s ---\n' "$path"
curl -fsSL "https://raw.githubusercontent.com/${repo}/${rev}/${path}" |
grep -n -E -A45 -B12 'UpdateApplyDesireSpec|ErrOwnerConflict|ErrVersionConflict' || true
doneLength of output: 28136
Both pinned store implementations check The records can then reference different target versions without a second apply writer. This finding remains valid.
You are interacting with an AI system. |
||
| return nil, fmt.Errorf( | ||
| "desireclient: failed to create paired read desire for %s/%s: %w", gvk.Kind, name, err) | ||
| } | ||
|
|
||
| switch decision.Operation { | ||
| case manifest.OperationCreate: | ||
| if _, err = c.store.CreateApplyDesire(ctx, desire.ApplyDesire{ | ||
| Identity: applyID, | ||
| Owner: c.owner, | ||
| Spec: desire.ApplySpec{KubeContent: kubeContent}, | ||
| }); err != nil { | ||
| c.logApplyError(ctx, applyID, err) | ||
| return nil, fmt.Errorf("desireclient: failed to create apply desire for %s/%s: %w", namespace, name, err) | ||
| } | ||
| case manifest.OperationUpdate: | ||
| if _, err = c.store.UpdateApplyDesireSpec( | ||
| ctx, applyID, desire.ApplySpec{KubeContent: kubeContent}, c.owner, existing.Version, | ||
| ); err != nil { | ||
| c.logApplyError(ctx, applyID, err) | ||
| return nil, fmt.Errorf( | ||
| "desireclient: failed to update apply desire for %s/%s: %w", applyID.Namespace, applyID.Name, err) | ||
| } | ||
| case manifest.OperationSkip: | ||
| // Nothing to do. | ||
| default: | ||
| return nil, fmt.Errorf("desireclient: unexpected apply decision operation %q", decision.Operation) | ||
| } | ||
|
|
||
| c.log.Debugf(ctx, "ApplyResource %s/%s: operation=%s reason=%s", | ||
| applyID.Namespace, applyID.Name, result.Operation, result.Reason) | ||
| return result, nil | ||
| } | ||
|
|
||
| func (c *Client) logApplyError(ctx context.Context, id desire.Identity, err error) { | ||
| switch { | ||
| case errors.Is(err, desire.ErrOwnerConflict): | ||
| c.log.Errorf(ctx, "ApplyResource %s/%s: operation failed with ErrOwnerConflict", id.Namespace, id.Name) | ||
|
Contributor
There was a problem hiding this comment. Nit, following on from last round. The levels are right now, |
||
| case errors.Is(err, desire.ErrDeletePending): | ||
| c.log.Warnf(ctx, "ApplyResource %s/%s: operation failed with ErrDeletePending", id.Namespace, id.Name) | ||
| case errors.Is(err, desire.ErrVersionConflict): | ||
| c.log.Warnf(ctx, "ApplyResource %s/%s: operation failed with ErrVersionConflict", id.Namespace, id.Name) | ||
| default: | ||
| c.log.Errorf(ctx, "ApplyResource %s/%s: operation failed with unexpected error: %v", id.Namespace, id.Name, err) | ||
| } | ||
| } | ||
|
|
||
| // ensureReadDesire creates the paired read desire if it doesn't already | ||
| // exist. ErrAlreadyExists is a no-op success. | ||
| func (c *Client) ensureReadDesire(ctx context.Context, id desire.Identity, targetVersion string, recreate bool) error { | ||
| _, err := c.store.CreateReadDesire(ctx, desire.ReadDesire{ | ||
| Identity: id, | ||
| Owner: c.owner, | ||
| TargetVersion: targetVersion, | ||
| }) | ||
| if err == nil { | ||
| return nil | ||
| } | ||
|
|
||
| if !errors.Is(err, desire.ErrAlreadyExists) { | ||
| return fmt.Errorf("desireclient: failed to create read desire for %s/%s: %w", id.Namespace, id.Name, err) | ||
| } | ||
|
|
||
| if !recreate { | ||
| return nil | ||
| } | ||
|
|
||
| des, err := c.store.GetReadDesire(ctx, id) | ||
| if err != nil { | ||
| return fmt.Errorf("desireclient: failed to get existing read desire for %s/%s: %w", id.Namespace, id.Name, err) | ||
| } | ||
|
|
||
| if des.TargetVersion == targetVersion { | ||
| return nil | ||
| } | ||
|
|
||
| err = c.store.DeleteReadDesire(ctx, id, c.owner, des.Version) | ||
| if err != nil { | ||
| return fmt.Errorf("desireclient: failed to delete existing read desire for %s/%s: %w", id.Namespace, id.Name, err) | ||
| } | ||
|
|
||
| if _, err := c.store.CreateReadDesire(ctx, desire.ReadDesire{ | ||
| Identity: id, | ||
| Owner: c.owner, | ||
| TargetVersion: targetVersion, | ||
| }); err != nil { | ||
| return fmt.Errorf("desireclient: failed to create read desire for %s/%s: %w", id.Namespace, id.Name, err) | ||
| } | ||
|
coderabbitai[bot] marked this conversation as resolved.
|
||
|
|
||
| return nil | ||
|
|
||
| } | ||
Oops, something went wrong.
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.
Uh oh!
There was an error while loading. Please reload this page.