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

Fix deliveryservice_stats API to use already-vendored influxdb client, fix RPM builds to discover all vendor dirs#3990

Merged
ocket8888 merged 3 commits into
apache:masterfrom
rawlinp:fix-influx-import
Oct 21, 2019
Merged

Fix deliveryservice_stats API to use already-vendored influxdb client, fix RPM builds to discover all vendor dirs#3990
ocket8888 merged 3 commits into
apache:masterfrom
rawlinp:fix-influx-import

Conversation

@rawlinp

@rawlinp rawlinp commented Oct 14, 2019 •
edited
Loading

Copy link
Copy Markdown
Contributor

What does this PR (Pull Request) do?

Move the vendored influxdb library to the top-level vendor dir for TO and TS to share. Fix the deliveryservice_stats API to use the already-vendored library. Fix the rpm builds for TO, TM, and TS to properly discover all vendor dirs the repo and make the RPM builds fail if unvendored dependencies are added (except for the golang.org/x/* packages).

  • This PR is not related to any Issue

Which Traffic Control components are affected by this PR?

  • Traffic Ops
  • Traffic Monitor
  • Traffic Stats
  • build

What is the best way to verify this PR?

  1. Build RPMs and verify CIAB starts up properly
cd infrastructure/cdn-in-a-box
make
docker-compose up --build
  1. GET /api/1.1/deliveryservice_stats, make sure the endpoint still works
  2. Run the traffic_ops_golang unit tests
  3. For good measure, run the TO API tests and make sure they pass
  4. verify RPM installation/upgrade/downgrade: install earlier versions of the rpms on a centos7 host, upgrade using the rpms built with this PR, downgrade, and upgrade

The following criteria are ALL met by this PR

  • This PR fixes a build issue, no tests necessary
  • This PR fixes a build issue, no docs necessary
  • 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)

@rawlinp rawlinp added Traffic Ops related to Traffic Ops tech debt rework due to choosing easy/limited solution labels Oct 14, 2019
@asf-ci

asf-ci commented Oct 14, 2019

Copy link
Copy Markdown
Contributor

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

@asf-ci

asf-ci commented Oct 14, 2019

Copy link
Copy Markdown
Contributor

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

ocket8888
ocket8888 approved these changes Oct 14, 2019
View reviewed changes

@ocket8888 ocket8888 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Builds, tests pass, weasel likes it.

@ocket8888

ocket8888 commented Oct 14, 2019

Copy link
Copy Markdown
Contributor

retest this please

@asf-ci

asf-ci commented Oct 14, 2019

Copy link
Copy Markdown
Contributor

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

@rawlinp rawlinp added the WIP "Work-in-Progress" - do not merge! (use 'draft' pull requests from now on) label Oct 14, 2019
@rawlinp

rawlinp commented Oct 14, 2019

Copy link
Copy Markdown
Contributor Author

Hold up, marked this WIP while I investigate build issues. It seems like the RPM build process isn't discovering the top-level vendor dir.

@rawlinp rawlinp changed the title Fix deliveryservice_stats API to use vendored influxdb client Fix deliveryservice_stats API to use already-vendored influxdb client, fix RPM builds to discover all vendor dirs Oct 17, 2019
@rawlinp rawlinp added the build related to the build process label Oct 17, 2019
@asf-ci

asf-ci commented Oct 17, 2019

Copy link
Copy Markdown
Contributor

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

Rawlin Peters added 3 commits October 18, 2019 16:06
Fix deliveryservice_stats to use vendored influxdb client
3fa1a41
Fix LICENSE file for moved influxdb vendoring
b807a01
Fix TO, TM, and TS Go builds to find and use vendored deps
69a90f0 
Fix the Go builds so that they don't download dependencies that we have
vendored in our repo. Only the golang.org/x/* packages should be
downloaded each time, otherwise the vendored deps should always be used
for the build.
@asf-ci

asf-ci commented Oct 18, 2019

Copy link
Copy Markdown
Contributor

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

@rawlinp rawlinp removed the WIP "Work-in-Progress" - do not merge! (use 'draft' pull requests from now on) label Oct 21, 2019
ocket8888
ocket8888 reviewed Oct 21, 2019
View reviewed changes
Comment thread traffic_monitor/build/traffic_monitor.spec
ocket8888
ocket8888 approved these changes Oct 21, 2019
View reviewed changes

@ocket8888 ocket8888 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Works, tests pass, weasel still likes it

@ocket8888

ocket8888 commented Oct 21, 2019

Copy link
Copy Markdown
Contributor

retest this please

@asf-ci

asf-ci commented Oct 21, 2019

Copy link
Copy Markdown
Contributor

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

@ocket8888

ocket8888 commented Oct 21, 2019

Copy link
Copy Markdown
Contributor

retest this please

@asf-ci

asf-ci commented Oct 21, 2019

Copy link
Copy Markdown
Contributor

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

@ocket8888

ocket8888 commented Oct 21, 2019

Copy link
Copy Markdown
Contributor

ok, CI is busted.

@ocket8888 ocket8888 merged commit 981d022 into apache:master Oct 21, 2019
@rawlinp rawlinp deleted the fix-influx-import branch October 21, 2019 20:27
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

1 more reviewer

@ocket8888 ocket8888 ocket8888 approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

build related to the build process tech debt rework due to choosing easy/limited solution Traffic Ops related to Traffic Ops

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@rawlinp @asf-ci @ocket8888