minikube config get/set/unset cmd #545
Conversation
r2d4
force-pushed
the
config-set
branch
3 times, most recently
from
a706f54
to
4da0b0e
Compare
|
|
||
| // A generous cidr regex | ||
| // validates general format, but will allow some invalid values of the form xxx.xxx.xxx.xxx/xx | ||
| const CIDRregex = `^([0-9]{1,3}\.){3}[0-9]{1,3}(\/[0-9]|[1-2][0-9]3[0-2])?$` |
There was a problem hiding this comment.
Switch this to net.ParseCIDR
r2d4
force-pushed
the
config-set
branch
2 times, most recently
from
818ba3e
to
a06ea20
Compare
| } | ||
| glog.Errorf("Could not open file %s: %s", constants.ConfigFile, err) | ||
| } | ||
| var m map[string]interface{} |
There was a problem hiding this comment.
Should this be the MinikubeConfig type alias you defined above? (same for line 89)
|
This is looking really good. Just a few nits, and you'll need to add copyright headers to fix travis. |
r2d4
force-pushed
the
config-set
branch
2 times, most recently
from
9ff9d79
to
2b662e0
Compare
Current coverage is 32.05% (diff: 27.74%)@@ master #545 diff @@
==========================================
Files 38 44 +6
Lines 1676 1828 +152
Methods 0 0
Messages 0 0
Branches 0 0
==========================================
+ Hits 543 586 +43
- Misses 1020 1125 +105
- Partials 113 117 +4
|
|
@minikube-bot test this please |
1 similar comment
|
@minikube-bot test this please |
r2d4
changed the title
minikube config get/set/unset cmd
|
@dlorenc integration tests are passing now. I was setting a logging config property in the integration test which made the output unexpected. |
|
|
||
| // Reads in the JSON minikube config | ||
| func ReadConfig() MinikubeConfig { | ||
| f, err := os.Open(constants.ConfigFile) |
There was a problem hiding this comment.
Sorry, one last nit. It looks like the error handling in this function is a bit off. glog.Errorf doesn't return anything, it just logs the error. It looks like you could end up returning an unitialized map here if Open throws an error, or decode throws an error.
There was a problem hiding this comment.
Fixed, propagated those errors all the way through
This introduces the minikube config get/set/unset command. At a basic level, it allows a more user friendly interface for setting minikube config values, but it also allows us to run validations and callbacks before and after the values have been set.
|
LGTM |
This introduces the minikube config get/set/unset command. At a basic level, it allows a more user friendly interface for setting minikube config values, but it also allows us to run validations and callbacks before and after the values have been set.
This will allow us to validate settings to catch them before minikube gets incorrectly configured and might help us prevent some problems before they happen. Right now there are only some basic validations, drivers and CIDR but in the future we could have more complex ones if we have interdependent settings.
There are also callbacks that can give some output to the user. Right now there is only the "requires restart" message so that users know they will have to recreate their VM before their driver changes for instance.
In the future we could use this to set more complex settings like enabling addons or something. Let me know what you guys think.
(theres still a bit of work and cleanup to do before merging, but I wanted to get a first draft out before I go any further)