NO-JIRA: Add UBIQUITOUS_LANGUAGE.md#533
Conversation
|
Pipeline controller notification For optional jobs, comment This repository is configured in: LGTM mode |
|
@mdbooth: This pull request explicitly references no jira issue. 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-robot
added
the
jira/valid-reference
WalkthroughA new Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes 🚥 Pre-merge checks | ✅ 12✅ Passed checks (12 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
[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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@UBIQUITOUS_LANGUAGE.md`:
- Around line 63-64: The glossary entry "Paused" and "Paused Annotation"
currently describe pause as a CAPI-only reconciler behavior tied to the
cluster.x-k8s.io/paused annotation; update the other mentions that refer to the
"old authority" during migration to explicitly state that pause semantics apply
only to CAPI controllers/resources (not MAPI resources) or that the migration
process treats CAPI-managed resources as paused by CAPI controllers, clarifying
that MAPI resources are not paused by the same annotation; locate and edit the
instances referencing "old authority" and "migration" to use this explicit
phrasing and reference the cluster.x-k8s.io/paused annotation and the "Paused"
glossary term for consistency.
- Line 115: Replace the phrase "Authoritative copy" with the canonical glossary
term "Authoritative API" in the sentence quoted (the line containing the sync
controller description), so it reads consistently with the defined terms
(Authoritative API and Non-Authoritative) and avoids reintroducing ambiguity
about the copy vs API wording.
🪄 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: 8d2f3de9-265f-4ea5-9fae-ac8dcf3d4caf
📒 Files selected for processing (1)
UBIQUITOUS_LANGUAGE.md
| | **Paused** | A state where a CAPI resource's reconciler stops processing it, controlled by the `cluster.x-k8s.io/paused` annotation | Frozen, suspended, disabled | | ||
| | **Paused Annotation** | The `cluster.x-k8s.io/paused` annotation whose presence requests a CAPI controller to stop reconciling a resource | | |
There was a problem hiding this comment.
Pausing semantics are internally inconsistent across APIs.
Line 63 defines Paused as a CAPI-only behavior, but Line 100/Line 111 imply the “old authority” is always paused during migration. That reads as if MAPI resources can also be paused the same way, which conflicts with the glossary definition.
Suggested wording adjustment
-- During **Migration**, the old authority is **Paused** to prevent conflicting reconciliation
+- During **Migration**, if the old authority is a CAPI resource, it is **Paused** to prevent conflicting reconciliation; for MAPI authority, use explicit controller handoff semantics instead of CAPI pause terminology-- **Domain expert:** "Both controllers are always running. The migration controller sets the status to **Migrating**, then requests the old **Authoritative API**'s resource to be **Paused**. It waits for the **Paused Condition** to confirm the old controller has stopped reconciling that specific resource, so only one controller is operating on any given **Instance** at a time."
+- **Domain expert:** "Both controllers are always running. The migration controller sets the status to **Migrating**. When the old authority is CAPI, it requests that resource to be **Paused** and waits for the **Paused Condition**; otherwise it uses the API-specific handoff path. This ensures only one controller operates on any given **Instance** at a time."Also applies to: 100-100, 111-111
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@UBIQUITOUS_LANGUAGE.md` around lines 63 - 64, The glossary entry "Paused" and
"Paused Annotation" currently describe pause as a CAPI-only reconciler behavior
tied to the cluster.x-k8s.io/paused annotation; update the other mentions that
refer to the "old authority" during migration to explicitly state that pause
semantics apply only to CAPI controllers/resources (not MAPI resources) or that
the migration process treats CAPI-managed resources as paused by CAPI
controllers, clarifying that MAPI resources are not paused by the same
annotation; locate and edit the instances referencing "old authority" and
"migration" to use this explicit phrasing and reference the
cluster.x-k8s.io/paused annotation and the "Paused" glossary term for
consistency.
| > | ||
| > **Dev:** "And the Non-Authoritative copy -- is it just a mirror?" | ||
| > | ||
| > **Domain expert:** "Exactly. The sync controller performs **Conversion** from the **Authoritative** copy and updates the **Non-Authoritative** one. It tracks this via the **Synchronized Condition** and **Synchronized Generation** so we know when they've diverged." |
There was a problem hiding this comment.
Use canonical glossary term to avoid reintroducing ambiguity.
Line 115 says “Authoritative copy”, but the defined term is Authoritative API (and Non-Authoritative). Prefer the exact glossary term for consistency.
Suggested edit
-- > **Domain expert:** "Exactly. The sync controller performs **Conversion** from the **Authoritative** copy and updates the **Non-Authoritative** one. It tracks this via the **Synchronized Condition** and **Synchronized Generation** so we know when they've diverged."
+- > **Domain expert:** "Exactly. The sync controller performs **Conversion** from the copy managed by the **Authoritative API** and updates the **Non-Authoritative** one. It tracks this via the **Synchronized Condition** and **Synchronized Generation** so we know when they've diverged."📝 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.
| > **Domain expert:** "Exactly. The sync controller performs **Conversion** from the **Authoritative** copy and updates the **Non-Authoritative** one. It tracks this via the **Synchronized Condition** and **Synchronized Generation** so we know when they've diverged." | |
| > **Domain expert:** "Exactly. The sync controller performs **Conversion** from the copy managed by the **Authoritative API** and updates the **Non-Authoritative** one. It tracks this via the **Synchronized Condition** and **Synchronized Generation** so we know when they've diverged." |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@UBIQUITOUS_LANGUAGE.md` at line 115, Replace the phrase "Authoritative copy"
with the canonical glossary term "Authoritative API" in the sentence quoted (the
line containing the sync controller description), so it reads consistently with
the defined terms (Authoritative API and Non-Authoritative) and avoids
reintroducing ambiguity about the copy vs API wording.
|
@mdbooth: all tests passed! 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. |
|
Do we want to hook this up to AGENTS.md then? TY |
3 participants
Generated using https://github.com/mattpocock/skills/blob/main/ubiquitous-language/SKILL.md
Adds a project-specific glossary which:
Summary by CodeRabbit