apache / cloudstack-cloudmonkey

Apache Cloudstack Cloudmonkey
Apache License 2.0
91 stars 60 forks source link

cmk ignores the CLI profile arg when loading the cache file #115

Closed kgranroth-tm closed 1 year ago

kgranroth-tm commented 2 years ago

Summary

Cloudmonkey requires a default profile in .cmk/config to read the .cmk/profiles/*.cache files and it will ignore the command line arg when loading said cache, preferring the default profile.

System

OS: MacOS 11 CPU: darwin.x86-64 CMK Version: 6.2.0 (build: 8aae61e, 2021-09-23T10:50:26+0530)

Setup

Create a sample .cmk/config with something like:

profile = dev
[dev]
...
[prod]
...

Then perform a sync:

$ cmk sync
Discovered 194 APIs

Verify that it did create the sync cache for dev:

$ ls ~/.cmk/profiles
dev.cache

Verify that my custom command did sync and is cached:

$ jq '.api[] | select(.name=="myCustomCommand") | {name: .name}' ~/.cmk/profiles/dev.cache   
{
  "name": "myCustomCommand"
}

Attempt 1: Shouldn't work but does

Let's try to use one of those recently synced custom commands for prod, which doesn't have any cache file at all (as seen above):

cmk -d -p prod myCustomCommand
[debug] Trying to load profile: prod
[debug] cmdline args:cmk, -d, -p, prod, myCustomCommand
[debug] ExecCmd args: myCustomCommand
<actual results of working command>

That works, even though it shouldn't. The debug output shows that it is loading prod profile but I would expect it to fail since there is no prod.cache with the custom commands. Instead, it is reading dev.cache for the custom commands and only use prod as the profile for the actual execution of the command.

Attempt 2: Should work, but doesn't

Let's remove the default profile from .cmk/config (setting it to profile = or entirely deleting the line both do the same):

profile =
[dev]
...
[prod]
...

Then, since dev.cache already exists, let's try and use that:

$ cmk -d -p dev myCustomCommand 
[debug] Trying to load profile: dev
[debug] cmdline args:cmk, -d, -p, dev, myCustomCommand
[debug] ExecCmd args: myCustomCommand
Error: unknown command or API requested

That should work since there is a dev.cache file; there is a [dev] profile defined in the .cmk/config; and it does recognize that -p dev is valid. But because there is no profile = dev in .cmk/config, it will not work.

Workarounds

If myCustomCommand is the same in all profiles, then I can simply arbitrarily set profile = dev in .cmk/config and the command line -p <profile> will work for all profiles.

Alternatively, I can have separate config files per profile and use the new -c <config> option, but that only works starting recently.

nvazquez commented 1 year ago

Thanks @kgranroth-tm, I'll work on a fix for these issues

nvazquez commented 1 year ago

Hi @kgranroth-tm - I posted a fix for the problem 1, could you please test it?

About number 2, I'm able to run commands for cached profiles even when the profile line is removed from the ~/.cmk/config file - is there any additional step needed?

kgranroth-tm commented 1 year ago

Hi @nvazquez. Thanks for the attempted fix. Unfortunately, this does not address the root of the bug and instead just impacts one of the workarounds. The bug remains that cmk is not using the CLI profile arg to load the cache file and instead just uses the profile = line in the config file.

Here are a series of tests using your patched version of cmk.

Test 1: Empty profile config; existing dev profile cache; specify dev profile on CLI -- command is still not found since cache is not loaded

$ grep profile ~/.cmk/config
profile =

$ ls ~/.cmk/profiles
dev.cache

$ ./bin/cmk -d -p dev myCustomCommand
[debug] Trying to load profile: dev
[debug] cmdline args:./bin/cmk, -d, -p, dev, myCustomCommand
[debug] ExecCmd args: myCustomCommand
Error: unknown command or API requested

Test 2: Set config profile to dev; existing dev profile; it doesn't matter if profile is set in CLI at all since it works against the config for each

$ grep profile ~/.cmk/config
profile = dev

$ ./bin/cmk -d -p dev myCustomCommand
[debug] Trying to load profile: dev
[debug] cmdline args:./bin/cmk, -d, -p, dev, myCustomCommand
[debug] ExecCmd args: myCustomCommand
<myCustomCommand output>

$ ./bin/cmk -d myCustomCommand
[debug] cmdline args:./bin/cmk, -d, myCustomCommand
[debug] ExecCmd args: myCustomCommand
<myCustomCommand output>

Test 3: Empty profile config; existing dev profile cache; specify prod profile on CLI -- get new accurate error that the prod.cache does not exist

$ grep profile ~/.cmk/config
profile =

$ ./bin/cmk -d -p prod myCustomCommand
Cannot find a cache file for the profile: prod

Test 4: Set profile config to dev; existing dev profile cache; specify prod profile on CLI -- still get new accurate error that the prod.cache does not exist since it still doesn't

$ grep profile ~/.cmk/config
profile = dev

$ ./bin/cmk -d -p prod myCustomCommand
Cannot find a cache file for the profile: prod

Test 5: Specify prod profile on CLI; attempt sync of the cache -- first regression since the error checking happens before the command parsing and thus the command to sync the profile cache will never get there since the profile cache doesn't exist! Catch-22.

$ ./bin/cmk -d -p prod sync
Cannot find a cache file for the profile: prod

Test 6: Set profile config to prod; attempt sync of the cache with no profile set in CLI - this works since cmk does honor whatever the config profile is set to and the new error code won't trigger with no CLI profile arg

$ grep profile ~/.cmk/config
profile = prod

$ ./bin/cmk sync
Discovered 197 APIs

$ ls ~/.cmk/profiles
prod.cache

Test 7: Set profile config to prod; existing prod profile cache; specify prod profile on CLI -- this works and it does read the prod cache correctly

$ grep profile ~/.cmk/config
profile = prod

$ ./bin/cmk -d -p prod myCustomCommand
[debug] Trying to load profile: prod
[debug] cmdline args:./bin/cmk, -d, -p, prod, myCustomCommand
[debug] ExecCmd args: myCustomCommand
<myCustomCommand output>

Test 8: Empty profile config; existing prod profile cache; specify prod profile on CLI -- this still fails because cmk is not using the CLI arg to load the cache. There is no new error because the profile cache does exist (even though it is ignored).

$ grep profile ~/.cmk/config
profile =

$ ./bin/cmk -d -p prod myCustomCommand
[debug] Trying to load profile: prod
[debug] cmdline args:./bin/cmk, -d, -p, prod, myCustomCommand
[debug] ExecCmd args: myCustomCommand
Error: unknown command or API requested
kgranroth-tm commented 1 year ago

An incredibly quick glance at the code suggests that the core problem is that LoadCache is only ever called while reading the config and never when setting the profile, thus it only ever recognizes the config's profile.

An even quicker experimental fix does appear to work for me in all of my tests:

git diff
diff --git a/config/config.go b/config/config.go
index e9d6a02..245f237 100644
--- a/config/config.go
+++ b/config/config.go
@@ -304,6 +304,7 @@ func (c *Config) LoadProfile(name string) {
    conf.Section(name).MapTo(profile)
    setActiveProfile(c, profile)
    c.Core.ProfileName = name
+   LoadCache(c)
 }

 // UpdateConfig updates and saves config
nvazquez commented 1 year ago

Thanks @kgranroth-tm for your tests and suggestion, I have applied it on the PR #131. Can you please review it again?

kgranroth-tm commented 1 year ago

All of my tests now pass with flying colors. Thank you @nvazquez, this patch works great!