search

uw-ictd / colte

Community LTE Project
MIT License
59 stars 27 forks source link

Support for alternative billing paradigms #107

Closed matt9j closed 2 years ago

matt9j commented 2 years ago

Right now colte comes as a single heavyweight package including the open5gs cellular core, all of the colte-provided configuration management, and the basic colte prepaid billing suite. We have several use cases now where there is a desire to support just the core network configuration parts of CoLTE without billing, but right now we require those users to download and install the entire billing suite just to disable it. We also have a new use case where we want to deploy open5gs with the https://github.com/uw-ictd/commgestion-web interface for non-prepaid network access and resource management, but still use the colte configuration and management scripts without duplicating that effort.

Given these needs, this ticket offers a proposal for splitting up the colte package into a small set of subpackages, partly undoing the consolidation that was done when we migrated away from OAI.

To maintain compatibility for current users, the colte package would maintain current behavior, installing the open5gs LTE (4G) core, colte config suite, and colte prepaid billing/network admin.

I propose that we create several sub-packages that are pulled in as dependencies by the top-level colte package as needed. These would be colte-cn-4g (the 4g network core + config scripts), colte-cn-5g (the 5g network core + config scripts), and colte-billing-prepaid (the current prepaid billing and admin suite).

The biggest challenge I know of now with this approach would be handling the sequencing of package installs with partial configuration, particularly the case that the billing suite is installed later in a network, after users have already been added without billing metadata. The billing suite maintains a set of database tables with user information that's usually inserted by the config tool as users are added, but if the billing suite is not installed the database tables will not be present. If it's installed later, we will need to handle the case of "filling in" the missing billing info for the users added earlier.

spencersevilla commented 2 years ago

i really like this proposed split (4g network, 5g network, and billing). My only feedback is in the naming schemes. I'd suggest dropping "cn" from the package names (e.g. "colte-4g" and "colte-5g") just for cleanliness, and TBH I'm not entirely sure what we should do with the "colte" package name. Half of me agrees with you (keep current behavior), half of me wants to move it to just the 4g + config scripts, basically a pointer to "colte-4g" (since it's our most popular use-case by far and kinda the expected behavior), and half of me wants to just retire it as more engineering effort goes into 5g (i.e. "colte" wouldn't exist so you'd have to specify "colte-4g" or "colte-5g"). Or maybe retire it for now and if/when we feel confident about 4g/5g integration bring it back, so installing "colte" gets you both, and "colte-4g" or "colte-5g" just give you one half each, respectively.

matt9j commented 2 years ago

Thanks for your comments @spencersevilla ! Responding to specific pieces:

I'd suggest dropping "cn" from the package names (e.g. "colte-4g" and "colte-5g") just for cleanliness

I think whatever the final naming scheme is, it should still have a separate "name" for the core network part of the system that should map to a package name. The way I was envisioning it was that the entire project would be "colte" (see next point), and the different core networks would both provide "colte-cn", which would be depended upon by the billing package. Given that relationship I thought it would be easier to communicate about colte-cn, of which there are two varieties, colte-cn-5g, and colte-cn-4g, rather than needing folks to keep the mapping in their head that colte-cn (or some other name) can be fulfilled by colte-4g and colte-5g.

not entirely sure what we should do with the "colte" package name

As a project name, I feel like similarly to the 3GPP, it's outgrown its original meaning and now just refers to the project, rather than LTE technology in general :P I'm open to using some other name though if you have thoughts on a better one.

As a package name, I think the most correct way forward would be to have "colte" continue to provide the same behavior it currently does. In the long run maybe it can be retired, but for the transition period I think maintaining current behavior is the most straightforward path until we see a need to retire it.

maybe retire it for now and if/when we feel confident about 4g/5g integration bring it back, so installing "colte" gets you both, and "colte-4g" or "colte-5g" just give you one half each, respectively.

In the long run, I think it would be better to encourage users to install the components they want, rather than the top-level package, as long as we can keep the number of components manageable. I can see an argument for the top-level name giving you the kitchen sink though too, so either way is okay with me. That maybe gets complicated with billing, but since we don't have an alternate billing package yet, is just speculation on my part : )

matt9j commented 2 years ago

WIP is here: https://github.com/uw-ictd/colte/tree/colte-split-core

spencersevilla commented 2 years ago

Great/smart point about the project evolving beyond its name!

For the traditional "colte" namespace, I'm convinced, we can just maintain current behavior for now. I also strong agree with your thought that we should encourage users to just install the components we want, as long as the number stays manageable, cause that led to a number of pain-points back in OAI land. You can see that the open5gs package structure actually has a bunch of packages that are depended on (like open5gs-core) but not actually "advertised" per se.

I appreciate your fleshing out the idea of "colte-cn" being depended on by the billing packages; I'm following you now and think that's a smart idea. I'm okay with "colte-cn-4g" and "colte-cn-5g" containing the "meat" of colte's logic (download and config the core network) but maybe we could provide a virtual package name to be used kind of as a shortcut? My only concern that remains is basically a cleaner namespace. I think we should have a simple and short package name (ideally the default imo) that refers to our most common use case, which I see today being setting up a 4g network without billing, and then in the future being either a 5g or joint 4g/5g network without billing. Any thoughts there?

spencersevilla commented 2 years ago

Thanks for all this hard work! I like this refactor a lot and think it will deeply help us support/manage our use-cases. My small thoughts/questions/nits are below:

Overall this looks awesome and thanks again for your hard work and leading this effort!

matt9j commented 2 years ago

Sorry I missed this feedback since it was in the issue and not in the PR! Responses below:

I noticed that the colte-cn-4g package provides coltenat and open5gs_dbconf. Does it make more sense to put them into colte-essential, given that (I assume) they'd be identical for both cn-4g and cn-5g?

Possibly-- although I think they can be moved at the point we actually add the 5G package. I would advocate for keeping it the way it is now since the change should be cheap.

nfpm-colte-cn-4g.yaml lines 87-93 appear to copy the same file twice, only difference being a file_info:mode difference?

Ah, that's a mistake-- will fix now!

On a related note I noticed that you changed a couple of mode: values to add a lower case o (e.g. from 0o755 to 0o755). I'm curious about this shift, looks strange to me?

This is making them explicitly octal values (instead of decimal), which is required by nfpm. I actually found the issue when removing the leading 0, which then led to them being interpreted as decimal and showing up with bogus permissions :facepalm: The leading 0 implying octal depends on the specific yaml version, and thus becomes somewhat unreliable. Explicit 0o (think like 0x for hex) is the only way to have the parse be consistent across yaml verisons

I also noticed that both colte-cn-4g and colte-essential depend on python3, python3-yaml, and python3-ruamel, despite colte-cn-4g depending on colte-essential. I know its a minor point but I'm not sure what the "best practice" is here - on one hand its nice to be explicit, on the other hand just feels like we're copying code over that doesn't need to be.

I would prefer to stay explicit and not rely on the transitive behavior, since colte-cn-4g is also using these python libraries directly.

I really really like how you define/import packages in coltedb.py, think it's a super smart way to reduce the complexity. I'm wondering if it makes more sense to increase the try statements as we add other permutations (e.g. a postpaid or cn-5g) or does it make more sense to have those files defined using the same package name? I'm thinking, the coltedb.py code wouldnt have any switch statements, simply import "core_network", and then depending on what's installed (cn-4g, cn-5g, cn-trump10g, etc), the corresponding package writes a different file to /usr/lib/python3/dist-packages/core_network.py.

Yeah, I'm not sure what the right approach is there. We can look at the tradeoffs in more detail once we actually have a competing cn implementation. My intuition is that it's better to be explicit about which one is being imported if N is small, and better to go with the package route with a well-defined interface if we start supporting lots of variants. Another nice thing about the multiple try approach is that it makes it a little easier to do local development, since multiple packages are not necessarily exclusive and can be installed side-by-side (probably not realistic for deployment, but nice for a dev machine).

matt9j commented 2 years ago

@spencersevilla responses above! I'll fix the nits pointed out, and close this issue at some point in the future if discussion lapses. Thank you for the detailed review!

spencersevilla commented 2 years ago

All these points make perfect sense to me. Thanks for the explicit pointer about 0o, I've never heard of that as a framing for octal values before... TIL!

matt9j commented 2 years ago

nfpm-colte-cn-4g.yaml lines 87-93 appear to copy the same file twice, only difference being a file_info:mode difference?

Ah, that's a mistake-- will fix now!

Looking at the code, it's an eye test, but actually different files : ) the first is colteconf, and the second is coltedb.

spencersevilla commented 2 years ago

God damn that's an eye-tester for sure. Nice catch!

matt9j commented 2 years ago

Done-- released as colte 0.16.1!