-
Notifications
You must be signed in to change notification settings - Fork 351
Removed Riak from default CiaB #6252
Conversation
infrastructure/cdn-in-a-box/optional/docker-compose.traffic-vault.yml
Outdated
Show resolved
Hide resolved
rawlinp
left a comment
There was a problem hiding this 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?
|
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: |
|
My pitch: do we need to provide an optional Riak-based TV for CiaB at all? |
|
Its useful for, among other things, testing migrating between TV backends. Similarly to how we kept TO Perl in CDN in a Box until the Perl implementation of Traffic Ops was removed, IMO we should keep Riak as an option in CDN in a Box until it is removed as a supported TV backend, even though it is deprecated.
|
|
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. |
|
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. |
|
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. |
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) |
* 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)
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?
What is the best way to verify this PR?
Run CiaB and verify that the
trafficvaultcontainer is not runVerify that CiaB works normally
Run CiaB with the
-f optional/docker-compose.traffic-vault.ymlflag and theTV_BACKENDvariable invariables.envupdated to use RiakVerify 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