Validate resources.request.storage for each item in spec.tiflash.storageClaims#4635
Validate resources.request.storage for each item in spec.tiflash.storageClaims#4635hoyhbx wants to merge 2 commits intopingcap:masterfrom
resources.request.storage for each item in spec.tiflash.storageClaims#4635Conversation
|
[REVIEW NOTIFICATION] This pull request has been approved by:
To complete the pull request process, please ask the reviewers in the list to review by filling The full list of commands accepted by this bot can be found here. DetailsReviewer can indicate their review by submitting an approval review. |
|
/merge |
|
This pull request has been accepted and is ready to merge. DetailsCommit hash: 6fc2786 |
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## master #4635 +/- ##
==========================================
- Coverage 59.01% 59.01% -0.01%
==========================================
Files 227 227
Lines 26357 26357
==========================================
- Hits 15555 15554 -1
- Misses 9298 9299 +1
Partials 1504 1504
|
|
/run-all-tests |
|
/test pull-e2e-kind-across-kubernetes |
|
/run-all-tests |
1 similar comment
|
/run-all-tests |
|
/test pull-e2e-kind pull-e2e-kind-serial |
bdb8aad to
ebdd0e2
Compare
|
Merge canceled because a new commit is pushed. |
|
Hi @KanShiori I just rebased it, is it possible for you to take a look and merge this fix? |
What problem does this PR solve?
This PR solves the problem that assigning empty objects under
storageClaimsof tiflash can bypass the validation of the operator.We found that when we specified tiflash spec with some empty objects under
storageClaims, we got rejection from the events saying thatstorageis a required field. However, we did not get error messages or alerts from the operator indicating thatstorageis required.Additionally, from the source code of TiDB operator, we discovered that assigning empty objects under
storageClaimscan bypass the operator's validation code. For details, please refer to the issue we submitted: #4613What is changed and how does it work?
We believed it is necessary to check whether
storageis set when specifyingstorageClaimsfor tiflash. We added a sanity check in the functionvalidateTiFlashSpecto see whetherstorageexists in the resource requests.Code changes
Tests
Side effects
Related changes
Release Notes
Please refer to Release Notes Language Style Guide before writing the release note.