apache / trafficcontrol

Apache Traffic Control is an Open Source implementation of a Content Delivery Network
https://trafficcontrol.apache.org/
Apache License 2.0
1.03k stars 339 forks source link

cacheurl is deprecated in ATS 7.x #1130

Closed smalenfant closed 6 years ago

smalenfant commented 6 years ago

cacheurl is heavily used in the configuration generator for ATS. cacheurl has been deprecated in favor to cachekey in ATS 7.x.

There is currently no selection based on ATS version in ApacheTrafficServer.pm which makes it difficult to make those changes.

@dg4prez Can you take a look at this and tell me how we should implement this? For example here :

This is how we can replace cache_url ignore string with cachekey. How could we add the conditional based on ATS version targeted? Using the trafficserver/package/value?

                        #$text .= " \@plugin=cacheurl.so \@pparam=cacheurl_qstring.config";
                        $text .= " \@plugin=cachekey.so \@pparam=--remove-all-params=true";

This one above is easy, although how do we deal with all the other custom cacheurl_ parameters that gets added in the profiles? (Cache URL Expression in DS definition)

dg4prez commented 6 years ago

We do have a cachekey implementation that is in master now. It’s ds profile based. I’m not sure if there are additional automatically inserted cacheurl references - I’ll have to check that out. I didn’t think there were but I could be mistaken.

This isn’t version based, though.

Derek

On Sep 6, 2017, at 7:50 AM, Steve Malenfant notifications@github.com wrote:

cacheurl is heavily used in the configuration generator for ATS. cacheurl has been deprecated in favor to cachekey in ATS 7.x.

There is currently no selection based on ATS version in ApacheTrafficServer.pm which makes it difficult to make those changes.

@dg4prez https://github.com/dg4prez Can you take a look at this and tell me how we should implement this? For example here :

This is how we can replace cache_url ignore string with cachekey. How could we add the conditional based on ATS version targeted? Using the trafficserver/package/value?

                    #$text .= " \@plugin=cacheurl.so \@pparam=cacheurl_qstring.config";
                    $text .= " \@plugin=cachekey.so \@pparam=--remove-all-params=true";

This one above is easy, although how do we deal with all the other custom cacheurl_ parameters that gets added in the profiles? (Cache URL Expression in DS definition)

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/apache/incubator-trafficcontrol/issues/1130, or mute the thread https://github.com/notifications/unsubscribe-auth/AB8HYoNHCLnusK-RXAQCszba4jzA4HMwks5sfocWgaJpZM4POQLB.

rob05c commented 6 years ago

Like @dg4prez said, we have cachekey working, and configurable. But it's not an automatic transition, and there's no documentation explaining how to do it.

I think we need documentation on how to migrate from cacheurl to cachekey before we can resolve this.

ezelkow1 commented 6 years ago

This should be handled by https://github.com/apache/incubator-trafficcontrol/pull/1602 and https://github.com/apache/incubator-trafficcontrol/pull/1588

rob05c commented 6 years ago

@ezelkow1 Is it all automatic, though? Or are there any config changes or parameters or anything that have to be set to use ATS 7, that need to be documented?

ezelkow1 commented 6 years ago

Its not a completely automatic transition, users will have to convert their cacheurls to cachekeys if they want to move to ats7

Users will need to make parameters for a new cachekey.config with whatever settings they need for their key

rob05c commented 6 years ago

Ok, we should probably put that under a heading for "Using ATS 6+", with a short paragraph explaining, in https://github.com/apache/incubator-trafficcontrol/blob/master/docs/source/admin/traffic_server.rst

ezelkow1 commented 6 years ago

Added documentation in https://github.com/apache/incubator-trafficcontrol/pull/1854

There may be other migration issues that can also be added there

rob05c commented 6 years ago

TC now supports Cachekey, and documentation on migrating has been merged with #1854, therefore I'm resolving this. Any new Cachekey problems should be put in a new issue.