-
Notifications
You must be signed in to change notification settings - Fork 73
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: enable specify localDiskName for localdiskclaim #1089
Conversation
Codecov Report
❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more. @@ Coverage Diff @@
## main #1089 +/- ##
==========================================
- Coverage 42.15% 40.55% -1.61%
==========================================
Files 24 24
Lines 1587 1662 +75
==========================================
+ Hits 669 674 +5
- Misses 846 914 +68
- Partials 72 74 +2
... and 1 file with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
Available(). | ||
HasNotReserved(). |
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.
what will happen if localdiskname is matched but other attrributes are not? is this disk can be assigned directly to a disk claim?
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.
No, the logic of all conditions is AND.
@@ -72,6 +72,9 @@ type DiskClaimDescription struct { | |||
|
|||
// Capacity of the disk in bytes | |||
Capacity int64 `json:"capacity,omitempty"` | |||
|
|||
// LocalDiskName for specifying LocalDisk | |||
LocalDiskName string `json:"localDiskName,omitempty"` |
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.
should be array of LocalDiskName?
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.
It's a good idea, but it may conflict with NodeName
property. So in my view, localDiskName
is fine.
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.
it may be possible to specify multiple LocalDisk per node, right?
00dada5
to
baccdcf
Compare
Signed-off-by: 庞玮 <pangwei@qianxin.com>
baccdcf
to
af9fbf7
Compare
Signed-off-by: 庞玮 <pangwei@qianxin.com>
Signed-off-by: 庞玮 <pangwei@qianxin.com>
Signed-off-by: 庞玮 <pangwei@qianxin.com>
af9fbf7
to
cd76bd9
Compare
I've updated this pr. New properties, Review again plz. |
lgtm thanks for the contribution!! |
What this PR does / why we need it:
#1087
Special notes for your reviewer:
Does this PR introduce a user-facing change?