Skip to content

Commit 0e31413

Browse files
author
David Magton
committed
[api] Reorganize v1alpha1 types and conditions
- Split API into *_types.go and *_conditions.go; add shared helpers - Rename condition constants to object-scoped Replicated*Cond* identifiers - Update controllers/tests to use the new condition/type names - Regenerate CRDs and move Cursor rules under .cursor/rules/* Signed-off-by: David Magton <david.magton@flant.com>
1 parent 1e8dfa1 commit 0e31413

File tree

80 files changed

+2012
-1659
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

80 files changed

+2012
-1659
lines changed

.cursor/api_conditions_rules.mdc

Lines changed: 0 additions & 44 deletions
This file was deleted.
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,9 @@ alwaysApply: true
1010
- If I add a new API object/type or modify an existing one in `api/` (especially changes to `// +kubebuilder:*` markers, validation markers, printcolumns, subresources, etc.), I MUST run code generation and include the regenerated outputs in the same change.
1111
- In this repo, run generation from the repository root:
1212
- `bash hack/generate_code.sh`
13+
- If I am intentionally doing an **API-only refactor stage** where changes outside `api/` are temporarily forbidden/undesired (e.g. the rest of the repo is not yet refactored and will not compile), then:
14+
- It is acceptable to **defer CRD regeneration** (outputs under `crds/`) until the stage when cross-repo refactor is allowed.
15+
- I MUST still keep `api/v1alpha1` internally consistent and compilable; prefer running **object/deepcopy generation only** when possible, instead of editing generated files by hand.
1316

1417
- Generated files (MUST NOT edit by hand):
1518
- Do NOT edit `zz_generated*` files (e.g. `api/v1alpha1/zz_generated.deepcopy.go`) manually.
Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,58 @@
1+
---
2+
description: API Conditions naming rules (v1alpha1)
3+
globs:
4+
- "api/**/*_conditions.go"
5+
- "!api/linstor/**/*.go"
6+
alwaysApply: true
7+
---
8+
9+
- Condition constants naming:
10+
- Every API object `Status` MUST expose `.status.conditions` (`[]metav1.Condition`) (see `types_rules.mdc`).
11+
- Any API object that has at least one standardized/used condition MUST have its own condition type/reason constants scoped by object name.
12+
- If the API type exposes `.status.conditions` but there are **no** standardized/used conditions yet:
13+
- The `Conditions` field MUST remain in the API (it is part of the contract).
14+
- The `<objprefix>_conditions.go` file MAY be absent.
15+
- Do NOT create placeholder/empty condition constants “just in case”.
16+
- Current API types that expose `.status.conditions` in this repo:
17+
- `ReplicatedVolume`
18+
- `ReplicatedVolumeReplica`
19+
- `ReplicatedVolumeAttachment`
20+
- `ReplicatedStorageClass`
21+
- `ReplicatedStoragePool`
22+
23+
- Condition Type constants MUST be named:
24+
- `<ObjName>Cond<CondTypeName>Type`
25+
- `CondTypeName` MUST match the string value of `.Type`.
26+
- Examples:
27+
- `ReplicatedVolumeCondIOReadyType = "IOReady"`
28+
- `ReplicatedVolumeReplicaCondDataInitializedType = "DataInitialized"`
29+
- `ReplicatedVolumeAttachmentCondReplicaIOReadyType = "ReplicaIOReady"`
30+
31+
- Condition Reason constants MUST be named:
32+
- `<ObjName>Cond<CondTypeName>Reason<ReasonName>`
33+
- `CondTypeName` MUST match the string value of the condition type (the `.Type` string).
34+
- `ReasonName` MUST match the string value of `.Reason`.
35+
- Examples:
36+
- `ReplicatedVolumeReplicaCondScheduledReasonReplicaScheduled = "ReplicaScheduled"`
37+
- `ReplicatedVolumeCondQuorumReasonQuorumLost = "QuorumLost"`
38+
- `ReplicatedVolumeAttachmentCondAttachedReasonSettingPrimary = "SettingPrimary"`
39+
40+
- Conditions grouping (MUST):
41+
- Keep each condition type and **all of its reasons in a single `const (...)` block**.
42+
- Conditions MUST be ordered alphabetically by condition type name within the file/package.
43+
- Reasons within a condition MUST be ordered alphabetically by reason constant name.
44+
45+
- Conditions comments (MUST):
46+
- Avoid controller-specific comments like “managed by X” in API packages.
47+
- Add short English docs: what the condition represents and what the reasons mean.
48+
49+
- Value stability (MUST):
50+
- Do NOT change string values of `.Type` and `.Reason` constants.
51+
- Only rename Go identifiers when reorganizing/clarifying.
52+
53+
- Scoping & duplication (MUST):
54+
- Do NOT use generic `ConditionType*` / `Reason*` constants.
55+
- If the same reason string is used by multiple conditions, create separate constants per condition type, even if the string is identical.
56+
- Example: `"NodeNotReady"`:
57+
- `ReplicatedVolumeReplicaCondOnlineReasonNodeNotReady = "NodeNotReady"`
58+
- `ReplicatedVolumeReplicaCondIOReadyReasonNodeNotReady = "NodeNotReady"`
Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
---
2+
description: API file structure and conventions (sds-replicated-volume)
3+
globs:
4+
- "api/**/*.go"
5+
- "!api/linstor/**/*.go"
6+
alwaysApply: true
7+
---
8+
9+
- Object prefixes (MUST):
10+
- Use short prefixes: `rv`, `rvr`, `rva`, `rsc`, `rsp`.
11+
12+
- File naming per object (MUST):
13+
- `<objprefix>_types.go`: API types (kubebuilder tags), object/spec/status structs, adapters for interfaces (e.g. GetConditions/SetConditions) and tightly coupled constants/types and pure set/get/has helpers (no I/O, no external context).
14+
- `<objprefix>_conditions.go`: condition Type/Reason constants for the object.
15+
- MAY be absent if the API object exposes `.status.conditions` but there are no standardized/used conditions yet (do not create empty placeholder constants).
16+
- `<objprefix>_custom_logic_that_should_not_be_here.go`: non-trivial/domain logic helpers (everything that does not fit `*_types.go`).
17+
18+
- Common file naming (MUST):
19+
- `common_types.go`: shared types/enums/constants for the API package.
20+
- `common_helpers.go`: shared pure helpers used across API types.
21+
- `labels.go`: well-known label keys (constants).
22+
- `finalizers.go`: module finalizer constants.
23+
- `register.go`: scheme registration.
Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
---
2+
description: API naming rules for label keys and finalizers (sds-replicated-volume)
3+
globs:
4+
- "api/**/labels.go"
5+
- "api/**/finalizers.go"
6+
- "!api/linstor/**/*.go"
7+
alwaysApply: true
8+
---
9+
10+
## Label keys (`labels.go`)
11+
12+
- **Constant naming (MUST)**:
13+
- Label key constants MUST end with `LabelKey`.
14+
- Good: `ReplicatedVolumeLabelKey`, `NodeNameLabelKey`
15+
- Bad: `LabelReplicatedVolume`, `NodeLabel`, `LabelNodeName`
16+
17+
- **Prefix constant (MUST)**:
18+
- The label prefix constant MUST be private and named `labelPrefix` (unless there is a proven need to export it).
19+
- The prefix value MUST be the module-scoped prefix:
20+
- `sds-replicated-volume.deckhouse.io/`
21+
22+
- **Value format (MUST)**:
23+
- Label key values MUST be built as `labelPrefix + "<suffix>"`.
24+
- The `<suffix>` part MUST be lowercase-kebab-case, without repeating the module name.
25+
- Good: `labelPrefix + "replicated-volume"`
26+
- Bad: `labelPrefix + "sds-replicated-volume-replicated-volume"`
27+
28+
- **Layout (MUST)**:
29+
- Keep all exported `...LabelKey` constants in a single `const (...)` block.
30+
- Avoid commented-out placeholder constants; prefer adding constants only when actually needed.
31+
32+
## Finalizers (`finalizers.go`)
33+
34+
- **Constant naming (MUST)**:
35+
- Finalizer constants MUST end with `Finalizer`.
36+
- Good: `ControllerFinalizer`, `AgentFinalizer`
37+
- Bad: `FinalizerController`, `ControllerFinalizerName`
38+
39+
- **Value format (MUST)**:
40+
- Finalizer values MUST be module-scoped and stable:
41+
- `sds-replicated-volume.deckhouse.io/<component>`
42+
- `<component>` MUST be lowercase and short (e.g. `controller`, `agent`).
43+
44+
- **Stability (MUST)**:
45+
- Do NOT change existing finalizer string values (this would break cleanup semantics).

.cursor/rules/api-types/RULE.md

Lines changed: 145 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,145 @@
1+
---
2+
description: API rules for type-centric layout, enums, status, naming, and helpers/custom logic
3+
globs:
4+
- "api/**/*_types.go"
5+
- "api/**/common_types.go"
6+
- "!api/linstor/**/*.go"
7+
- "!api/**/zz_generated*"
8+
alwaysApply: true
9+
---
10+
11+
## Code layout: type-centric blocks (MUST)
12+
13+
- **Type-centric blocks** MUST be used to organize code:
14+
- Each type MUST be readable without scrolling across the file (keep related declarations together).
15+
- Code from different types MUST NOT be interleaved.
16+
17+
- **API object file layout** (MUST):
18+
- This applies to typical files containing one API root object (`type <Obj> struct`) plus its `Spec`/`Status`/`List`.
19+
- The main flow MUST read top-to-bottom without jumping:
20+
- Root object: `type <Obj> struct { ... }`
21+
- `type <Obj>List struct { ... }` (see List rule below)
22+
- `type <Obj>Spec struct { ... }`
23+
- Spec-local types/enums/constants/interfaces/helpers used by `Spec`
24+
- `type <Obj>Status struct { ... }`
25+
- Status-local types/enums/constants/interfaces/helpers used by `Status`
26+
- Secondary/helper types referenced by the above (pseudo-DFS), keeping each type block contiguous
27+
- Shared helpers (if any) at the very end
28+
29+
- **Block structure** for each type MUST follow this strict order:
30+
- `type <TypeName> struct { ... }`
31+
- Enums and constants belonging to this type (incl. tightly-coupled sub-enums)
32+
- Interfaces tightly coupled to the type
33+
- Public methods of the type
34+
- Private helpers of the type
35+
36+
- **Block ordering in a file** MUST be a human-oriented dependency order (pseudo-DFS), not alphabetical:
37+
- Main (primary) type of the file
38+
- Types directly referenced by the main type
39+
- Secondary/helper types
40+
41+
- **List types** (MUST):
42+
- `<Obj>List` SHOULD be placed immediately after `<Obj>` (right under the root object), to make navigation consistent and fast.
43+
- `<Obj>List` MUST NOT split the `Spec`/`Status` flow (i.e. do not put it between `Spec` and spec-local enums/helpers, or between `Status` and status-local enums/helpers).
44+
- If there is a strong reason (rare), `<Obj>List` MAY be placed after `Status`/secondary types, but keep it as a single contiguous block (no interleaving).
45+
46+
- **Locality rule for enums/constants/helpers** (MUST):
47+
- If an enum/const/helper is primarily used by `Spec`, it MUST be placed in the Spec-local section (right after `type <Obj>Spec ...` and its methods).
48+
- If an enum/const/helper is primarily used by `Status`, it MUST be placed in the Status-local section (right after `type <Obj>Status ...` and its methods).
49+
- If an enum/const/helper is used by both `Spec` and `Status`, it SHOULD be placed with the `Spec` section (earlier) unless that hurts readability; do NOT duplicate it.
50+
51+
- **Shared helpers**:
52+
- Avoid generic helpers without a clear owning type.
53+
- If a helper is used by multiple types, it MUST be placed after all type blocks (or moved to `common_helpers.go` if shared broadly).
54+
55+
- Enums (MUST):
56+
- If a field has a finite set of constant values, model it as an enum:
57+
- `type EnumType string`
58+
- `const ( EnumTypeValue1 EnumType = "Value1" ... )`
59+
- Enum declaration order MUST be contiguous:
60+
- `type EnumType string`
61+
- `const (...)` with all enum values
62+
- enum helpers (if any) — right after the const block
63+
- Enums MUST provide `String()` method:
64+
- `func (e EnumType) String() string { return string(e) }`
65+
- Keep enum values documented (short English comment per value or a short block comment).
66+
- Do NOT create separate wrapper types for arbitrary string/number/bool fields unless there is a strong, confirmed need.
67+
- Common enums (MUST):
68+
- If the same enum is used by multiple API objects, it MUST be moved to `common_types.go`.
69+
- Do NOT move enums to `common_types.go` if they are only used by a single API object.
70+
71+
- Status (MUST):
72+
- `Spec` and `Status` structs MUST be embedded as values on the root object (e.g. `Spec TSpec`, `Status TStatus`), not `*TStatus`.
73+
- Every API object `Status` MUST expose `.conditions` as `[]metav1.Condition`:
74+
- Field name MUST be `Conditions []metav1.Condition`.
75+
- Use the standard kubebuilder/patch markers for mergeable conditions list:
76+
- `// +patchMergeKey=type`
77+
- `// +patchStrategy=merge`
78+
- `// +listType=map`
79+
- `// +listMapKey=type`
80+
- `// +optional`
81+
- JSON tag: ``json:"conditions,omitempty"`` and patch tags consistent with the above.
82+
- Condition Type/Reason constants are defined in `<objprefix>_conditions.go` only when they become standardized/used (see `conditions_rules.mdc`).
83+
84+
- Type naming (MUST):
85+
- This section applies to ALL API types (including enums).
86+
- Names MUST be unique within the API package.
87+
- Names MUST NOT start with short object prefixes like `RV`, `RVR`, `RVA`, `RSC`, `RSP`.
88+
- Usually, names MUST NOT start with the full object name if the type is not generic and is unlikely to clash:
89+
- Good: `ReplicaType`, `DiskState`
90+
- Prefer full object name only for generic/repeated concepts (below).
91+
- If the type name is generic and likely to be repeated across objects (e.g. `Phase`, `Type`), it MUST start with the full object name:
92+
- Examples: `ReplicatedStoragePoolPhase`, `ReplicatedStoragePoolType`, `ReplicatedVolumeAttachmentPhase`
93+
- Structural type name (e.g. `Spec`, `Status`) MUST be prefixed by the full object name:
94+
- Examples: `ReplicatedVolumeSpec`, `ReplicatedVolumeStatus`, `ReplicatedStorageClassSpec`, `ReplicatedStorageClassStatus`
95+
96+
## Helpers vs custom_logic_that_should_not_be_here (MUST)
97+
98+
Write helpers in `*_types.go`. If a function does **not** fit the rules below, it MUST go to `*_custom_logic_that_should_not_be_here.go`.
99+
100+
## What belongs in `*_types.go` (MUST)
101+
102+
Helpers are **pure**, **local**, **context-free** building blocks.
103+
104+
- **Pure / deterministic**:
105+
- Same input → same output.
106+
- No reads of current time, random, env vars, filesystem, network, Kubernetes API, shell commands.
107+
- No goroutines, channels, retries, backoff, sleeping, polling.
108+
109+
- **No external context**:
110+
- Do not require `context.Context`, `*runtime.Scheme`, `client.Client`, informers, listers, recorders, loggers.
111+
- Do not require controller-runtime utilities (e.g. `controllerutil.*`).
112+
113+
- **Allowed operations**:
114+
- Field reads/writes on in-memory structs and maps/slices.
115+
- Simple validation and parsing/formatting that is deterministic.
116+
- Nil-guards and trivial branching.
117+
- Returning `(value, ok)` / `(changed bool)` patterns.
118+
119+
- **Typical helper shapes (examples)**:
120+
- `HasX() bool`, `GetX() (T, bool)`, `SetX(v T) (changed bool)`, `ClearX() (changed bool)`
121+
- `IsXEqual(...) bool`, `XEquals(...) bool`
122+
- `ParseX(string) X` / `FormatX(...) string` (no I/O, no time, no external lookups)
123+
124+
## What MUST NOT be in `*_types.go` (MUST NOT)
125+
126+
If any of these are present, the code belongs in `*_custom_logic_that_should_not_be_here.go`.
127+
128+
- **Business / orchestration logic**:
129+
- Decisions that interpret cluster state, desired/actual reconciliation, phase machines, progress tracking.
130+
- Anything that “synchronizes” different parts of an object (spec ↔ status, spec ↔ labels/annotations, cross-object references).
131+
132+
- **Conditions/status mutation logic**:
133+
- Creating/updating `metav1.Condition` / using `meta.SetStatusCondition` / computing reason/message based on multi-step state.
134+
- Anything that sets `.status.phase`, `.status.reason`, counters, aggregates, etc. based on logic.
135+
136+
- **Controller/Kubernetes integration**:
137+
- `controllerutil.SetControllerReference`, finalizer management with external expectations, scheme usage.
138+
- Any reads/writes via API clients (even if “simple”).
139+
140+
- **I/O and side effects**:
141+
- File/network access, exec/shell, OS calls, time-based logic (`time.Now`, `time.Since`), randomness.
142+
143+
- **Non-trivial control flow**:
144+
- Complex `if/switch` trees, multi-branch logic tied to domain semantics.
145+
- Loops that encode placement/selection/scheduling decisions.
File renamed without changes.
File renamed without changes.
File renamed without changes.

0 commit comments

Comments
 (0)