moved last applied spec from annotation to status

[AndrewChubatiuk]

fixes #1802
moved last applied spec from annotation to status
besides fixing given issue it should also decrease amount of requests during each reconcile since before two patch requests happened in case of spec diff (one to update annotation, another - to update status), now only one call is needed

---

Summary by cubic

Moved “last applied spec” from annotations to status.lastAppliedSpec across all CRDs to fix operator issue #1802 and reduce reconcile API calls. Controllers now detect spec changes via status, not annotation patches.

• Refactors

  • Added schemaless status.lastAppliedSpec to all CRDs and CRD schemas; detect changes via UpdateLastSpec (equality.DeepEqual).
  • Removed annotation-based helpers, JSON hooks, and LastAppliedSpecAnnotation; factories read Status.LastAppliedSpec to build prevCR; updated tests.
  • Dropped patchAnnotationPredicate and WithEventFilter from controllers.
  • Minor Makefile tweak to adjust config-reloader flags docs for dynamic defaults.

• Performance

  • Replaced two patches (annotation + status) with a single status update when the spec changes.

^{Written for commit 73fb53c. Summary will update on new commits.}

[cubic-dev-ai]

11 issues found across 53 files

Prompt for AI agents (all issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="internal/controller/operator/controllers.go">

<violation number="1" location="internal/controller/operator/controllers.go:361">
P0: Critical logic inversion: All `HasSpecChanges()` implementations return `equality.Semantic.DeepEqual(...)`, which is `true` when specs are **equal** (no changes). This inverts the boolean — `specChanged` will be `true` when nothing changed and `false` when the spec actually differs. The implementations should negate the result: `return !equality.Semantic.DeepEqual(&cr.Spec, cr.Status.LastAppliedSpec)`.</violation>
</file>

<file name="api/operator/v1beta1/vmalertmanager_types.go">

<violation number="1" location="api/operator/v1beta1/vmalertmanager_types.go:496">
P1: Inverted boolean logic: `HasSpecChanges()` returns `DeepEqual(...)` which is `true` when specs are **equal** (no changes), but the function name and callers expect `true` when there **are** changes. Should be negated.

Note: This pattern exists in other types too, but is currently masked by `UnmarshalJSON` always overwriting `LastAppliedSpec` with current spec. If that behavior is ever corrected to preserve the stored last-applied-spec, this inversion will cause real issues.</violation>
</file>

<file name="api/operator/v1beta1/vmauth_types.go">

<violation number="1" location="api/operator/v1beta1/vmauth_types.go:735">
P0: Inverted boolean logic: `equality.Semantic.DeepEqual` returns `true` when specs are **equal** (no changes), but the method name `HasSpecChanges` implies it should return `true` when there **are** changes. This will cause callers expecting `true` = "has changes" to skip reconciliation when changes exist and reconcile when nothing changed.</violation>
</file>

<file name="api/operator/v1beta1/vlogs_types.go">

<violation number="1" location="api/operator/v1beta1/vlogs_types.go:312">
P1: HasSpecChanges now returns true when the spec matches the last applied spec, which inverts the intended "changes" signal and will skip updates when the spec actually changed. Return the negated equality check instead.</violation>
</file>

<file name="api/operator/v1beta1/vmagent_types.go">

<violation number="1" location="api/operator/v1beta1/vmagent_types.go:244">
P2: UnmarshalJSON overwrites status.LastAppliedSpec with the current spec, discarding the stored last-applied state from the API server. This prevents detecting spec changes after the first reconcile.</violation>

<violation number="2" location="api/operator/v1beta1/vmagent_types.go:645">
P1: HasSpecChanges is inverted: it returns true when specs match and false when they differ. This will skip updates when the spec changes and trigger updates when nothing changed.</violation>
</file>

<file name="api/operator/v1/vlagent_types.go">

<violation number="1" location="api/operator/v1/vlagent_types.go:190">
P1: UnmarshalJSON overwrites Status.LastAppliedSpec on every decode, which discards the persisted last-applied spec from status and prevents detecting spec changes. Only initialize it when it’s missing or leave it untouched.</violation>

<violation number="2" location="api/operator/v1/vlagent_types.go:493">
P2: HasSpecChanges currently returns true when specs are equal, so it won’t report changes. Invert the comparison (and treat nil last-applied as changed) so the method reflects actual spec differences.</violation>
</file>

<file name="api/operator/v1/vlcluster_types.go">

<violation number="1" location="api/operator/v1/vlcluster_types.go:765">
P0: Bug: `HasSpecChanges()` returns inverted result. `equality.Semantic.DeepEqual` returns `true` when objects are **equal** (no changes), but the method name and all callers expect `true` when there **are** changes. This will cause the controller to always trigger update logic when nothing changed and skip updates when the spec actually changes.</violation>
</file>

<file name="api/operator/v1/vlsingle_types.go">

<violation number="1" location="api/operator/v1/vlsingle_types.go:192">
P1: Overwriting `Status.LastAppliedSpec` on every unmarshal erases the stored last-applied spec from the API, so reconcile can no longer detect changes between `spec` and the persisted status.</violation>
</file>

<file name="api/operator/v1beta1/vmalert_types.go">

<violation number="1" location="api/operator/v1beta1/vmalert_types.go:513">
P1: Bug: `HasSpecChanges()` returns inverted result. `equality.Semantic.DeepEqual` returns `true` when objects are **equal** (no changes), but this method should return `true` when there **are** changes. The result needs to be negated.</violation>
</file>

_{Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.}

[AndrewChubatiuk]

@cubic-dev-ai review this PR

[cubic-dev-ai]

@cubic-dev-ai review this PR

@AndrewChubatiuk I have started the AI code review. It will take a few minutes to complete.

[cubic-dev-ai]

1 issue found across 53 files

Prompt for AI agents (all issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="api/operator/v1beta1/vmsingle_types.go">

<violation number="1" location="api/operator/v1beta1/vmsingle_types.go:93">
P1: HasAnyStreamAggrRule now checks spec equality instead of whether stream aggregation rules exist, so callers will get true whenever the spec changed even if no aggregation rules are configured.</violation>
</file>

_{Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.}

[AndrewChubatiuk]

@cubic-dev-ai review this PR

[cubic-dev-ai]

@cubic-dev-ai review this PR

@AndrewChubatiuk I have started the AI code review. It will take a few minutes to complete.

[cubic-dev-ai]

1 issue found across 64 files

Prompt for AI agents (all issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="internal/controller/operator/factory/vmdistributed/zone.go">

<violation number="1" location="internal/controller/operator/factory/vmdistributed/zone.go:105">
P1: Bug: `prevZ` reads from `cr` instead of `prevCR`, and additionally `buildVMAgentSpec` is a closure that captures `cr` — it uses `cr.Spec.ZoneCommon.VMAgent.Spec` and `cr.Spec.Zones` for merging and remote write construction. As a result, `prevAgentSpec` will always equal the current agent spec, making VMAgent change detection in `specsChanged()` completely non-functional.

Fix requires both changing `prevZ` to `&prevCR.Spec.Zones[i]` **and** parameterizing `buildVMAgentSpec` to accept the source CR (or creating a separate builder for the previous spec).</violation>
</file>

_{Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.}

[AndrewChubatiuk]

@cubic-dev-ai review this PR

[cubic-dev-ai]

@cubic-dev-ai review this PR

@AndrewChubatiuk I have started the AI code review. It will take a few minutes to complete.

[cubic-dev-ai]

3 issues found across 64 files

Prompt for AI agents (all issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="internal/controller/operator/factory/vmdistributed/zone.go">

<violation number="1" location="internal/controller/operator/factory/vmdistributed/zone.go:187">
P2: Previous-zone lookup uses the current CR (`cr.Spec.Zones[i]`) instead of `prevCR.Spec.Zones[i]`, so the computed "prev" VMAgent spec isn’t actually from the previous CR. This can make specsChanged comparisons incorrect and skip/trigger upgrade steps unexpectedly. Use prevCR when selecting the previous zone.</violation>
</file>

<file name="internal/controller/operator/factory/reconcile/statefulset.go">

<violation number="1" location="internal/controller/operator/factory/reconcile/statefulset.go:346">
P2: The local `pod` variable shadows the outer pointer, so `podStatusesToError` can never run and failures lose pod status diagnostics. Reuse the outer pointer instead of declaring a new local variable.</violation>
</file>

<file name="docs/api.md">

<violation number="1" location="docs/api.md:246">
P3: The new `VLAgentStatus` (and related `*Status`) links point to anchors that don’t exist in docs/api.md, which will render broken internal links. Add the missing status sections/headings or remove these status links to keep the docs navigation accurate.

(Based on your team's feedback about keeping docs metadata and links accurate.) [FEEDBACK_USED]</violation>
</file>

_{Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.}

[AndrewChubatiuk]

@cubic-dev-ai review this PR

[cubic-dev-ai]

@cubic-dev-ai review this PR

@AndrewChubatiuk I have started the AI code review. It will take a few minutes to complete.

[cubic-dev-ai]

No issues found across 66 files