>ATSCFG can generate configuration for arbitrary servers, right? It can, but that's not my concern. My concern is that you don't know what version of ORT/atstccfg is on each cache. Is TO the same version that's on the cache? Or is it older or newer? Maybe the cache has a feature or bug fix not in TO yet, or vice versa. Now TC Operators are "verifying" the config is what it should be...with entirely different code. Footgun.
On Tue, Nov 5, 2019 at 9:44 AM ocket 8888 <[email protected]> wrote: > ATSCFG can generate configuration for arbitrary servers, right? If we don't > want it to be callable from the API, then maybe it should be a TP > extension, so operators can still see generated configuration files. > > On Mon, Nov 4, 2019 at 7:57 PM Robert Butts <[email protected]> wrote: > > > >If we do it right, maintaining the two data-loading paths separately > from > > the actual config gen logic shouldn't be that bad. > > > > That's what I had hoped. But it didn't turn out that way. I mean, have a > > look at > > > > > https://github.com/apache/trafficcontrol/tree/master/traffic_ops/traffic_ops_golang/ats > > > > It's just a lot of code, 5000 lines, to load and filter the necessary > data > > from SQL. If you see a way to reduce that, I'm definitely open to > > suggestions. But I didn't see one. > > > > >My main concern is that people depend on the ATS config APIs somewhat in > > order to see what config would actually be generated for a given cache in > > TO > > > > But with the config gen, that's no longer accurate. Because there's no > > guarantee the cache has the same version of ORT/atstccfg as TO. In fact, > > that's one of the big benefits: canary testing. > > > > That's a good point, and it actually worsens my fears about the dangers > of > > keeping the endpoints in there. > > > > Even if we did something like shelling out to atstccfg (which seems like > it > > could be painful to maintain), there's no way to know if any given > cache's > > ORT matches it. This feels like a pretty big footgun. Unless we had some > > way to actual make TO request from the cache, which would need some kind > of > > ORT-HTTP-service, which seems like a lot of development. > > > > >There are probably better ways to just keep the "ATS config gen library" > > pure in the FP sense and simplify the data-loading via the DB/API. If we > do > > it right, maintaining the two data-loading paths separately from the > actual > > config gen logic shouldn't be that bad. If the problem with the DB > > data-loading is that's it's very selective in its queries, and queries > are > > chained together to where results from one query are used to form the > > following query, we can simplify that by just querying all the possible > > data via the DB we need at once. > > > > Again, that's what I'd hoped, and it just didn't turn out that way. If > you > > can figure a way to significantly reduce the LoC that are > > mostly-unmaintained, say I don't know, closer to 1k lines, it'd certainly > > seem less dangerous. ORT version footguns notwithstanding. But I'm not > > seeing a way to do that. Even if we combined the DB loading into one > giant > > object, it'd still be a good bit of code to filter and pull out what each > > individual config file needs. > > > > >While not quite as handy as the ATS config API endpoints, I _think_ the > > `atstccfg` executable could get pretty close, but it could probably use a > > good README to explain how to use it as a standalone tool. Then operators > > could clone the repo locally, build it, configure it, and use it from the > > CLI to view config files for a given server. This might also mean making > it > > easier in general to use as a standalone CLI application. > > > > +1. I think that's the better solution, to eliminate the ORT/TO version > > mismatch footgun. You're right, it doesn't really have a readme or docs, > > because it was intended for ORT not humans. But, if we give it docs and > > maybe make it a little easier to use, and make it the "canonical" way to > > pre-check config files, that seems the safest and least footgunish. > > > > > > On Mon, Nov 4, 2019 at 4:47 PM Rawlin Peters <[email protected]> wrote: > > > > > I was thinking something similar, except instead of an external > > > service, just making the ATS config APIs just shell out to the > > > `atstccfg` executable and returning the results, but that seems > > > weird/bad. > > > > > > There are probably better ways to just keep the "ATS config gen > > > library" pure in the FP sense and simplify the data-loading via the > > > DB/API. If we do it right, maintaining the two data-loading paths > > > separately from the actual config gen logic shouldn't be that bad. If > > > the problem with the DB data-loading is that's it's very selective in > > > its queries, and queries are chained together to where results from > > > one query are used to form the following query, we can simplify that > > > by just querying all the possible data via the DB we need at once. > > > While that sounds expensive, it shouldn't matter if it's only used > > > occasionally by TO operators for checking cache configs. > > > > > > - Rawlin > > > > > > > > > On Mon, Nov 4, 2019 at 4:26 PM ocket 8888 <[email protected]> wrote: > > > > > > > > What if the config generator was an external service configurable > > through > > > > cdn.conf ? You could keep all the logic in one place, the API could > > > remain > > > > unbroken (depending on configuration), and people using Traffic > Portal > > > > could still use the "view configuration files" button. > > > > > > > > On Mon, Nov 4, 2019 at 3:24 PM Robert Butts <[email protected]> wrote: > > > > > > > > > I'd like to propose we deprecate & remove the Traffic Ops ATS Cache > > > Config > > > > > endpoints. In fact, I'd like to propose we Deprecate in Master now, > > > and in > > > > > the following Major Release (5.0) that we remove the code and > change > > > the > > > > > endpoints to return a 501 Not Implemented. > > > > > > > > > > This follows the TC SemVer versioning deprecate schedule (giving > > users > > > 2 > > > > > major versions to upgrade ORT before the endpoints older ORT uses > are > > > > > removed); but does _not_ follow the API SemVer Version Promise and > > > > > deprecate schedule. > > > > > > > > > > This is mostly breaking the Semantic Versioning API Promise. The > > > endpoints > > > > > would still exist, but they wouldn't be usable anymore. Normally, > I'm > > > the > > > > > last person to ever want to break SemVer. But in this particular > > case, > > > I > > > > > think it's more dangerous not to. > > > > > > > > > > Because the Cache Configs have been moved to the client-side > > generator > > > run > > > > > by ORT, these endpoints are no longer used. I had hoped to move > most > > > of the > > > > > code to a library, and I did move all the logic there. But > > > unfortunately, > > > > > what I found when I actually wrote the generator, was that a great > > > deal of > > > > > the code is loading data, not logic. The > > > > > `traffic_ops/traffic_ops_golang/ats` directory has 5,300 lines of > > code. > > > > > > > > > > That's 5k lines which won't be well-maintained, because nobody will > > be > > > > > using it, everyone will be using the ORT with the client-side > > > generator. > > > > > > > > > > These endpoints also aren't regular user endpoints; generally > > speaking, > > > > > nobody should be using them anymore, TC users should be using ORT > > which > > > > > calls atstccfg. If someone does want to generate their own configs, > > you > > > > > should use the lib/go-atscfg library calling regular data > endpoints, > > > not > > > > > the TO config endpoints which aren't used or maintained. If someone > > > really, > > > > > really wants TO to still generate the endpoints (along with a > > > > > different/modified ORT config deployment), you can still write a TO > > > plugin > > > > > to use lib/go-atscfg to create and serve configs. > > > > > > > > > > In summary, I don't take breaking users lightly, but I think the > > > danger of > > > > > keeping the unused ATS configs outweighs breaking SemVer here. > > > > > > > > > > Thoughts? > > > > > > > > > > >
