Patch Set 1: (5 comments)
https://gerrit.osmocom.org/#/c/3731/1/src/osmo_gsm_tester/bts_osmotrx.py File src/osmo_gsm_tester/bts_osmotrx.py: Line 137: } > (rather first get the defaults and then overlay this dict on top, otherwise All other confs can be taken from self.conf because they come from resources.conf, and cannot be changed at runtime by tests before starting the bts. This variables are not in self.conf because that's a static dict from resources.conf We could do something similar to what we do in nitb.py. At the end of configure(self): self.config = values Then self.config contains a dict with all configs (defaults, statics, dynamics) for that specific object. Then we can use self.config inside conf_for_bsc for everything, instead of using a mix of other object variables and self.conf and defaults. What do you think? https://gerrit.osmocom.org/#/c/3731/1/src/osmo_gsm_tester/bts_sysmo.py File src/osmo_gsm_tester/bts_sysmo.py: Line 154: } > (same) Let's follow the discussion on the other thread. https://gerrit.osmocom.org/#/c/3731/1/src/osmo_gsm_tester/resource.py File src/osmo_gsm_tester/resource.py: Line 527: raise RuntimeError('adding a list of reserved resources to itself?') > I guess this is a fact and not a question :) Copied it from same file, line 367 :) As you seem confident, I'll remove the "?". https://gerrit.osmocom.org/#/c/3731/1/src/osmo_gsm_tester/suite.py File src/osmo_gsm_tester/suite.py: Line 484: arfcn = suite_run._try_reserve_arfcn(supported_bands) > I assumed the arfcn could rather be resolved and picked like any other reso I thought a lot about all pros and cons of doing it one or the other way, and still not entirely happy with the solution. If done the way you mentioned, then it means we always need to pass a ARFCN scenario which matches capabilities of the BTS being requested (which again, is unknown unless we provide too a scenario specifying it), otherwise tests can fail. Same goes for the modem(s) in use. I really don't like this. I think the scenario is useful to select/filter which resources we do want to peek, that support specific functionalities we want to / need for the test, for instance a given cipher, auth, type of BTS, etc. This doesn't mean those scenarios are going to be "configured automatically" during every test using it. That's the job of the objects created from the selection of resources provided by the scenarios, because tuning of all parameters may be more complex that what you can do only combining dictionaries. See for instance the case of BTS passing parameters to BSC. This way we can provide a good "automatic configuration", and still give tests the possibility to configure stuff easily to test some specific case. I'm nowadays starting to think that there are actually 2 type of resources. Static ones (independent, limited amount) like a BTS or a modem, and dynamic ones (more dependent on other resources or specific use of test, or which can be dynamically created and require more logic), such as arfcn or ip_address. I think we should end up moving ip_address to this dynamic way, and do some module/hook to automatically provision new ips if required (eg run in a net namespace and launch a script to create a new virtual iface + ip addr when a new ip_address is requested). The only real cons I find so far with the way I did it is the issue you mention, ie we reserve resources after starting the tests. From user point of view, it's not a big issue as it still shows a NoResourceExn, so not a lot changes here. The only big issue I see with this proposal is that there could be some kind of deadlock while reserving resources if they are not acquired in the same order and there's several tests running in parallel, but I think we can probably solve that too, specially given that only a subset of resources is reserved after the test is started. So, I'm open to improve it, but not sure the way you proposed above is really improving what we have already here. I initially thought about that possibility but I was not convinced. https://gerrit.osmocom.org/#/c/3731/1/suites/register/register_band_1900.py File suites/register/register_band_1900.py: Line 5: arfcn = suite.reserve_arfcn(band='GSM-1900') > this shouldn't be a copy, just the same register test file, except that a * I explained what I think regarding this too on the other comment. Let's continue discussion there. -- To view, visit https://gerrit.osmocom.org/3731 To unsubscribe, visit https://gerrit.osmocom.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I6fb5d95bed1fa50c3deaf62a7a6df3cb276bc3c9 Gerrit-PatchSet: 1 Gerrit-Project: osmo-gsm-tester Gerrit-Branch: master Gerrit-Owner: Pau Espin Pedrol <[email protected]> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: Neels Hofmeyr <[email protected]> Gerrit-Reviewer: Pau Espin Pedrol <[email protected]> Gerrit-Reviewer: neels <[email protected]> Gerrit-HasComments: Yes
