networkop / docker-topo

Docker topology builder for network simulations
BSD 3-Clause "New" or "Revised" License
138 stars 41 forks source link

config not loaded for v2 #27

Closed hansbogert closed 5 years ago

hansbogert commented 5 years ago

Creating the v2/3-node.yml does not seem to load a config in ./config/3-node_cEOS-1 for example

steps to come to above conclusion:

docker ps                                  
CONTAINER ID        IMAGE                  COMMAND                  CREATED             STATUS              PORTS                                 NAMES
35007ac414f2        ceosimage:latest       "/sbin/init"             13 minutes ago      Up 13 minutes       0.0.0.0:9002->443/tcp                 3-node_cEOS-3
ed4f4d1707a6        ceosimage:latest       "/sbin/init"             13 minutes ago      Up 13 minutes       0.0.0.0:9001->443/tcp                 3-node_cEOS-2
e95cd1ca1711        ceosimage:latest       "/sbin/init"             13 minutes ago      Up 13 minutes       0.0.0.0:9000->443/tcp                 3-node_cEOS-1
(testdir)  hvdb@hvdb-ThinkPad-T480s  ~/src/testdir  grep -R 3-node_cEOS-1 config               
(testdir)  ✘ hvdb@hvdb-ThinkPad-T480s  ~/src/testdir  find . |grep  3-node_cEOS-1       
./config/3-node_cEOS-1
(testdir)  hvdb@hvdb-ThinkPad-T480s  ~/src/testdir  docker exec -ti 3-node_cEOS-1 ls -alh /mnt/flash
total 96K
drwxrwxr-x+ 1 root root 4.0K Jun 29 19:15 .
drwxr-xr-x  1 root root 4.0K Jun 16 03:18 ..
-rw-rw-rw-+ 1 root root    0 Jun 29 19:14 .assetTags
drwxrwxr-x+ 2 root root 4.0K Jun 29 19:14 .checkpoints
drwxrwx---+ 2 root root 4.0K Jun 29 19:14 .extensions
-rw-rwxr--+ 1 root root  231 Jun 29 19:13 AsuFastPktTransmit.log
drwxr-xr-x+ 2 root root 4.0K Jun 29 19:13 Fossil
-rw-rwxr--+ 1 root root  142 Jun 29 19:13 SsuRestore.log
-rw-rwxr--+ 1 root root  142 Jun 29 19:13 SsuRestoreLegacy.log
drwxrwx---+ 3 root root 4.0K Jun 29 19:13 debug
drwxrwxr-x+ 2 root root 4.0K Jun 29 19:13 fastpkttx.backup
-rw-rw-r--+ 1 root root  460 Jun 29 19:13 kickstart-config
drwxrwxr-x+ 3 root root 4.0K Jun 29 19:24 persist
drwxrwxr-x+ 3 root root 4.0K Jun 29 19:15 schedule
-rw-rwx---+ 1 root root   13 Jun 16 03:18 zerotouch-config
networkop commented 5 years ago

can you docker-topo --archive your setup and send a link it to me?

hansbogert commented 5 years ago

Ah I get it now, ./config should be relative to the yaml file location, is that intended? Seems like different behaviour as v1

networkop commented 5 years ago

this is how it works (for both v1 and v2). First, we extract CONF_DIR from the env variable or configuration file (defaulting to ./config)

CONF_DIR = t_yml.get('CONF_DIR',os.getenv('CONF_DIR', CONF_DIR))

Second, we check if there a config/ directory present in the same location as the topology YAML file, and if it exists, overwrite CONF_DIR

confdir_alt = os.path.join(t_file_pwd, CONF_DIR)
    if os.path.isdir(confdir_alt):
       CONF_DIR = confdir_alt

Does that match what you're observing?

hansbogert commented 5 years ago

Yes this is what I see, I totally overlooked the config directory in the examples/v2/ directory.

Not sure I agree with the order of precedence though

So if I understand correctly, in order of preference which config dir to load:

I'd argue that the environment should always take precedence, however this is another issue. Thank you for explaining the above.

networkop commented 5 years ago

TBH, this has evolved naturally over time, with little planning, so I'm happy to take suggestions. What order would make sense to you?

hansbogert commented 5 years ago

Ahh my conclusion was wrong, the environment variable does have precedence. The order makes sense.

/update: wrong again.. The yaml takes precedence it seems. I wanted to give my precedence order hereafter, but I'll refrain because it so highly circumstantial and opinion based

However I think the behaviour of CONF_DIR should be described in the README.md