Increase default sync interval for OpenStack datasource from 60s to 600s#660
Merged
SoWieMarkus merged 2 commits intomainfrom Mar 27, 2026
Merged
Increase default sync interval for OpenStack datasource from 60s to 600s#660SoWieMarkus merged 2 commits intomainfrom
SoWieMarkus merged 2 commits intomainfrom
Conversation
SoWieMarkus
requested review from
PhilippMatthes,
auhlig,
juliusclausnitzer,
mblos and
umswmayj
as code owners
umswmayj
approved these changes
Mar 27, 2026
PhilippMatthes
approved these changes
Mar 27, 2026
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughDefault OpenStack datasource sync interval changed from 60s to 600s across the Go API type, the CRD schema, and Helm datasource templates. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
PhilippMatthes
approved these changes
Mar 27, 2026
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
api/v1alpha1/datasource_types.go (1)
169-171: Plan rollout behavior for existing Datasource objects.This default change affects newly defaulted specs; existing CRs that already persisted
spec.openstack.syncInterval(e.g.,60s) won’t auto-migrate. If a global shift is intended, add a migration/backfill step.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@api/v1alpha1/datasource_types.go` around lines 169 - 171, You changed the default for SyncInterval (metav1.Duration field named SyncInterval on the Datasource CR) but existing CRs will keep their persisted spec values, so add a rollout/migration plan: implement a one-time backfill that finds Datasource objects missing spec.openstack.syncInterval (or equal to the old implicit default) and patches them to the new default value, or add controller logic in the reconciliation for DatasourceReconciler to set the field when it’s empty and persist the update; ensure the migration is idempotent, logged, and tested against existing CRs so cluster-wide behavior is consistent.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@api/v1alpha1/datasource_types.go`:
- Around line 169-171: You changed the default for SyncInterval (metav1.Duration
field named SyncInterval on the Datasource CR) but existing CRs will keep their
persisted spec values, so add a rollout/migration plan: implement a one-time
backfill that finds Datasource objects missing spec.openstack.syncInterval (or
equal to the old implicit default) and patches them to the new default value, or
add controller logic in the reconciliation for DatasourceReconciler to set the
field when it’s empty and persist the update; ensure the migration is
idempotent, logged, and tested against existing CRs so cluster-wide behavior is
consistent.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 65024bcc-14cd-4867-88c5-b5656095a195
📒 Files selected for processing (2)
api/v1alpha1/datasource_types.gohelm/library/cortex/files/crds/cortex.cloud_datasources.yaml
Contributor
Test Coverage ReportTest Coverage 📊: 68.1% |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
We encountered a bug in the openstack datasource controller in which initial reconciles are pushed back in the priority queue due to the controller enqueueing new reconciles with higher priority. This can lead to some of the datasources never reconciling. By setting the sync interval to a higher value this decreases the chance of the requeued reconcile to push back initial reconciles indefinitely.