-
Notifications
You must be signed in to change notification settings - Fork 147
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: add snapshot max count and size #2140
Conversation
ec24da2
to
c6c2969
Compare
c6c2969
to
5cc1b16
Compare
5cc1b16
to
bb42135
Compare
774e4e0
to
09c6b82
Compare
SnapshotMaxCount int `json:"snapshotMaxCount"` | ||
// +kubebuilder:validation:Type=string | ||
// +optional | ||
SnapshotMaxSize int64 `json:"snapshotMaxSize,string"` |
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.
Why kubebuilder:validation:Type=string
instead of int
?
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.
I followed VolumeSpec.Size
. @derekbit, could you share the idea to use string validation? This was added from #1136. Thank you.
longhorn-manager/k8s/pkg/apis/longhorn/v1beta2/volume.go
Lines 210 to 213 in 9cb537d
type VolumeSpec struct { | |
// +kubebuilder:validation:Type=string | |
// +optional | |
Size int64 `json:"size,string"` |
This pull request is now in conflicts. Could you fix it @FrankYang0529? 🙏 |
09c6b82
to
dda6a29
Compare
This pull request is now in conflicts. Could you fix it @FrankYang0529? 🙏 |
4b5dbec
to
35b0a82
Compare
This pull request is now in conflicts. Could you fix it @FrankYang0529? 🙏 |
8f2e738
to
398e6c5
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.
A few comments.
fd49333
to
e4b43f2
Compare
Signed-off-by: PoAn Yang <poan.yang@suse.com>
Signed-off-by: PoAn Yang <poan.yang@suse.com>
Signed-off-by: PoAn Yang <poan.yang@suse.com>
ref: longhorn/longhorn#6563
rely on:
Test Plan:
snapshot-max-count
settingsnapshotMaxCount
. The defaultsnapshotMaxCount
on the Volume will be 5.snapshotMaxCount
in Volume &updateSnapshotMaxCount
actionsnapshotMaxCount
which is lower than2
or more than250
.snapshotMaxCount=3
andsnapshotMaxSize=0
.volume-head
will be on the second branch)volume-head
will be on the third branch)updateSnapshotMaxCount
action to updatesnapshotMaxCount
to4
.updateSnapshotMaxCount
action to updatesnapshotMaxCount
to3
, because we can't makesnapshotMaxCount
be smaller than current usage.updateSnapshotMaxCount
action to updatesnapshotMaxCount
to3
.snapshotMaxSize
in Volume &updateSnapshotMaxSize
actionsnapshotMaxSize
which is more than0
and lower than2 * volume.spec.size
.size=1073741824
(1G),snapshotMaxCount=250
andsnapshotMaxSize=2357198848
(2.2G).file1
with950M
in the volume.file1
and createfile2
with950M
in the volume.file2
and createfile3
with950M
in the volume.2.2G
.updateSnapshotMaxSize
action to updatesnapshotMaxSize
to3430940672
(3.2G).updateSnapshotMaxSize
action to updatesnapshotMaxSize
to2357198848
(2.2G), because we can't makesnapshotMaxSize
be smaller than current usage.updateSnapshotMaxSize
action to updatesnapshotMaxSize
to2357198848
(2.2G).upgrade test