Proposal: Remove Traffic Ops API Cache Config Endpoints

[Robert Butts <ro...@apache.org>]

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?

Re: Proposal: Remove Traffic Ops API Cache Config Endpoints

[Rawlin Peters <ra...@apache.org>]

+0, only because I personally don't have to check configs using the
ATS config APIs very often but am generally in favor of deleting cruft
out of the API.

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 (notwithstanding things like __RETURN__ or the anymap
override stuff that ORT processes before laying down the configs).
That's pretty handy, so IMO we'd need something equally as handy for
anyone to use to check a config file for a given cache without having
to ssh into the cache itself.

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.

- Rawlin

On Mon, Nov 4, 2019 at 3:24 PM Robert Butts <ro...@apache.org> 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?

Re: Proposal: Remove Traffic Ops API Cache Config Endpoints

[ocket 8888 <oc...@gmail.com>]

Well their alternative is `ssh` into the server in question and run it
there. Which, depending on the organization, they may or may not have
access to do, and also it's a terrible UX to have to leave a configuration
UI and connect to a different machine via a different protocol manually to
see the effects of that configuration. It'd be one thing if we had knobs to
turn and could explain exactly what each did, but with Profiles and
Parameters the only way to know what will result from any combination of
ConfigFIle, Value, and Name is to have both an intimate understanding of
ATS and either see the output configuration files or read through all of
the ATSCFG code base to gain a full understanding. I think we're already
asking a lot and even if it's just an approximation I'm loathe to make
configuration files a pain to figure out.

On Tue, Nov 5, 2019 at 9:59 AM Robert Butts <ro...@apache.org> wrote:

> >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 <oc...@gmail.com> 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 <ro...@apache.org> 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 <ra...@apache.org>
> 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 <oc...@gmail.com>
> 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 <ro...@apache.org>
> 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?
> > > > > >
> > > >
> > >
> >
>

Re: Proposal: Remove Traffic Ops API Cache Config Endpoints

[Robert Butts <ro...@apache.org>]

>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 <oc...@gmail.com> 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 <ro...@apache.org> 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 <ra...@apache.org> 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 <oc...@gmail.com> 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 <ro...@apache.org> 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?
> > > > >
> > >
> >
>

Re: Proposal: Remove Traffic Ops API Cache Config Endpoints

[ocket 8888 <oc...@gmail.com>]

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 <ro...@apache.org> 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 <ra...@apache.org> 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 <oc...@gmail.com> 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 <ro...@apache.org> 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?
> > > >
> >
>

Re: Proposal: Remove Traffic Ops API Cache Config Endpoints

[Robert Butts <ro...@apache.org>]

>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 <ra...@apache.org> 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 <oc...@gmail.com> 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 <ro...@apache.org> 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?
> > >
>

Re: Proposal: Remove Traffic Ops API Cache Config Endpoints

[Rawlin Peters <ra...@apache.org>]

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 <oc...@gmail.com> 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 <ro...@apache.org> 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?
> >

Re: Proposal: Remove Traffic Ops API Cache Config Endpoints

[ocket 8888 <oc...@gmail.com>]

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 <ro...@apache.org> 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?
>