Skip to content

Refactor schedule-precedence to allow user overrides #820

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Jul 14, 2025

Conversation

black-dragon74
Copy link
Member

@black-dragon74 black-dragon74 commented Jun 20, 2025

This patch refactors the schedule-precedence logic
to allow the value to be overridden by a ConfigMap.

The default mode of operation still prefers PVCs first
as source of key rotation and reclaimspace annotations.

The values for schedule-precedence keys are now concretely
defined as either pvc or storageclass.

Copy link
Member

@Rakshith-R Rakshith-R left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This patch modifies the manager deployment to include the default ConfigMap for csi-addons.

Having it this way will enable us to configure default values on Install without requiring any code changes. It will also enable us to have different values if needed between upstream and downstream.

If it comes in as part of csv or install yamls, won't the default values override whatever values the user has set in the configmap during upgrade ?

@Madhu-1
Copy link
Member

Madhu-1 commented Jun 23, 2025

This patch modifies the manager deployment to include the default ConfigMap for csi-addons.
Having it this way will enable us to configure default values on Install without requiring any code changes. It will also enable us to have different values if needed between upstream and downstream.

If it comes in as part of csv or install yamls, won't the default values override whatever values the user has set in the configmap during upgrade ?

If we are not creating the configmap by default, we should not enforce for the creation, whoever is creating the CM should add the value. For upstream, we can update the document if required to update it.

What are the side effect and possibilities of updating the default in the commandline itself?

Signed-off-by: Niraj Yadav <niryadav@redhat.com>
This patch refactors the schedule-precedence logic
to allow the value to be overridden by a ConfigMap.

The default mode of operation still prefers PVCs first
as source of key rotation and reclaimspace annotations.

The values for `schedule-precedence` keys are now concretely
defined as either `pvc` or `storageclass`.

Signed-off-by: Niraj Yadav <niryadav@redhat.com>
@black-dragon74 black-dragon74 changed the title deploy: Install default ConfigMap as a part of the bundle Refactor schedule-precedence to allow user overrides Jun 26, 2025
Copy link
Member

@Rakshith-R Rakshith-R left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@black-dragon74 ,
Can you please add a PendingReleaseNotes.md file similar to the one we have at CephCSI and document this change in it ?

Comment on lines 7 to 8
- allow override of precedence of key rotation and reclaim space related annotations
using a ConfigMap key `schedule-precedence`.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mention possible values and default in this point itself.
Add a sentence mentioned "sc-only" option is deprecated and will be removed in next release.

This patch adds PendingReleaseNotes to track upcoming changes in
release notes.

Signed-off-by: Niraj Yadav <niryadav@redhat.com>
Copy link
Member

@Rakshith-R Rakshith-R left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks

@Rakshith-R Rakshith-R requested a review from a team July 14, 2025 10:59
mergify bot added a commit that referenced this pull request Jul 14, 2025
@mergify mergify bot merged commit 5100f91 into csi-addons:main Jul 14, 2025
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants