fix: remove deprecated -eula flag from vmbackupmanager args#2042
Conversation
AndrewChubatiuk
left a comment
There was a problem hiding this comment.
LGTM! Could you please add changelog entry as well?
Thanks, but this is an issue created to help new contributors get acquainted with the codebase and help us fix more challenging issues. As you can see, the fix is trivial and we could have fixed it long time ago. Are you planning to do further contributions here? |
haven't noticed this. didn't expect AI is required to fix this |
There was a problem hiding this comment.
1 issue found across 1 file (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:36">
P1: Custom agent: **Changelog Review Agent**
This changelog entry is missing the required user-centric before/now/impact explanation and currently reads as an internal implementation detail, which violates the mandatory structure in the Changelog Review Agent rule.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review, or fix all with cubic.
| * FEATURE: [vmagent](https://docs.victoriametrics.com/operator/resources/vmagent/): VMAgent CRs running in DaemonSet mode now support configuring rolling update strategy behavior. | ||
| * FEATURE: [vmoperator](https://docs.victoriametrics.com/operator/): Dry-run mode. See [#1832](https://github.com/VictoriaMetrics/operator/issues/1832). | ||
|
|
||
| * BUGFIX: [vmbackupmanager](https://docs.victoriametrics.com/operator/): remove deprecated `-eula` flag from vmbackupmanager and vmrestore container args. See [#1319](https://github.com/VictoriaMetrics/operator/issues/1319). |
There was a problem hiding this comment.
P1: Custom agent: Changelog Review Agent
This changelog entry is missing the required user-centric before/now/impact explanation and currently reads as an internal implementation detail, which violates the mandatory structure in the Changelog Review Agent rule.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At docs/CHANGELOG.md, line 36:
<comment>This changelog entry is missing the required user-centric before/now/impact explanation and currently reads as an internal implementation detail, which violates the mandatory structure in the Changelog Review Agent rule.</comment>
<file context>
@@ -33,6 +33,7 @@ aliases:
* FEATURE: [vmagent](https://docs.victoriametrics.com/operator/resources/vmagent/): VMAgent CRs running in DaemonSet mode now support configuring rolling update strategy behavior.
* FEATURE: [vmoperator](https://docs.victoriametrics.com/operator/): Dry-run mode. See [#1832](https://github.com/VictoriaMetrics/operator/issues/1832).
+* BUGFIX: [vmbackupmanager](https://docs.victoriametrics.com/operator/): remove deprecated `-eula` flag from vmbackupmanager and vmrestore container args. See [#1319](https://github.com/VictoriaMetrics/operator/issues/1319).
* BUGFIX: [vmoperator](https://docs.victoriametrics.com/operator/): VMPodScrape for VLAgent and VMAgent now uses the correct port; previously it used the wrong port and could cause scrape failures. See [#1887](https://github.com/VictoriaMetrics/operator/issues/1887).
* BUGFIX: [vmdistributed](https://docs.victoriametrics.com/operator/resources/vmdistributed/): updated VMAuth config consolidating all VMSelects into a single read and all VMClusters into a single write backend
</file context>
|
Fair point - I should have picked a more substantial issue. Added the changelog entry. I'm looking at #1677 (VMAgent DaemonSetMode volume bug) next. |
|
@mvanhorn could you please sign commits? |
The -eula flag has been deprecated in VictoriaMetrics and will be removed shortly. Stop passing it to vmbackupmanager and vmrestore containers to avoid incompatibility when the flag is removed upstream. The AcceptEULA field in the CRD is kept for now as it gates whether backup is configured at all. Fixes VictoriaMetrics#1319
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
37d1cd7 to
fb83ec8
Compare
|
@vrutkovs |
|
Right, I suppose a decriptive CHANGELOG is the only mechanism we have right now? |
|
upgrade tests that require license are failing, it's okay for fork repos |
|
Thanks for the merge, @AndrewChubatiuk! |
Summary
Stops passing the deprecated
-eulaflag to vmbackupmanager and vmrestore containers. The flag was deprecated last year and will be removed from VictoriaMetrics upstream.Changes
internal/controller/operator/factory/build/backup.go: Removed-eulafrom the args slice in bothVMBackupManager()andVMRestore()functions.The
AcceptEULAfield in the CRD spec is kept for now since it still gates whether backup configuration is enabled.Testing
go build ./internal/controller/operator/factory/build/...passesFixes #1319
This contribution was developed with AI assistance (Claude Code).
Summary by cubic
Stop passing the deprecated
-eulaflag tovmbackupmanagerandvmrestorecontainer args to prevent breakage when VictoriaMetrics removes it upstream. Added a changelog entry;AcceptEULAin the CRD still gates whether backup config is enabled.Written for commit fb83ec8. Summary will update on new commits.