Skip to content
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

feat(provisioner-localpv): Merge CAS config from PersistentVolumeClaim #190

Merged
merged 4 commits into from
Jul 15, 2024

Conversation

niladrih
Copy link
Member

@niladrih niladrih commented Jun 28, 2024

Requires #191

This change merges the CAS config on the PVC with the SC (SC get preference in case of conflict), when provisioning volumes. Imports changes from @nobiit's PR #144.

@niladrih niladrih force-pushed the nobidev/features/merge-cas-pvc branch from a7ada41 to 88bc07a Compare June 28, 2024 07:35
Copy link
Member

@Abhinandan-Purkait Abhinandan-Purkait left a comment

Choose a reason for hiding this comment

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

Can you please put in the description what is this covering?

@codecov-commenter
Copy link

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

Attention: Patch coverage is 0% with 25 lines in your changes missing coverage. Please review.

Project coverage is 37.93%. Comparing base (2f165df) to head (6a5e8bf).
Report is 10 commits behind head on develop.

Files Patch % Lines
cmd/provisioner-localpv/app/config.go 0.00% 8 Missing ⚠️
...kg/kubernetes/api/apps/v1/deployment/deployment.go 0.00% 8 Missing ⚠️
...kg/kubernetes/api/storage/v1/storageclass/build.go 0.00% 8 Missing ⚠️
...ernetes/api/core/v1/persistentvolumeclaim/build.go 0.00% 1 Missing ⚠️

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@             Coverage Diff             @@
##           develop     #190      +/-   ##
===========================================
- Coverage    38.20%   37.93%   -0.28%     
===========================================
  Files           36       36              
  Lines         3348     3372      +24     
===========================================
  Hits          1279     1279              
- Misses        1987     2011      +24     
  Partials        82       82              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@nobiit
Copy link
Contributor

nobiit commented Jul 2, 2024

Thanks guys, I missed it. Thanks for helping me out.

@niladrih niladrih force-pushed the nobidev/features/merge-cas-pvc branch 8 times, most recently from 4598245 to 740e66d Compare July 3, 2024 09:37
@niladrih
Copy link
Member Author

niladrih commented Jul 5, 2024

Thanks guys, I missed it. Thanks for helping me out.

No problem :)

nobiit and others added 3 commits July 15, 2024 04:56
Signed-off-by: Nobi <[email protected]>
Signed-off-by: Niladri Halder <[email protected]>
In the typical use-case for the provisioner, an admin sets the config on the
StorageClass, and a user sets the config on the PVC. The admin's config may
set boundaries on to the volumes which the PVC config map try to override.
Preferring StorageClass config when there is a conflict may be the right way
to go.

Signed-off-by: Niladri Halder <[email protected]>
@niladrih niladrih force-pushed the nobidev/features/merge-cas-pvc branch from 740e66d to f881a87 Compare July 15, 2024 09:17
@niladrih niladrih merged commit cab53c4 into develop Jul 15, 2024
8 checks passed
@niladrih niladrih deleted the nobidev/features/merge-cas-pvc branch July 15, 2024 09:38
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.

4 participants