-
Notifications
You must be signed in to change notification settings - Fork 30
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: added zfs storage pool configs [WD-8615] #687
Conversation
Demo starting at https://lxd-ui-687.demos.haus |
Instead of just adding |
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.
Some comments below.
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.
Did QA on a cluster backend, some comments on it below.
e940850
to
0e70dbe
Compare
tested ceph storage pools, no errors with config keys |
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 seems a bit complex to have the getClusterPool
and the getMemberPool
functions, that both rely on a generic getClusterAndMemberConfigs
function. I think all three functions should be defined closer to each other. And then maybe we should collapse them to a single function, because we need, the memberPool
and the clusterPool
always together. Currently we evaluate getClusterAndMemberConfigs
twice and are only ever interested into one of the two return values.
Sure, I will consolidate the logic into a single place and try simplify the dependencies a bit 👍 |
0e70dbe
to
827bee5
Compare
827bee5
to
0344a15
Compare
Signed-off-by: Mason Hu <mason.hu@canonical.com>
0344a15
to
9528b76
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.
LGTM, thanks for adding this 👍
Done
QA