Conversation
There was a problem hiding this comment.
8 issues found across 97 files
Note: This PR contains a large number of files. cubic only reviews up to 75 files per PR, so some files may not have been reviewed.
Prompt for AI agents (unresolved 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="docs/CHANGELOG.md">
<violation number="1" location="docs/CHANGELOG.md:16">
P1: Custom agent: **Changelog Review Agent**
This changelog entry violates the required prefix format: it uses `Dependency` instead of the mandatory `FEATURE`/`BUGFIX`/`SECURITY` prefix (Required structure §1).</violation>
<violation number="2" location="docs/CHANGELOG.md:27">
P1: Custom agent: **Changelog Review Agent**
This modified SECURITY entry is missing the mandatory affected-services section (Required structure §2), so it does not meet changelog format requirements.</violation>
<violation number="3" location="docs/CHANGELOG.md:27">
P3: Fix the trailing punctuation typo: this line has an extra closing parenthesis at the end.</violation>
</file>
<file name="docs/resources/vmagent.md">
<violation number="1" location="docs/resources/vmagent.md:59">
P2: This line references `VMSingle` in VMAgent documentation, which is incorrect and misleading for scrape configuration behavior.</violation>
<violation number="2" location="docs/resources/vmagent.md:73">
P2: The `selectAllByDefault` field is attributed to `VMSingle` instead of `VMAgent`, which misdocuments selector behavior.</violation>
<violation number="3" location="docs/resources/vmagent.md:79">
P2: This selector rule uses `VMSingle` namespaces in VMAgent docs, causing incorrect guidance on namespace scoping.</violation>
</file>
<file name="internal/controller/operator/factory/vmdistributed/vmdistributed_reconcile_test.go">
<violation number="1" location="internal/controller/operator/factory/vmdistributed/vmdistributed_reconcile_test.go:93">
P3: The test case input object is duplicated almost verbatim across scenarios; extract a shared base fixture and override only `VMAuth` per case to reduce maintenance drift.</violation>
</file>
<file name="internal/controller/operator/factory/vmauth/vmauth.go">
<violation number="1" location="internal/controller/operator/factory/vmauth/vmauth.go:661">
P2: Orphan cleanup incorrectly keeps VMServiceScrape when proxy protocol is enabled, leaving stale scrape resources that should be removed.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
docs/CHANGELOG.md
Outdated
| **Release date:** 23 February 2026 | ||
|
|
||
| SECURITY: upgrade Go builder from Go1.25.5 to Go1.25.7. See [the list of issues addressed in Go1.25.7](https://github.com/golang/go/issues?q=milestone%3AGo1.25.7+label%3ACherryPickApproved).) | ||
| * SECURITY: upgrade Go builder from Go1.25.5 to Go1.25.7. See [the list of issues addressed in Go1.25.7](https://github.com/golang/go/issues?q=milestone%3AGo1.25.7+label%3ACherryPickApproved).) |
There was a problem hiding this comment.
P1: Custom agent: Changelog Review Agent
This modified SECURITY entry is missing the mandatory affected-services section (Required structure §2), so it does not meet changelog format requirements.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At docs/CHANGELOG.md, line 27:
<comment>This modified SECURITY entry is missing the mandatory affected-services section (Required structure §2), so it does not meet changelog format requirements.</comment>
<file context>
@@ -13,10 +13,18 @@ aliases:
**Release date:** 23 February 2026
-SECURITY: upgrade Go builder from Go1.25.5 to Go1.25.7. See [the list of issues addressed in Go1.25.7](https://github.com/golang/go/issues?q=milestone%3AGo1.25.7+label%3ACherryPickApproved).)
+* SECURITY: upgrade Go builder from Go1.25.5 to Go1.25.7. See [the list of issues addressed in Go1.25.7](https://github.com/golang/go/issues?q=milestone%3AGo1.25.7+label%3ACherryPickApproved).)
* BUGFIX: [vmanomaly](https://docs.victoriametrics.com/operator/resources/vmanomaly/): fix configuration marshalling for [Prophet model](https://docs.victoriametrics.com/anomaly-detection/components/models/#prophet). Previously, using Prophet model would lead to panic during configuration marshalling.
</file context>
| * SECURITY: upgrade Go builder from Go1.25.5 to Go1.25.7. See [the list of issues addressed in Go1.25.7](https://github.com/golang/go/issues?q=milestone%3AGo1.25.7+label%3ACherryPickApproved).) | |
| * SECURITY: [vmoperator]: upgrade Go builder from Go1.25.5 to Go1.25.7. See [the list of issues addressed in Go1.25.7](https://github.com/golang/go/issues?q=milestone%3AGo1.25.7+label%3ACherryPickApproved). |
|
|
||
| These objects tell VMAgent from which targets and how to collect metrics and | ||
| generate part of [VMAgent](https://docs.victoriametrics.com/victoriametrics/vmagent/) scrape configuration. | ||
| These objects specify which targets VMSingle should scrape and how to collect metrics, and generate part of [VMAgent](https://docs.victoriametrics.com/victoriametrics/vmagent/) scrape configuration. |
There was a problem hiding this comment.
P2: This line references VMSingle in VMAgent documentation, which is incorrect and misleading for scrape configuration behavior.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At docs/resources/vmagent.md, line 59:
<comment>This line references `VMSingle` in VMAgent documentation, which is incorrect and misleading for scrape configuration behavior.</comment>
<file context>
@@ -56,44 +56,42 @@ Also, you can check out the [examples](https://docs.victoriametrics.com/operator
-These objects tell VMAgent from which targets and how to collect metrics and
-generate part of [VMAgent](https://docs.victoriametrics.com/victoriametrics/vmagent/) scrape configuration.
+These objects specify which targets VMSingle should scrape and how to collect metrics, and generate part of [VMAgent](https://docs.victoriametrics.com/victoriametrics/vmagent/) scrape configuration.
-For filtering scrape objects `VMAgent` uses selectors.
</file context>
| These objects specify which targets VMSingle should scrape and how to collect metrics, and generate part of [VMAgent](https://docs.victoriametrics.com/victoriametrics/vmagent/) scrape configuration. | |
| These objects specify which targets VMAgent should scrape and how to collect metrics, and generate part of [VMAgent](https://docs.victoriametrics.com/victoriametrics/vmagent/) scrape configuration. |
internal/controller/operator/factory/vmdistributed/vmdistributed_reconcile_test.go
Show resolved
Hide resolved
|
@AndrewChubatiuk any other fixes we want to add here? PTAL |
|
wanted additionally include #1902 |
|
wait, this PR is related to release 0.69 |
There was a problem hiding this comment.
2 issues found across 5 files (changes from recent commits).
Prompt for AI agents (unresolved 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=".github/workflows/crds.yaml">
<violation number="1" location=".github/workflows/crds.yaml:49">
P0: Dead execution path: The bzip2 command references a non-existent file path. The file was copied as `crd.yaml` in the previous step, not `crd.descriptionless.yaml`. This script will fail when executed because the file doesn't exist at this path.</violation>
</file>
<file name="docs/CHANGELOG.md">
<violation number="1" location="docs/CHANGELOG.md:14">
P1: Custom agent: **Changelog Review Agent**
This change violates the placement clause of the changelog rule: new entries must be kept in the `tip` section, but the `tip` header was replaced with a released-version header.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
805e857 to
4224b51
Compare
…ruct{} (#1848)
util/sets package provides a better interface to handle sets
Ensure that reconciliation doesn't change object resources when no changes to the spec applied
Ensure that controllers correctly stop handling paused objects
Testify asserts produce a significantly better output
…ble set to 100% (#1815) Signed-off-by: Vadim Rutkovsky <vadim@vrutkovs.eu> Co-authored-by: Vadim Rutkovsky <vadim@vrutkovs.eu>
Co-authored-by: cubic-dev-ai[bot] <191113872+cubic-dev-ai[bot]@users.noreply.github.com>
* feat: allow creating vmdistributed without vmauth set Users should be able to provide their own LB instead of VMAuth (taking care of proper switching themselves), so VMAuth should be optional for VMDistributed * updated docs --------- Co-authored-by: Andrii Chubatiuk <andrew.chubatiuk@gmail.com>
4224b51 to
0c393c3
Compare
* tests: update clientWithActions to record changes for all objects * tests: update tests to use new getTestClient * tests: move ClientWithActions to k8stools * tests: add data update configmap case * test: add daemonset reconcile tests * test: add HPA check case for deployment * test: add tests for HPA actions hpa * test: add tests for httproute reconciliation actions httproute fix httproute * test: add ests for ingress reconciliation action ingress * test: add tests for pdb reconciliation actions pdb * test: add tests for pvc reconciliation acitons * test: add tests for rbac reconciliation * test: add tests for secret reconciliation actions * test: add patch / delete actions * test: add tests for service reconciliation actions service * test: add tests for statefulset reconciliation actions * test: add tests for vmagent reconciliation actions * test: add tests for vmcluster reconciliation actions * test: add tests for vmauth reconciliation vmauth * test: status changes should not cause reconciliation * tests: add vmservicescrape reconcile tests vmservicescrape fix vmservicescrape * comment out tests * test: reduce timeouts and intervals for faster tests * test: add Kind to ClientAction * test: add reconcile tests checking actions and kinds when vmagent is created or updated * fix: sort asserts before creating to make CreateOrUpdate deterministic * test: add reconcile tests for vmalert is created or updated * test: add reconcile tests for vmalertmanager * test: add reconcile tests for vmanomaly * test: add reconcile tests for vmauth * test: add reconcile tests for vmcluster vmcluster * test: vmdistributed reconcile tests * test: vmsingle reconcile tests * test: vtcluster reconcile tests * test: add vtsingle reconcile tests * test: add reconciliation test case for deployment deploy * VMServiceScrape -> VMServiceScrapeForCRD * fix: make updateSTSPVC deterministi * undo vmservicescrape reconcile function renaming --------- Co-authored-by: Andrii Chubatiuk <andrew.chubatiuk@gmail.com>
There was a problem hiding this comment.
2 issues found across 49 files (changes from recent commits).
Prompt for AI agents (unresolved 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/reconcile/reconcile.go">
<violation number="1" location="internal/controller/operator/factory/reconcile/reconcile.go:33">
P3: Update the GoDoc comment to start with `InitDeadlines` so the exported API documentation matches the symbol.
(Based on your team's feedback about documenting exported structs and public methods.) [FEEDBACK_USED]</violation>
</file>
<file name="internal/controller/operator/factory/reconcile/reconcile_test.go">
<violation number="1" location="internal/controller/operator/factory/reconcile/reconcile_test.go:18">
P2: This `init` changes reconcile deadlines for the entire `reconcile` test package and never restores the defaults, which can inadvertently alter the behavior of unrelated tests. Consider scoping the shortened deadlines to `TestWaitForStatus` (with `t.Cleanup` to restore) or using a `TestMain` that resets the globals after running tests.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
internal/controller/operator/factory/reconcile/reconcile_test.go
Outdated
Show resolved
Hide resolved
…do not have any cleanup tasks (#1902) * reconcile: delete finalizer during reconcile only for resources that do not have any cleanup tasks * reconcile updates - convert NamespacedName to string in logs - update diffDeepDerivative, now slices and maps and considered equal if length of both is 0 (not only of the first maps as it was before), before labels and annotations diff was ignored - do not take PVC metadata diff into account during checking if STS should be recreated since metadata update is covered by a separate function - fixed diffDeep and diffDeepDerivative signature, arguments were swapped before
8938d70 to
401b5fe
Compare
There was a problem hiding this comment.
1 issue found across 22 files (changes from recent commits).
Prompt for AI agents (unresolved 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/manager/manager.go">
<violation number="1" location="internal/manager/manager.go:235">
P1: The new `reconcile.Init(...)` call wires `statusUpdateTTL` into `statusExpireTTL`, so the configured `controller.statusLastUpdateTimeTTL` no longer updates the actual `LastUpdateTime` TTL logic.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
0fe5bf9 to
935cbd1
Compare
|
lets bring #1925 in and 🚢 🇮🇹 |
* do not recreate STS if VCT size was increased decreased and recreate in other cases * do not throw error on STS recreation
There was a problem hiding this comment.
3 issues found across 6 files (changes from recent commits).
Prompt for AI agents (unresolved 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="docs/CHANGELOG.md">
<violation number="1" location="docs/CHANGELOG.md:25">
P1: Custom agent: **Changelog Review Agent**
These new changelog entries violate the changelog rule: they are added in a released section instead of `tip` (Placement rules), and they omit mandatory issue/PR links (Required structure §4 References).</violation>
<violation number="2" location="docs/CHANGELOG.md:25">
P3: Clarify this changelog item to explicitly describe when StatefulSet recreation is skipped vs required, and avoid ambiguous shorthand so users can understand upgrade behavior.
(Based on your team's feedback about clarifying behavior and roles in documentation text.) [FEEDBACK_USED]</violation>
</file>
<file name="internal/controller/operator/factory/reconcile/statefulset_pvc_expand.go">
<violation number="1" location="internal/controller/operator/factory/reconcile/statefulset_pvc_expand.go:134">
P1: Potential nil pointer dereference: `Storage()` returns `*resource.Quantity` which can be nil if storage isn't set. The old code checked `existingSize == nil || newSize == nil`, which was safe. The new code calls `.IsZero()` on potentially nil pointers, which would panic.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| @@ -11,16 +11,27 @@ aliases: | |||
| - /operator/changelog/index.html | |||
There was a problem hiding this comment.
P1: Custom agent: Changelog Review Agent
These new changelog entries violate the changelog rule: they are added in a released section instead of tip (Placement rules), and they omit mandatory issue/PR links (Required structure §4 References).
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At docs/CHANGELOG.md, line 25:
<comment>These new changelog entries violate the changelog rule: they are added in a released section instead of `tip` (Placement rules), and they omit mandatory issue/PR links (Required structure §4 References).</comment>
<file context>
@@ -22,6 +22,8 @@ aliases:
* BUGFIX: [vmdistributed](https://docs.victoriametrics.com/operator/resources/vmdistributed/): fix PVC being owned by StatefulSet and top-level object simultaenously. See [#1845](https://github.com/VictoriaMetrics/operator/issues/1845).
* BUGFIX: [vmoperator](https://docs.victoriametrics.com/operator/): remove unneeded finalizer from core K8s resources. See [#835](https://github.com/VictoriaMetrics/operator/issues/835).
* BUGFIX: [vmdistributed](https://docs.victoriametrics.com/operator/resources/vmdistributed/): remove finalizers from VMServiceScrape and VMPodScrape objects, and keep finalizers on VMAgent, VMCluster, and VMAuth when DeletionTimestamp is not empty.
+* BUGFIX: [vmoperator](https://docs.victoriametrics.com/operator/): do not recreate STS if VCT size was increased and recreate in other cases.
+* BUGFIX: [vmoperator](https://docs.victoriametrics.com/operator/): wait for STS deletion in case of recreation without throwing an error.
</file context>
internal/controller/operator/factory/reconcile/statefulset_pvc_expand.go
Outdated
Show resolved
Hide resolved
| * BUGFIX: [vmdistributed](https://docs.victoriametrics.com/operator/resources/vmdistributed/): fix PVC being owned by StatefulSet and top-level object simultaenously. See [#1845](https://github.com/VictoriaMetrics/operator/issues/1845). | ||
| * BUGFIX: [vmoperator](https://docs.victoriametrics.com/operator/): remove unneeded finalizer from core K8s resources. See [#835](https://github.com/VictoriaMetrics/operator/issues/835). | ||
| * BUGFIX: [vmdistributed](https://docs.victoriametrics.com/operator/resources/vmdistributed/): remove finalizers from VMServiceScrape and VMPodScrape objects, and keep finalizers on VMAgent, VMCluster, and VMAuth when DeletionTimestamp is not empty. | ||
| * BUGFIX: [vmoperator](https://docs.victoriametrics.com/operator/): do not recreate STS if VCT size was increased and recreate in other cases. |
There was a problem hiding this comment.
P3: Clarify this changelog item to explicitly describe when StatefulSet recreation is skipped vs required, and avoid ambiguous shorthand so users can understand upgrade behavior.
(Based on your team's feedback about clarifying behavior and roles in documentation text.)
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At docs/CHANGELOG.md, line 25:
<comment>Clarify this changelog item to explicitly describe when StatefulSet recreation is skipped vs required, and avoid ambiguous shorthand so users can understand upgrade behavior.
(Based on your team's feedback about clarifying behavior and roles in documentation text.) </comment>
<file context>
@@ -22,6 +22,8 @@ aliases:
* BUGFIX: [vmdistributed](https://docs.victoriametrics.com/operator/resources/vmdistributed/): fix PVC being owned by StatefulSet and top-level object simultaenously. See [#1845](https://github.com/VictoriaMetrics/operator/issues/1845).
* BUGFIX: [vmoperator](https://docs.victoriametrics.com/operator/): remove unneeded finalizer from core K8s resources. See [#835](https://github.com/VictoriaMetrics/operator/issues/835).
* BUGFIX: [vmdistributed](https://docs.victoriametrics.com/operator/resources/vmdistributed/): remove finalizers from VMServiceScrape and VMPodScrape objects, and keep finalizers on VMAgent, VMCluster, and VMAuth when DeletionTimestamp is not empty.
+* BUGFIX: [vmoperator](https://docs.victoriametrics.com/operator/): do not recreate STS if VCT size was increased and recreate in other cases.
+* BUGFIX: [vmoperator](https://docs.victoriametrics.com/operator/): wait for STS deletion in case of recreation without throwing an error.
</file context>
| * BUGFIX: [vmoperator](https://docs.victoriametrics.com/operator/): do not recreate STS if VCT size was increased and recreate in other cases. | |
| * BUGFIX: [vmoperator](https://docs.victoriametrics.com/operator/): skip StatefulSet recreation when `volumeClaimTemplates` size is increased; recreate StatefulSet for other immutable spec changes. |
… or relabelling is set in ingestOnly mode (#1926) Co-authored-by: cubic-dev-ai[bot] <191113872+cubic-dev-ai[bot]@users.noreply.github.com>
No description provided.