Skip to content

Conversation

@jmelis
Copy link

@jmelis jmelis commented Jan 26, 2026

No description provided.

@coderabbitai
Copy link

coderabbitai bot commented Jan 26, 2026

Walkthrough

A new documentation file for Maestro client interactions is introduced, covering architecture overview, REST and gRPC client usage patterns, lifecycle management, update strategies, deletion propagation policies, and troubleshooting guidance.

Changes

Cohort / File(s) Summary
Documentation
docs/client-guide.md
New file documenting Maestro client interactions, including architecture overview, REST/gRPC client usage, Consumer lifecycle, resource management (ResourceBundles, ManifestWorks, Feedback Rules), update strategies (Update, CreateOnly, ServerSideApply, ReadOnly), deletion with propagation policies (Foreground, Orphan, SelectivelyOrphan), code examples, selective orphaning, TTL-based deletion, status checks, and troubleshooting guidance. (+293 lines)

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~5 minutes

🚥 Pre-merge checks | ✅ 1 | ❌ 2
❌ Failed checks (2 inconclusive)
Check name Status Explanation Resolution
Title check ❓ Inconclusive The title 'Client guide' is vague and does not clearly indicate what the guide covers or the specific changes being introduced. Use a more specific title that describes the purpose of the guide, such as 'Add Maestro client interaction guide' or 'Document client REST and gRPC usage patterns'.
Description check ❓ Inconclusive No pull request description was provided by the author, making it impossible to assess whether any description exists. Add a pull request description explaining the purpose of the client guide, key sections covered, and intended audience or use cases.
✅ Passed checks (1 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


Comment @coderabbitai help to get the list of available commands and usage tips.

@jmelis jmelis marked this pull request as draft January 26, 2026 14:31
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 Fix all issues with AI agents
In `@docs/client-guide.md`:
- Around line 201-218: The fenced code block showing the deletion flow is
missing a language identifier which triggers MD040; update the fenced block (the
triple-backtick block that begins before "1. User requests deletion") to include
a language token such as text (e.g., change ``` to ```text) so the markdown
linter stops flagging it and the block renders consistently.
- Around line 114-128: Add a language identifier to the fenced code block that
starts with "ManifestWork" in docs/client-guide.md to satisfy MD040; update the
opening ``` to include a language (e.g., "text") so the block becomes ```text
and keep the block contents unchanged.

Comment on lines +114 to +128
```
ManifestWork
├── metadata.generation ← MW spec version (you set this)
├── spec.workload.manifests[] ← The K8s resources
├── spec.manifestConfigs[] ← Feedback rules
└── status
├── conditions[]
│ ├── type: Applied/Available
│ └── observedGeneration ← MW version agent processed
└── resourceStatus.manifests[]
├── resourceMeta ← Which object
├── conditions[] ← Did agent apply it? (no observedGen here)
└── statusFeedback.jsonRaw ← Contains fields requested via feedbackRules
(e.g., observedGeneration, conditions, replicas)
```
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Add a language identifier to the fenced block.
MD040 flags this block; add a language (e.g., text) for consistency.

✅ Suggested fix
-```
+```text
 ManifestWork
 ├── metadata.generation              ← MW spec version (you set this)
 ├── spec.workload.manifests[]        ← The K8s resources
 ├── spec.manifestConfigs[]           ← Feedback rules
 └── status
     ├── conditions[]
     │   ├── type: Applied/Available
     │   └── observedGeneration       ← MW version agent processed
     └── resourceStatus.manifests[]
         ├── resourceMeta             ← Which object
         ├── conditions[]             ← Did agent apply it? (no observedGen here)
         └── statusFeedback.jsonRaw   ← Contains fields requested via feedbackRules
                                         (e.g., observedGeneration, conditions, replicas)
📝 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
```
ManifestWork
├── metadata.generation ← MW spec version (you set this)
├── spec.workload.manifests[] ← The K8s resources
├── spec.manifestConfigs[] ← Feedback rules
└── status
├── conditions[]
│ ├── type: Applied/Available
│ └── observedGeneration ← MW version agent processed
└── resourceStatus.manifests[]
├── resourceMeta ← Which object
├── conditions[] ← Did agent apply it? (no observedGen here)
└── statusFeedback.jsonRaw ← Contains fields requested via feedbackRules
(e.g., observedGeneration, conditions, replicas)
```
🧰 Tools
🪛 markdownlint-cli2 (0.18.1)

114-114: Fenced code blocks should have a language specified

(MD040, fenced-code-language)

🤖 Prompt for AI Agents
In `@docs/client-guide.md` around lines 114 - 128, Add a language identifier to
the fenced code block that starts with "ManifestWork" in docs/client-guide.md to
satisfy MD040; update the opening ``` to include a language (e.g., "text") so
the block becomes ```text and keep the block contents unchanged.

Comment on lines +201 to +218
```
1. User requests deletion (REST DELETE or gRPC Delete)
2. Maestro marks resource as "deleting" (soft delete, sets deleted_at timestamp)
3. Maestro sends delete CloudEvent to agent
4. Agent deletes resources from target cluster (per PropagationPolicy)
5. Agent sends confirmation CloudEvent back
6. Maestro hard deletes the record from database
```
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Add a language identifier to the fenced block.
MD040 flags this block; add a language (e.g., text) for consistency.

✅ Suggested fix
-```
+```text
 1. User requests deletion (REST DELETE or gRPC Delete)
           │
           ▼
 2. Maestro marks resource as "deleting" (soft delete, sets deleted_at timestamp)
           │
           ▼
 3. Maestro sends delete CloudEvent to agent
           │
           ▼
 4. Agent deletes resources from target cluster (per PropagationPolicy)
           │
           ▼
 5. Agent sends confirmation CloudEvent back
           │
           ▼
 6. Maestro hard deletes the record from database
🧰 Tools
🪛 markdownlint-cli2 (0.18.1)

201-201: Fenced code blocks should have a language specified

(MD040, fenced-code-language)

🤖 Prompt for AI Agents
In `@docs/client-guide.md` around lines 201 - 218, The fenced code block showing
the deletion flow is missing a language identifier which triggers MD040; update
the fenced block (the triple-backtick block that begins before "1. User requests
deletion") to include a language token such as text (e.g., change ``` to
```text) so the markdown linter stops flagging it and the block renders
consistently.

// Resource Bundles (read-only + delete)
client.DefaultAPI.ApiMaestroV1ResourceBundlesGet(ctx) // List
client.DefaultAPI.ApiMaestroV1ResourceBundlesIdGet(ctx, id) // Get (includes status)
client.DefaultAPI.ApiMaestroV1ResourceBundlesIdDelete(ctx, id)// Delete
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This API is deprecated; please use the gRPC client’s Delete method instead

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, we intend to keep the RESTful API read-only going forward—only for query operations. For create/update/delete actions, we plan to unify all mutation logic under gRPC. So for new users, we strongly recommend using the gRPC client’s Delete method instead, but, this REST API will remain functional until all existing consumers have migrated off it.

workClient.ManifestWorks(clusterName).Create(ctx, manifestWork, metav1.CreateOptions{})
workClient.ManifestWorks(clusterName).Update(ctx, manifestWork, metav1.UpdateOptions{})
workClient.ManifestWorks(clusterName).Delete(ctx, name, metav1.DeleteOptions{})
workClient.ManifestWorks(clusterName).Watch(ctx, metav1.ListOptions{}) // streams status updates
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the workClient also support get/list

workClient.ManifestWorks(clusterName).Get(ctx, name, metav1.GetOptions{})
workClient.ManifestWorks(clusterName).List(ctx, name, metav1.ListOptions{})


For a visual sequence diagram of the delete flow, see [Resource Delete Flow](./maestro.md#maestro-resource-flow).

> **Note:** Maestro's database is a tracking layer, not the source of truth for what's running on clusters. If the database is wiped directly (bypassing the API), resources on target clusters remain intact. Source clients can resync to restore Maestro's tracking state.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the database is wiped directly (bypassing the API), the agent client on the target cluster will delete all associated resources during its next resync, as they are no longer tracked by Maestro.

},
```

### TTL Auto-Deletion
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is not in the maestro I believe @skeeey

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, yes, the maestro does not support this for now

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants