-
Notifications
You must be signed in to change notification settings - Fork 351
Add blueprint for Flexible Topologies #4537
Conversation
mitchell852
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.
some more comments
|
Currently, assigning caches to a steering delivery service has no effect as the target delivery services are the delivery services that really serve the content. I think the same should apply to assigning a topology to a steering delivery service. In fact, the delivery service create/update apis should prevent any such assignment by returning a 400 Bad Request if trying to assign a topology to a steering delivery service. |
|
Currently, you can queue updates at the cdn, cache group or server (cache) level. Would it make sense to be able to queue updates for a Topology? I.e. DS A uses Topology B. You make a change to DS A that requires a queue updates. Might make sense to queue at the topology level as opposed to the cdn level. |
I'm hesitant to add "yet another way to queue updates on caches". Topologies are multi-CDN, so would queuing updates on a Topology queue updates on all servers in the Topology across all CDNs? IMO it seems like "queue updates on CDN" is the default operator behavior when making changes to delivery services. |
|
Something else to think about, when adding more tiers to a topology that's going to increase revalidation delays (or worse, likelyhood of never completing). Currently there is a rule that parent tiers must revalidate before child tiers. |
|
That is a good point, I can mention that in the operational impacts. |
|
|
||
| **API Constraints:** | ||
| - `firstHeaderRewrite`, `middleHeaderRewrite`, and `lastHeaderRewrite` can only be set if `topology` is not null | ||
| - `edgeHeaderRewrite` and `midHeaderRewrite` cannot be set if `topology` is not null |
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.
Instead of having parallel mutually exclusive fields serving the same purpose, couldn't we just drop the first and last fields? Seems like that creates additional tradition steps when converting to topologies. The concept of a middleHeaderRewrite doesn't currently exist, so it's fine to be topology only.
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.
This is on purpose -- we want it to be part of the process of migrating legacy DSes to Topologies. Part of that is making sure the existing header-rewrite configs are safe and make sense in the Topology you're assigning the DS to.
|
|
||
| All relevant Delivery Service APIs will have their JSON request and response objects modified to include a new `topology` field which references the name of the topology it's assigned to: | ||
| ```JSON | ||
| All relevant Delivery Service APIs will have their JSON request and response objects modified to include the following new fields: |
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.
The same treatment should be applied to rawRemap. This also lets us remove the ##OVERRIDE anymap hack I think.
What does this PR (Pull Request) do?
Adds a blueprint for Flexible Topologies.
Which Traffic Control components are affected by this PR?
What is the best way to verify this PR?
Read it!
The following criteria are ALL met by this PR