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

Conversation

@mattjackson220
Copy link
Contributor

Closes: #5927

This PR moves the Riak container in CiaB to only run if the optional flag is provided instead of by default.


Which Traffic Control components are affected by this PR?

  • Documentation
  • CDN in a Box

What is the best way to verify this PR?

Run CiaB and verify that the trafficvault container is not run
Verify that CiaB works normally
Run CiaB with the -f optional/docker-compose.traffic-vault.yml flag and the TV_BACKEND variable in variables.env updated to use Riak
Verify that Riak can successfully be used when the flag is provided
Verify that CiaB works normally with Riak

If this is a bugfix, which Traffic Control versions contained the bug?

PR submission checklist

@mattjackson220 mattjackson220 added documentation related to documentation cdn-in-a-box related to the Docker-based CDN-in-a-Box system improvement The functionality exists but it could be improved in some way. labels Oct 5, 2021
Copy link
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.

I unset TV_BACKEND then ran this to build the trafficvault image:

docker-compose -f docker-compose.yml -f optional/docker-compose.traffic-vault.yml build trafficvault

but I'm getting this error:

 > [6/7] ADD enroller/server_template.json     traffic_vault/run.sh     traffic_vault/sslkeys.xml     traffic_ops/to-access.sh     /:
------
failed to compute cache key: "/traffic_vault/sslkeys.xml" not found: not found
ERROR: Service 'trafficvault' failed to build : Build failed

I think maybe all the trafficvault/ references in the Dockerfile need to be optional/trafficvault/ now?

@rawlinp
Copy link
Contributor

rawlinp commented Oct 6, 2021

Update: that fixed the build, but it seems running w/ Riak is not working because the enroller is trying to create the DS before Riak is ready, which makes it 500 since we now generate self-signed certs at creation:

INFO: enroller.go:1054: 2021-10-06T23:16:43.063716314Z: new file : /shared/enroller/deliveryservices/010-demo1.json
INFO: enroller.go:1061: 2021-10-06T23:16:43.063750984Z: creating deliveryservices from /shared/enroller/deliveryservices/010-demo1.json
INFO: enroller.go:234: 2021-10-06T23:16:43.192919103Z: error creating Delivery Service: error requesting Traffic Ops: path 'https://trafficops.infra.ciab.test:443/api/4.0/deliveryservices' gave HTTP error 500 Internal Server Error - error-level alerts: Internal Server Error - alerts: {Alerts:[{Text:Internal Server Error Level:error}]}
INFO: enroller.go:1084: 2021-10-06T23:16:43.192980923Z: error creating deliveryservices from /shared/enroller/deliveryservices/010-demo1.json: error requesting Traffic Ops: path 'https://trafficops.infra.ciab.test:443/api/4.0/deliveryservices' gave HTTP error 500 Internal Server Error - error-level alerts: Internal Server Error

@ocket8888
Copy link
Contributor

My pitch: do we need to provide an optional Riak-based TV for CiaB at all?

@zrhoffman
Copy link
Member

zrhoffman commented Oct 7, 2021 via email

@rawlinp
Copy link
Contributor

rawlinp commented Oct 7, 2021

I was also considering just getting rid of Riak in CIAB entirely, but I came to the same conclusion as Zach. Until we're fully migrated onto Postgres and completely trust that the migration tool won't need any Riak-related bugfixes, we should probably keep it in CIAB.

@ocket8888
Copy link
Contributor

The only reason CiaB needed TO-Perl was because it _wasn't _ optional, since there were API 1.x endpoints that were never re-written into Go, and because it provided the static HTTP file server that Traffic Router needs to work. If it were possible I'd have ripped it out much earlier.

I don't really think CiaB needs to support every setup that the project as a whole supports, especially when it's deprecated.

All that being said, I don't think supporting legacy Riak KV-based TV makes CiaB worse, really, so if someone's willing to do the work anyway, go for it, I guess. Just my two cents.

@rawlinp
Copy link
Contributor

rawlinp commented Oct 7, 2021

It's almost fully supported already, we just need to make the enroller wait to create DSes until Riak is ready or just have the enroller retry failed requests indefinitely. Which, maybe it should do that, because today it just ignores the error and moves on, which makes it harder to realize that something failed and that your CIAB won't start up properly.

@ocket8888
Copy link
Contributor

... today it just ignores the error and moves on ...

Yeah, that is a problem. There are actually some things failing to load that are being ignored, which has been going on for who knows how long. Tenants, I think.

Retrying requests indefinitely would be an improvement on current behavior, but I think it'd be better to just fail if a request fails, and have it wait on Riak being available before running (or maybe just before doing DS ops, since Riak takes sooooo long to start)

@rawlinp rawlinp merged commit b9f8877 into apache:master Oct 7, 2021
@zrhoffman zrhoffman added this to the 6.0.1 milestone Nov 5, 2021
zrhoffman pushed a commit that referenced this pull request Nov 5, 2021
* Removed Riak from default CiaB

* removed trafficvault from github action

* updated per comments

* updated Dockerfile and added wait for vault before creating DSes

(cherry picked from commit b9f8877)
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

cdn-in-a-box related to the Docker-based CDN-in-a-Box system documentation related to documentation improvement The functionality exists but it could be improved in some way.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

CDN in a Box still runs Riak server

4 participants