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? > > > > > > >
