trafficcontrol icon indicating copy to clipboard operation
trafficcontrol copied to clipboard

Port legacy perl TO extension checks from perl to go

Open guzzijason opened this issue 6 years ago • 20 comments

What does this PR (Pull Request) do?

  • This change ports (most) of the legacy perl server checks to go.
  • Performance (and logic problems) of the original DSCP check script was the primary motivation - in our environment, the DSCP scan was taking ~20 hours to run, and the results were unreliable.
  • ADD support for performing DSCP checks of SSL delivery services, which was previously missing.
  • ADD option to FQDN check for validating PTR records
  • Other scripts were ported over for sake of consistency.

Standalone programs only, and no changes are made to existing Traffic Ops API components, hence no integrated testing is included.

Which Traffic Control components are affected by this PR?

  • Traffic Ops Extensions (server checks)

What is the best way to verify this PR?

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

The following criteria are ALL met by this PR

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

Additional Information

guzzijason avatar Oct 22 '19 18:10 guzzijason

Can one of the admins verify this patch?

asf-ci avatar Oct 22 '19 19:10 asf-ci

What's the motivation for this? Check extensions just run as scripts with no actual dependency on the Perl TO codebase, right?

ocket8888 avatar Oct 22 '19 19:10 ocket8888

ok to test

ocket8888 avatar Oct 22 '19 19:10 ocket8888

@ocket8888 I just emailed the dev/users lists with some more info regarding motivation.

The only dependency was an update to the go client library (https://github.com/apache/trafficcontrol/pull/3470) which has already been merged.

guzzijason avatar Oct 22 '19 19:10 guzzijason

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

asf-ci avatar Oct 22 '19 19:10 asf-ci

Can we add those to the Traffic Ops build? Would be nice to be available and compiled when installing traffic_ops RPM.

smalenfant avatar Oct 28 '19 18:10 smalenfant

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

asf-ci avatar Oct 28 '19 19:10 asf-ci

Do not merge - there were changes in ccff78a that broke the code in this PR, that I was not anticipating. I need to sort that out now.

guzzijason avatar Oct 28 '19 20:10 guzzijason

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

asf-ci avatar Oct 28 '19 22:10 asf-ci

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

asf-ci avatar Oct 29 '19 00:10 asf-ci

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

asf-ci avatar Oct 29 '19 00:10 asf-ci

I'm satisfied that the code in this PR is now building and running satisfactorily with the go client library version that are in currently master.

guzzijason avatar Oct 29 '19 00:10 guzzijason

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

asf-ci avatar Oct 29 '19 00:10 asf-ci

Can you rebase to fix those conflicts?

I also think you probably have a better chance of getting this reviewed if each extension is a separate PR, but that's just my opinion.

ocket8888 avatar Dec 19 '19 15:12 ocket8888

@guzzijason Can we rebase this on master again please? Some issues with the Traffic Ops client versioning (my guess)

smalenfant avatar Mar 12 '21 13:03 smalenfant

@smalenfant yes, I've recently updated the ones we're using internally here. I'll work on updating this PR.

guzzijason avatar Mar 17 '21 14:03 guzzijason

@guzzijason In light of TC 6 to remove the API 1.x API, could we resolve the conflict on this one?

smalenfant avatar Sep 09 '21 16:09 smalenfant

How do we build those components? I tried to "wget" them but I can't figure how to compile them. I would like to install them on a different server than my Traffic Ops as well.

smalenfant avatar Sep 09 '21 19:09 smalenfant

It actually looks like they would be packaged in the Traffic Ops RPM. But the build failed.

github.com/apache/trafficcontrol/traffic_ops/app/bin/checks imports
	github.com/apache/trafficcontrol/traffic_ops/client: no matching versions for query "latest"
+ echo 'Could not vendor go module dependencies'

smalenfant avatar Sep 09 '21 19:09 smalenfant

The unversioned client import path was deprecated in 4.0 or 4.1 and disabled/removed in 5.0, I believe. Needs to add a version prefix - should probably just use v4- since it's packaged with TO.

ocket8888 avatar Sep 09 '21 20:09 ocket8888