longhorn-manager
longhorn-manager copied to clipboard
refactor(setting): add value range in setting definition and use it in validation
ref: https://github.com/longhorn/longhorn/issues/7441
Some ideas:
- use map to present max and min value instead of use int directly.
- Because we still want to
omitemptyanddistinguish 0 or nil. We then need to use*intin the structure so it can show 0 correctly and omitempy when nil. - We then need to declare a value first and assign it to structure as pointer for every definition which might make code not clean.
val := 0 difinition := SettingDefinition { Minimum: &val } - Use map, we can avoid the above problem.
- Because we still want to
- For some multi-choice settings, we used another util function to validate it which is not correct
SettingNameOfflineReplicaRebuilding,SettingNameSnapshotDataIntegrity,SettingNameBackupCompressionMethod,SettingNameReplicaAutoBalancecc @derekbit @c3y1huang- In the setting here, they should be global setting, so they are not allowed to have setting like
Ignored. Ignoredshould be used in volume level setting for it to follow global setting.- Thus, we shouldn't use util validation function like
ValidateReplicaAutoBalance()to validate it because it allowsIgnoredsetting. - I moved these settings to
multi-choicecategory, so they follow the choices inside thedefinition.Choice.
- I separated
< 0and< 1, they should be the same, but I think it make developers more clear about the setting and no need to look up the code.
E2E test: https://ci.longhorn.io/job/private/job/longhorn-tests-regression/6460/console
-vs test_settings.py
E2E test PASSED: https://ci.longhorn.io/job/private/job/longhorn-tests-regression/6521/
-m coretest
E2E test PASSED: https://ci.longhorn.io/job/private/job/longhorn-tests-regression/6528/
This pull request is now in conflict. Could you fix it @ChanYiLin? 🙏
This pull request is now in conflict. Could you fix it @ChanYiLin? 🙏
Conflicts resolved.