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
Ceph(fix): Updates given to StorageClass won't work. #590
Ceph(fix): Updates given to StorageClass won't work. #590
Conversation
531aa29
to
5623795
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
-
Commits are not clean.
-
Also, here's what this code seems to be doing:
if StorageClass is found delete StorageClass create Default StorageClass if StorageClass is not found create Default StorageClass
We don't want to delete existing StorageClass on every reconcile. We want to recreate it iff an update is required.
@umangachapagain , Thanks for the review, how can I check iff there's an update for the existing SC?. Thanks |
b346166
to
2c039ad
Compare
I will resolve the PR once #563 is merged. |
2c039ad
to
72b8d46
Compare
This PR fixes the issue, where the updates given on StorageClass were not working.Now,the cluster will delete the existing sc and create a new one when requested for updation. Signed-off-by: RAJAT SINGH <rajasing@redhat.com>
72b8d46
to
6e22082
Compare
/lgtm |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: jarrpa, rajatsingh25aug The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
@rajatsingh25aug: The following tests failed, say
Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here. |
This need a backport PR to |
|
@umangachapagain |
I'd expect that to be done.
If it passed on local machine, it should pass on CI too. My point being, we shouldn't merge without understanding why the CI failed. And, if we are sure it's a constantly reappearing issue in CI, we need to fix it. |
Sure, Understood. Also, |
/cherrypick release-4.5 |
@jarrpa: #590 failed to apply on top of branch "release-4.5":
In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
This PR fixes the issue, where the updates given on StorageClass were not working. Now, the cluster will delete the existing sc and create a new one when requested for updation.
Signed-off-by: RAJAT SINGH rajasing@redhat.com
This PR is dependent on #563 and will rebase it when that will get merged.
BZ id: https://bugzilla.redhat.com/show_bug.cgi?id=1846085