You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cloudstack.apache.org by GitBox <gi...@apache.org> on 2020/10/02 20:18:29 UTC
[GitHub] [cloudstack-cloudmonkey] soreana opened a new pull request #70: Added new option to change default cmk config file.
soreana opened a new pull request #70:
URL: https://github.com/apache/cloudstack-cloudmonkey/pull/70
Cloudmonkey command lets us use a different config file rather than the default one. This pull request added the same functionality to the `cmk` command.
```
cmk -c ./my-config
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack-cloudmonkey] weizhouapache commented on pull request #70: Added new option to change default cmk config file.
Posted by GitBox <gi...@apache.org>.
weizhouapache commented on pull request #70:
URL: https://github.com/apache/cloudstack-cloudmonkey/pull/70#issuecomment-783371982
tested ok.
1. if /root/.cmk/config does not exist, and it is passed
```
# bin/cmk -c /root/.cmk/config list accounts filter=id
Config file doesn't exist.
```
2. if /root/.cmk/config does not exist, but not passed. following command will create a new file
```
# bin/cmk list accounts filter=id
🙈 Error: failed to authenticate with the CloudStack server, please check the settings: Post "http://localhost:8080/client/api": dial tcp [::1]:8080: connect: connection refused
```
3. use a pre-defined config
```
# bin/cmk -c /root/.cmk/config.testcloud list accounts filter=id
{
"account": [
{
"id": "1648da77-c83e-4941-80eb-bd01028afbfb"
}
],
"count": 1
}
```
4. use a non-existing config file
```
# bin/cmk -c /root/.cmk/config.nothing list accounts filter=id
Config file doesn't exist.
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack-cloudmonkey] rhtyd commented on pull request #70: Added new option to change default cmk config file.
Posted by GitBox <gi...@apache.org>.
rhtyd commented on pull request #70:
URL: https://github.com/apache/cloudstack-cloudmonkey/pull/70#issuecomment-801802921
Thanks @weizhouapache merging based on your remark.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack-cloudmonkey] ravening commented on a change in pull request #70: Added new option to change default cmk config file.
Posted by GitBox <gi...@apache.org>.
ravening commented on a change in pull request #70:
URL: https://github.com/apache/cloudstack-cloudmonkey/pull/70#discussion_r514119482
##########
File path: config/config.go
##########
@@ -324,10 +325,13 @@ func (c *Config) UpdateConfig(key string, value string, update bool) {
}
// NewConfig creates or reload config and loads API cache
-func NewConfig() *Config {
+func NewConfig(configFilePath *string) *Config {
defaultConf := defaultConfig()
defaultConf.Core = nil
defaultConf.ActiveProfile = nil
+ if *configFilePath != "" {
+ defaultConf.ConfigFile, _ = filepath.Abs(*configFilePath)
Review comment:
Its creating a config file if the path provided doesn't exist which is not good
It should either throw error message and quit or display warning message and use default config file
```
# ls
bin CHANGES.md cli cmd cmk.go config Dockerfile go.mod go.sum LICENSE Makefile performrelease.sh README.md snap vendor
root@mgt01:~/cloudstack-cloudmonkey#
root@mgt01:~/cloudstack-cloudmonkey# ./bin/cmk -c fkTheWholeWorld
Apache CloudStack 🐵 CloudMonkey 6.1.0
Report issues: https://github.com/apache/cloudstack-cloudmonkey/issues
(localcloud) 🐱 > exit
root@mgt01:~/cloudstack-cloudmonkey# ./bin/cmk -c screwCorona
Apache CloudStack 🐵 CloudMonkey 6.1.0
Report issues: https://github.com/apache/cloudstack-cloudmonkey/issues
(localcloud) 🐱 > exit
root@mgt01:~/cloudstack-cloudmonkey# ./bin/cmk -c breakAllStuff
Apache CloudStack 🐵 CloudMonkey 6.1.0
Report issues: https://github.com/apache/cloudstack-cloudmonkey/issues
(localcloud) 🐱 > exit
# ls -lrt
total 96
-rw-r--r-- 1 root root 3926 Oct 29 09:19 README.md
-rw-r--r-- 1 root root 6501 Oct 29 09:19 Makefile
-rw-r--r-- 1 root root 10225 Oct 29 09:19 LICENSE
-rw-r--r-- 1 root root 1498 Oct 29 09:19 Dockerfile
drwxr-xr-x 2 root root 4096 Oct 29 09:19 cli
-rw-r--r-- 1 root root 5294 Oct 29 09:19 CHANGES.md
drwxr-xr-x 2 root root 4096 Oct 29 09:19 cmd
drwxr-xr-x 2 root root 4096 Oct 29 09:19 snap
-rwxr-xr-x 1 root root 5322 Oct 29 09:19 performrelease.sh
-rw-r--r-- 1 root root 4149 Oct 29 09:19 go.sum
-rw-r--r-- 1 root root 1966 Oct 29 09:19 go.mod
drwxr-xr-x 5 root root 4096 Oct 29 09:19 vendor
drwxr-xr-x 2 root root 4096 Oct 29 09:21 config
-rw-r--r-- 1 root root 2108 Oct 29 09:21 cmk.go
drwxr-xr-x 2 root root 4096 Oct 29 09:23 bin
-rw-r--r-- 1 root root 253 Oct 29 09:29 fkTheWholeWorld <<<<<<<<<<<<<<<<<<<
-rw-r--r-- 1 root root 253 Oct 29 09:30 screwCorona <<<<<<<<<<<<<<<<<<<<<<
-rw-r--r-- 1 root root 253 Oct 29 09:30 breakAllStuff. <<<<<<<<<<<<<<<<<<<<<<
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack-cloudmonkey] rhtyd commented on pull request #70: Added new option to change default cmk config file.
Posted by GitBox <gi...@apache.org>.
rhtyd commented on pull request #70:
URL: https://github.com/apache/cloudstack-cloudmonkey/pull/70#issuecomment-773150966
Need some manual testing, otherwise LGTM
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack-cloudmonkey] weizhouapache commented on pull request #70: Added new option to change default cmk config file.
Posted by GitBox <gi...@apache.org>.
weizhouapache commented on pull request #70:
URL: https://github.com/apache/cloudstack-cloudmonkey/pull/70#issuecomment-801771606
@rhtyd it is tested ok by me
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack-cloudmonkey] weizhouapache commented on pull request #70: Added new option to change default cmk config file.
Posted by GitBox <gi...@apache.org>.
weizhouapache commented on pull request #70:
URL: https://github.com/apache/cloudstack-cloudmonkey/pull/70#issuecomment-742413849
@rhtyd can you look at this pr ?
it works in my testing.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack-cloudmonkey] weizhouapache commented on pull request #70: Added new option to change default cmk config file.
Posted by GitBox <gi...@apache.org>.
weizhouapache commented on pull request #70:
URL: https://github.com/apache/cloudstack-cloudmonkey/pull/70#issuecomment-781956913
> @weizhouapache there was a new change since your last comment can you and/or @ravening help with manual testing, happy to merge after that.
@rhtyd good. Rakesh and me will test it.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack-cloudmonkey] rhtyd commented on pull request #70: Added new option to change default cmk config file.
Posted by GitBox <gi...@apache.org>.
rhtyd commented on pull request #70:
URL: https://github.com/apache/cloudstack-cloudmonkey/pull/70#issuecomment-773150966
Need some manual testing, otherwise LGTM
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack-cloudmonkey] soreana commented on a change in pull request #70: Added new option to change default cmk config file.
Posted by GitBox <gi...@apache.org>.
soreana commented on a change in pull request #70:
URL: https://github.com/apache/cloudstack-cloudmonkey/pull/70#discussion_r540071433
##########
File path: config/config.go
##########
@@ -324,10 +325,13 @@ func (c *Config) UpdateConfig(key string, value string, update bool) {
}
// NewConfig creates or reload config and loads API cache
-func NewConfig() *Config {
+func NewConfig(configFilePath *string) *Config {
defaultConf := defaultConfig()
defaultConf.Core = nil
defaultConf.ActiveProfile = nil
+ if *configFilePath != "" {
+ defaultConf.ConfigFile, _ = filepath.Abs(*configFilePath)
Review comment:
Thanks @ravening fixed it.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack-cloudmonkey] ravening commented on a change in pull request #70: Added new option to change default cmk config file.
Posted by GitBox <gi...@apache.org>.
ravening commented on a change in pull request #70:
URL: https://github.com/apache/cloudstack-cloudmonkey/pull/70#discussion_r514119482
##########
File path: config/config.go
##########
@@ -324,10 +325,13 @@ func (c *Config) UpdateConfig(key string, value string, update bool) {
}
// NewConfig creates or reload config and loads API cache
-func NewConfig() *Config {
+func NewConfig(configFilePath *string) *Config {
defaultConf := defaultConfig()
defaultConf.Core = nil
defaultConf.ActiveProfile = nil
+ if *configFilePath != "" {
+ defaultConf.ConfigFile, _ = filepath.Abs(*configFilePath)
Review comment:
Its creating config file if the path provided doesnt exist which is not good
```
# ls
bin CHANGES.md cli cmd cmk.go config Dockerfile go.mod go.sum LICENSE Makefile performrelease.sh README.md snap vendor
root@mgt01:~/cloudstack-cloudmonkey#
root@mgt01:~/cloudstack-cloudmonkey# ./bin/cmk -c fkTheWholeWorld
Apache CloudStack 🐵 CloudMonkey 6.1.0
Report issues: https://github.com/apache/cloudstack-cloudmonkey/issues
(localcloud) 🐱 > exit
root@mgt01:~/cloudstack-cloudmonkey# ./bin/cmk -c screwCorona
Apache CloudStack 🐵 CloudMonkey 6.1.0
Report issues: https://github.com/apache/cloudstack-cloudmonkey/issues
(localcloud) 🐱 > exit
root@mgt01:~/cloudstack-cloudmonkey# ./bin/cmk -c breakAllStuff
Apache CloudStack 🐵 CloudMonkey 6.1.0
Report issues: https://github.com/apache/cloudstack-cloudmonkey/issues
(localcloud) 🐱 > exit
# ls -lrt
total 96
-rw-r--r-- 1 root root 3926 Oct 29 09:19 README.md
-rw-r--r-- 1 root root 6501 Oct 29 09:19 Makefile
-rw-r--r-- 1 root root 10225 Oct 29 09:19 LICENSE
-rw-r--r-- 1 root root 1498 Oct 29 09:19 Dockerfile
drwxr-xr-x 2 root root 4096 Oct 29 09:19 cli
-rw-r--r-- 1 root root 5294 Oct 29 09:19 CHANGES.md
drwxr-xr-x 2 root root 4096 Oct 29 09:19 cmd
drwxr-xr-x 2 root root 4096 Oct 29 09:19 snap
-rwxr-xr-x 1 root root 5322 Oct 29 09:19 performrelease.sh
-rw-r--r-- 1 root root 4149 Oct 29 09:19 go.sum
-rw-r--r-- 1 root root 1966 Oct 29 09:19 go.mod
drwxr-xr-x 5 root root 4096 Oct 29 09:19 vendor
drwxr-xr-x 2 root root 4096 Oct 29 09:21 config
-rw-r--r-- 1 root root 2108 Oct 29 09:21 cmk.go
drwxr-xr-x 2 root root 4096 Oct 29 09:23 bin
-rw-r--r-- 1 root root 253 Oct 29 09:29 fkTheWholeWorld <<<<<<<<<<<<<<<<<<<
-rw-r--r-- 1 root root 253 Oct 29 09:30 screwCorona <<<<<<<<<<<<<<<<<<<<<<
-rw-r--r-- 1 root root 253 Oct 29 09:30 breakAllStuff. <<<<<<<<<<<<<<<<<<<<<<
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack-cloudmonkey] weizhouapache commented on pull request #70: Added new option to change default cmk config file.
Posted by GitBox <gi...@apache.org>.
weizhouapache commented on pull request #70:
URL: https://github.com/apache/cloudstack-cloudmonkey/pull/70#issuecomment-784913332
@soreana could you add the new flag to cmd/command.go ?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack-cloudmonkey] soreana commented on pull request #70: Added new option to change default cmk config file.
Posted by GitBox <gi...@apache.org>.
soreana commented on pull request #70:
URL: https://github.com/apache/cloudstack-cloudmonkey/pull/70#issuecomment-784946311
@weizhouapache good point. Added!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack-cloudmonkey] rhtyd merged pull request #70: Added new option to change default cmk config file.
Posted by GitBox <gi...@apache.org>.
rhtyd merged pull request #70:
URL: https://github.com/apache/cloudstack-cloudmonkey/pull/70
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack-cloudmonkey] ravening commented on a change in pull request #70: Added new option to change default cmk config file.
Posted by GitBox <gi...@apache.org>.
ravening commented on a change in pull request #70:
URL: https://github.com/apache/cloudstack-cloudmonkey/pull/70#discussion_r507817543
##########
File path: config/config.go
##########
@@ -324,10 +325,13 @@ func (c *Config) UpdateConfig(key string, value string, update bool) {
}
// NewConfig creates or reload config and loads API cache
-func NewConfig() *Config {
+func NewConfig(configFilePath *string) *Config {
defaultConf := defaultConfig()
defaultConf.Core = nil
defaultConf.ActiveProfile = nil
+ if *configFilePath != "" {
+ defaultConf.ConfigFile, _ = filepath.Abs(*configFilePath)
Review comment:
What if the file doesnt exist?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack-cloudmonkey] weizhouapache commented on pull request #70: Added new option to change default cmk config file.
Posted by GitBox <gi...@apache.org>.
weizhouapache commented on pull request #70:
URL: https://github.com/apache/cloudstack-cloudmonkey/pull/70#issuecomment-703078094
tested ok
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack-cloudmonkey] rhtyd commented on pull request #70: Added new option to change default cmk config file.
Posted by GitBox <gi...@apache.org>.
rhtyd commented on pull request #70:
URL: https://github.com/apache/cloudstack-cloudmonkey/pull/70#issuecomment-781929581
@weizhouapache there was a new change since your last comment can you and/or @ravening help with manual testing, happy to merge after that.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org