Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Cache-Side Config Generator #3762

Merged
merged 15 commits into from Sep 25, 2019
Merged

Conversation

rob05c
Copy link
Member

@rob05c rob05c commented Jul 29, 2019

What does this PR (Pull Request) do?

Adds a cache-side config generator. This is a small go binary (atstccfg), which ORT calls instead of requesting Traffic Ops. The atstccfg generator then generates configs it can, and proxies to TO for configs it can't yet generate.

This PR includes generating parent.config and all profile configs, with everything else being proxied. The plan is to move configs over one at a time, until all configs are generated cache-side.

See https://cwiki.apache.org/confluence/display/TC/Cache-Side+Config+Generation

It also moves the logic into /lib/go-atscfg, a library that can be used if someone wants to generate configs themselves somewhere else.

I've parity-tested the generated configs, against Traffic Ops generation, on all our production edges, mids, and profiles. They all diff identical (except where order is nondeterministic in Perl).

Includes documentation, explaining the new config generation, what's generated where and how.
Includes unit tests.
Includes Changelog.

  • This PR is not related to any other Issue

Which Traffic Control components are affected by this PR?

  • Traffic Ops
  • Traffic Ops ORT

What is the best way to verify this PR?

Install ORT, run, verify config files match previous versions / TO.

If this is a bug fix, what versions of Traffic Control are affected?

Not a bug fix.

The following criteria are ALL met by this PR

  • This PR includes tests OR I have explained why tests are unnecessary
  • This PR includes documentation OR I have explained why documentation is unnecessary
  • This PR includes an update to CHANGELOG.md OR such an update is not necessary
  • This PR includes any and all required license headers
  • This PR ensures that database migration sequence is correct OR this PR does not include a database migration
  • This PR DOES NOT FIX A SERIOUS SECURITY VULNERABILITY (see the Apache Software Foundation's security guidelines for details)

@rob05c rob05c added Traffic Ops related to Traffic Ops cache-config Cache config generation labels Jul 29, 2019
@asfgit
Copy link
Contributor

asfgit commented Jul 29, 2019

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/trafficcontrol-PR/4052/
Test FAILed.

@asfgit
Copy link
Contributor

asfgit commented Aug 6, 2019

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/trafficcontrol-PR/4107/
Test FAILed.

@asfgit
Copy link
Contributor

asfgit commented Aug 7, 2019

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/trafficcontrol-PR/4108/
Test FAILed.

@asfgit
Copy link
Contributor

asfgit commented Aug 9, 2019

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/trafficcontrol-PR/4110/
Test FAILed.

@rob05c rob05c added the new feature A new feature, capability or behavior label Aug 9, 2019
@rob05c rob05c marked this pull request as ready for review August 9, 2019 20:03
@asfgit
Copy link
Contributor

asfgit commented Aug 9, 2019

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/trafficcontrol-PR/4111/
Test FAILed.

@asfgit
Copy link
Contributor

asfgit commented Aug 9, 2019

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/trafficcontrol-PR/4112/
Test FAILed.

Copy link
Member

@ezelkow1 ezelkow1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, ran some comparison tests and go tests. Docs could just use a rewrite

@rob05c
Copy link
Member Author

rob05c commented Aug 29, 2019

Addressed PR comments, fixed merge conflicts.

@rob05c rob05c mentioned this pull request Aug 29, 2019
7 tasks
@asf-ci
Copy link
Contributor

asf-ci commented Aug 29, 2019

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/trafficcontrol-PR/4190/
Test FAILed.

@rob05c
Copy link
Member Author

rob05c commented Aug 29, 2019

retest this please

@asf-ci
Copy link
Contributor

asf-ci commented Aug 29, 2019

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/trafficcontrol-PR/4192/
Test FAILed.

@asf-ci
Copy link
Contributor

asf-ci commented Sep 3, 2019

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/trafficcontrol-PR/4195/
Test FAILed.

@ocket8888
Copy link
Contributor

License header problems:

Error                                 Unknown! traffic_ops/ort/atstccfg/urisigningconfig_test.go
Error                                 Unknown! traffic_ops/ort/atstccfg/urlsigconfig_test.go

@ocket8888
Copy link
Contributor

It also doesn't like the license of your pkg dependency:

Error                                    BSD~! vendor/github.com/pkg/errors/.travis.yml
Error                                     BSD! vendor/github.com/pkg/errors/LICENSE
Error                                    BSD~! vendor/github.com/pkg/errors/Makefile
Error                                    BSD~! vendor/github.com/pkg/errors/appveyor.yml
Error                                    BSD~! vendor/github.com/pkg/errors/bench_test.go
Error                                    BSD~! vendor/github.com/pkg/errors/errors.go
Error                                    BSD~! vendor/github.com/pkg/errors/errors_test.go
Error                                    BSD~! vendor/github.com/pkg/errors/example_test.go
Error                                    BSD~! vendor/github.com/pkg/errors/format_test.go
Error                                    BSD~! vendor/github.com/pkg/errors/json_test.go
Error                                    BSD~! vendor/github.com/pkg/errors/stack.go
Error                                    BSD~! vendor/github.com/pkg/errors/stack_test.go

LICENSE Outdated Show resolved Hide resolved
@rawlinp
Copy link
Contributor

rawlinp commented Sep 6, 2019

Where did we land on the idea to share code between the new atstccfg tool and the API and just replacing DB queries w/ API calls? Are we doing that?

@asf-ci
Copy link
Contributor

asf-ci commented Sep 17, 2019

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/trafficcontrol-PR/4264/
Test PASSed.

@rob05c rob05c force-pushed the to-cache-side-configs branch 2 times, most recently from a0dcbe7 to e22d392 Compare September 17, 2019 20:36
@rob05c
Copy link
Member Author

rob05c commented Sep 17, 2019

License header problems

Fixed.

@rob05c
Copy link
Member Author

rob05c commented Sep 17, 2019

Where did we land on the idea to share code between the new atstccfg tool and the API and just replacing DB queries w/ API calls? Are we doing that?

All the logic is shared, in /lib/go-atscfg. The data loading is still using regular DB calls, though. I didn't see a clean way to use the API within TO. It's technically possible, but it would be much slower, and load lots of unnecessary data.

@asf-ci
Copy link
Contributor

asf-ci commented Sep 17, 2019

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/trafficcontrol-PR/4269/
Test FAILed.

@asf-ci
Copy link
Contributor

asf-ci commented Sep 17, 2019

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/trafficcontrol-PR/4270/
Test PASSed.

@rawlinp
Copy link
Contributor

rawlinp commented Sep 17, 2019

All the logic is shared, in /lib/go-atscfg. The data loading is still using regular DB calls, though. I didn't see a clean way to use the API within TO. It's technically possible, but it would be much slower, and load lots of unnecessary data.

Yeah, I don't think it's necessary to try to use the API within TO. Thanks!

@asf-ci
Copy link
Contributor

asf-ci commented Sep 18, 2019

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/trafficcontrol-PR/4273/
Test PASSed.

@rob05c rob05c mentioned this pull request Sep 18, 2019
7 tasks
@asf-ci
Copy link
Contributor

asf-ci commented Sep 18, 2019

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/trafficcontrol-PR/4278/
Test PASSed.

Adds a client-side ATS config gen interceptor to ORT, with an initial
parent.config gen and passing everything else to TO. Plan is to
add all configs to be client-side generated.
Also adds a client function to specify server and DS IDs, as well as
a missing client func for the existing deliveryservice?cdn param.
For debugging, or emergencies.
Also changes atstccfg to do things ORT needed:

 - changed to return the HTTP code as the exit code, on error
 - added CLI option to return which configs are generated (vs proxied)
 - added retry num option, for failed TO attempts
 - changed to use lib/tc-log, and take args for where to log
 - fixed missing license headers
 - added integrity check via SHA512 or Content-length headers
@asf-ci
Copy link
Contributor

asf-ci commented Sep 20, 2019

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/trafficcontrol-PR/4294/
Test PASSed.

@mitchell852 mitchell852 merged commit dc9bdf5 into apache:master Sep 25, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cache-config Cache config generation new feature A new feature, capability or behavior Traffic Ops related to Traffic Ops
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants