Skip to content
This repository was archived by the owner on Nov 24, 2025. It is now read-only.

Feature/refetch#6118

Merged
rawlinp merged 35 commits intoapache:masterfrom
tcfdev:feature/refetch
Nov 2, 2021
Merged

Feature/refetch#6118
rawlinp merged 35 commits intoapache:masterfrom
tcfdev:feature/refetch

Conversation

@tcfdev
Copy link
Copy Markdown
Collaborator

@tcfdev tcfdev commented Aug 18, 2021

This PR is to merge the changes necessary to implement the Traffic Ops related changes for the Refetch Blueprint (PR 5910).

TLDR; Modified the /jobs endpoints for Traffic Ops to take an InvalidationType string that must be either 'REFRESH' or 'REFETCH'. There was some extra work around this modification, but that was its primary purpose.

The changes primarily fall on schema changes for the database and Traffic Ops API changes. Currently this is targeting Traffic Ops API 4.0.

TO Database:

  • Remove unused tables and columns (job_status table, job_agent table, various columns in the job table)
  • Modify the data within the job table to reflect the new schema and goal (e.g. remove the keyword column and change the parameters column to ttl_hr, etc.)
  • TODO/Outstanding: Update app/db/seeds.sql and app/db/patches.sql to account for db schema changes Done

TO API:

  • Add v4.0 InvalidationJob endpoints and structs to accomplish the requirements in the blueprint/refetch goals.
  • Modify the previous endpoints to ensure compatibility with the DB schema changes

TO clients:

  • Modify the v4 clients to take advantage of the TO v4.0 API and tc structs
  • Ensure the tests pass

TP:

  • Modify TP to use the stable (3.1) version of the /jobs endpoint

T3C (formerly ORT):

  • Slight modification to ensure tests pass with the DB schema

Documentation:

  • TODO/Outstanding: Want to ensure we are actually going to merge this into v4.0 before modifying all the documentation to reflect this.

Which Traffic Control components are affected by this PR?

  • Documentation
  • Traffic Control Cache Config (T3C, formerly ORT)
  • Traffic Control Go Client v4
  • Traffic Ops
  • Traffic Portal

What is the best way to verify this PR?

To test whether REFETCH works specifically or not, you will need to set the parameter refetch_enabled to true for the global config file in the DB parameter table. Otherwise only REFRESH jobs will be allowed.

The TO database migration scripts will need to be run to ensure the transition from a previous schema works to the new. This is accomplished by using the db/admin utility as outlined here.

Once the DB migration has taken place, the Traffic Ops client API integration tests can be run. You will need TO to be up and running with the to_test database set up as described here.

  • To test all currently supported APIs (ensures backwards compatibility as well) you can do something akin to:
cd github.com/apache/trafficcontrol/traffic_ops/testing/api
# default location is github.com/apache/trafficcontrol/traffic_ops/testing/api/conf/traffic-ops-test.conf
go test --cfg <location for your test config> ./...
  • To test only the invalidation job specific changes for API 4.0 you could run something akin to:
cd github.com/apache/trafficcontrol/traffic_ops/testing/api/v4
# default location is github.com/apache/trafficcontrol/traffic_ops/testing/api/conf/traffic-ops-test.conf
go test -run Jobs --cfg <location for your test config>

Once the Traffic Ops Integration tests confirm the new and previous implementations of the /jobs endpoints still work then you can proceed to running and testing the TP integration tests still work. This will verify that TP is utilizing the previous stable version of the TO API (v3.1) and not the new /jobs endpoints.

Ensure a local version of TO is up and running with the correct DB schema. This should already be the case based on the TO client integration tests from before.

Ensure Traffic Portal is building and running as outline (here)[https://traffic-control-cdn.readthedocs.io/en/latest/development/traffic_portal.html#traffic-portal].

By default, the TP integration tests are set up to run against CIAB. If you're running this locally you will need to modify the config file located at github.com/apache/trafficcontrol/traffic_portal/test/integration/config.json. The params section will need to be modified:

  "params": {
    "apiUrl": " https://localhost:8443/api/4.0", 
    "baseUrl": "https://localhost:9090",
    "login": {
      "username": "admin",
      "password": "twelve12"
    },

Where
apiUrl = Running Traffic Ops API URL (This can and should be set to 4.0)
baseUrl = Running Traffic Portal URL
login.username = existing administrator account username
login.password = existing administrator account password
You can also, optionally, remove the --headless flag to see the integration tests as they run in Chrome Browser

cd github.com/apache/trafficcontrol/traffic_portal/test/integration
npm i
npm run start-webdriver
npm run test

PR submission checklist

  • This PR has tests
  • This PR has documentation - This is delayed for the time being until accepted or rejected in the current RC (6.0) for ATC.
  • This PR has a CHANGELOG.md entry
  • This PR DOES NOT FIX A SERIOUS SECURITY VULNERABILITY (see the Apache Software Foundation's security guidelines for details)

Copy link
Copy Markdown
Contributor

@ocket8888 ocket8888 left a comment

Choose a reason for hiding this comment

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

I tried to look kind of broad-strokes at the design; there's more to say but it's not worth saying yet since this is just a draft.

Biggest thing that stood out to me was the CRUDer rearing its ugly head again

Comment thread lib/go-tc/invalidationjobs.go Outdated
Comment thread lib/go-tc/invalidationjobs.go Outdated
Comment thread traffic_ops/app/db/migrations/2021081321025500_refetch.up.sql Outdated
Comment thread traffic_ops/traffic_ops_golang/invalidationjobs/invalidationjobs.go Outdated
Comment thread traffic_ops/traffic_ops_golang/invalidationjobs/invalidationjobs.go Outdated
@rawlinp
Copy link
Copy Markdown
Contributor

rawlinp commented Aug 20, 2021

Biggest thing that stood out to me was the CRUDer rearing its ugly head again

This PR doesn't appear to be using the CRUDer any more than the original /jobs implementation. It still uses it for Read it appears, but create, update, and delete are all as CRUDer-free as the prior version. I'm guessing the prior version functions were mostly just copied then modified to work with the new changes we want.

@ocket8888
Copy link
Copy Markdown
Contributor

I guess it's not fair to expect him to do more work than he needs to. It's just so terrible right now I can't believe he thought any of it was worth keeping.

@tcfdev tcfdev force-pushed the feature/refetch branch 2 times, most recently from 95b54f6 to e367988 Compare September 1, 2021 16:11
@tcfdev tcfdev marked this pull request as ready for review September 7, 2021 19:49
Copy link
Copy Markdown
Contributor

@rawlinp rawlinp left a comment

Choose a reason for hiding this comment

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

Didn't manage to complete this review, but here are 22 comments in the meantime. A lot of them are related to using a "major version alias", so let me know if I need to explain that better. The basic idea is that it should be easy to add new minor version structs without having to update hundreds of function signatures all over the place.

Comment thread cache-config/t3c-generate/cfgfile/cfgfile_test.go Outdated
Comment thread cache-config/t3c-generate/cfgfile/cfgfile_test.go Outdated
Comment thread cache-config/t3cutil/getdatacfg.go Outdated
Comment thread cache-config/t3cutil/toreq/clientfuncs.go Outdated
Comment thread lib/go-atscfg/atscfg.go Outdated
Comment thread lib/go-tc/invalidationjobs.go Outdated
Comment thread lib/go-tc/invalidationjobs.go Outdated
Comment thread lib/go-tc/invalidationjobs.go Outdated
Comment thread lib/go-tc/invalidationjobs.go Outdated
Comment thread lib/go-util/ptr.go Outdated
@rawlinp rawlinp added new feature A new feature, capability or behavior Traffic Ops related to Traffic Ops cache-config Cache config generation labels Sep 17, 2021
Copy link
Copy Markdown
Contributor

@rawlinp rawlinp left a comment

Choose a reason for hiding this comment

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

Almost done reviewing this entire thing -- just need to finish the 2nd half of traffic_ops/traffic_ops_golang/invalidationjobs/invalidationjobs.go .

Comment thread traffic_ops/app/db/migrations/2021081321025500_refetch.down.sql Outdated
Comment thread traffic_ops/testing/api/v4/jobs_test.go Outdated
Comment thread traffic_ops/testing/api/v4/tc-fixtures.json Outdated
Comment thread traffic_ops/testing/api/v4/traffic_control_test.go Outdated
Comment thread traffic_ops/v4-client/job.go Outdated
Comment thread traffic_ops/traffic_ops_golang/invalidationjobs/invalidationjobs.go Outdated
Comment thread traffic_ops/traffic_ops_golang/invalidationjobs/invalidationjobs.go Outdated
Comment thread traffic_ops/traffic_ops_golang/invalidationjobs/invalidationjobs.go Outdated
Comment thread traffic_ops/traffic_ops_golang/invalidationjobs/invalidationjobs.go Outdated
Comment thread traffic_ops/traffic_ops_golang/invalidationjobs/invalidationjobs.go Outdated
Copy link
Copy Markdown
Contributor

@rawlinp rawlinp left a comment

Choose a reason for hiding this comment

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

Ok, I've finally finished going through this entire thing.

Comment thread traffic_ops/traffic_ops_golang/invalidationjobs/invalidationjobs.go Outdated
Comment thread traffic_ops/traffic_ops_golang/invalidationjobs/invalidationjobs.go Outdated
Comment thread lib/go-atscfg/atscfg.go Outdated
Comment thread lib/go-atscfg/regexrevalidatedotconfig.go Outdated
@zrhoffman
Copy link
Copy Markdown
Member

#6118's TP GHA failed due to #6283 needs to be rebased onto master now that #6284 is merged in order to pass

Comment thread traffic_ops/testing/api/v4/jobs_test.go Outdated
Comment thread traffic_ops/traffic_ops_golang/invalidationjobs/invalidationjobs.go Outdated
@rawlinp
Copy link
Copy Markdown
Contributor

rawlinp commented Nov 2, 2021

I think the DB migrations may need to be updated to a more recent timestamp: traffic_ops/app/db/migrations/2021081321025500_refetch.down.sql

Because there have been other migrations that have come in with more recent timestamps, this one won't be run in dev environments that have already run those more recent migrations.

Taylor Frey added 20 commits November 2, 2021 11:49
The load_schema calls are being performed after migrations have occured.
This is resulting in create_tables.sql attempting to be applied to a DB
state that can conflict. If a table exists, is modified by the migrations
then it may not match the original schema as defined in create_tables.sql.
This can result in a failure when the script is run.
Use concrete values instead of pointers in DB call.
Use explicit ignore for returned error in db call.
Keep routes major+minor aware.
…ang.

Remove references to Perl in README.md test file

Move common database call(s) to dbhelper

Change pointer fields to concrete values
Make the id query param required for updates.

Fix the named parameter colon escape.

Ensure helper function still remains in atscfg library.
When a change is done in go-tc that causes a new struct, use a
type alias to ensure the change is localized and minimized throughout
go-atscfg and t3c
@rawlinp
Copy link
Copy Markdown
Contributor

rawlinp commented Nov 2, 2021

TP integration test failure appears to be a transient issue unrelated to this PR, merging ahead.

@rawlinp rawlinp merged commit c37baa8 into apache:master Nov 2, 2021
@tcfdev tcfdev mentioned this pull request Nov 3, 2021
4 tasks
@zrhoffman zrhoffman added this to the 6.1.0 milestone Nov 3, 2021
@zrhoffman zrhoffman added database relating to setup/installation/structure of the Traffic Ops database TO Client (Go) related to the Go implementation of a TC client Traffic Portal v1 related to Traffic Portal version 1 labels Feb 4, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

cache-config Cache config generation database relating to setup/installation/structure of the Traffic Ops database new feature A new feature, capability or behavior TO Client (Go) related to the Go implementation of a TC client Traffic Ops related to Traffic Ops Traffic Portal v1 related to Traffic Portal version 1

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants