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

Ds active flag#4996

Closed
ocket8888 wants to merge 18 commits intoapache:masterfrom
ocket8888:ds-active-flag
Closed

Ds active flag#4996
ocket8888 wants to merge 18 commits intoapache:masterfrom
ocket8888:ds-active-flag

Conversation

@ocket8888
Copy link
Copy Markdown
Contributor

@ocket8888 ocket8888 commented Aug 27, 2020

What does this PR (Pull Request) do?

  • This PR is not related to any Issue

This PR implements the Delivery Services Active flag changes described in the "Delivery Services 'Active' flag" blueprint in Traffic Ops. Traffic Portal is changed to use the stable 3.1 version of the Traffic Ops API for Delivery Service-related API calls until the necessary changes are made to support the new typing of the 'active' flag in APIv4.

Which Traffic Control components are affected by this PR?

  • CDN in a Box
  • Documentation
  • Traffic Ops
  • Traffic Portal

What is the best way to verify this PR?

Make sure all the tests pass and verify that PRIMED and INACTIVE Delivery Services don't appear in CDN Snapshots or monitoring configuration.

The following criteria are ALL met by this PR

  • This PR includes tests
  • This PR includes documentation
  • This PR includes an update to CHANGELOG.md
  • This PR includes any and all required license headers
  • This PR ensures that database migration sequence is correct
  • This PR DOES NOT FIX A SERIOUS SECURITY VULNERABILITY

@ocket8888 ocket8888 marked this pull request as ready for review May 19, 2021 18:09
@ocket8888 ocket8888 added 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. Traffic Ops related to Traffic Ops Traffic Portal v1 related to Traffic Portal version 1 labels May 19, 2021
ocket8888 added 17 commits May 21, 2021 13:05
This also made a few fields that are never allowed to be nil in a valid context not 'nullable'
Also removed unnecessary lastUpdated fields - many of which are now incorrect, as DSes use RFC3339
…inactive' instead of 'fully active' which was a breaking change
…things until TP work can be done to support new 'active' typing
@rawlinp
Copy link
Copy Markdown
Contributor

rawlinp commented Jun 15, 2021

Revisiting the blueprint, this in is the problem description:

Currently, there is no way to create a Delivery Service (with assigned servers) that will not be distributed to cache server configuration

With topologies, is the with assigned servers bit even relevant anymore? Assigning/unassigning topologies will be more straightforward than assigning individual servers, so is this a big enough deal that we can't just have operators unassign the DS's topology or create the DS without one until ready to distribute the config?

@ocket8888
Copy link
Copy Markdown
Contributor Author

The Topology method of server assignment has the same issue as direct assignment afaik, but it actually presents a greater process problem IMO, since the "Clone DS" functionality in TP wouldn't duplicate server assignments, but will now clone Topology assignment.

is this a big enough deal that we can't just have operators unassign the DS's topology or create the DS without one until ready to distribute the config?

We could also just have had operators not assign servers until the DS config was ready for cache servers to receive it. The answer is that I'm not sure, but I don't think the situation has changed.

On another note, this should be a draft for the moment. The changes it makes collide heavily with #5922 and I don't intend to update it until that PR is merged.

@ocket8888 ocket8888 marked this pull request as draft June 15, 2021 19:03
@rob05c
Copy link
Copy Markdown
Member

rob05c commented Jun 15, 2021

The Topology method of server assignment has the same issue as direct assignment afaik, but it actually presents a greater process problem IMO, since the "Clone DS" functionality in TP wouldn't duplicate server assignments, but will now clone Topology assignment.

We could also just have had operators not assign servers until the DS config was ready for cache servers to receive it. The answer is that I'm not sure, but I don't think the situation has changed.

Hm, does it solve that problem if the Operator creates a "Zero Topology," that new in-progress DSes can be assigned to? Would we even need any code to support that, or can we just make it an idiom?

Capabilities are another way to solve the problem. For new DSes, you can simply make a Required Capability that no Server has, "DS_IN_PROGRESS" or whatever. For Servers, Operators can create a "Used" Capability and make it a Requirement of the/all DSes, and then new in-progress Servers won't be used by anything until they're given that Capability.

@ocket8888 ocket8888 mentioned this pull request Jun 15, 2021
6 tasks
@rawlinp
Copy link
Copy Markdown
Contributor

rawlinp commented Jun 15, 2021

It sounds like we have many ways to skin this cat already without needing the changes for DS Active Flags. In the spirit of agility, it's almost been a year since this was originally opened, but it hasn't been enough of a priority to get it across the finish line.

Looking at the original issue that this was intended to fix, there is this:

so that the DS is removed from the caches, but could be re-enabled quickly, but not deleted from TODB (and the associated Keys and Certs deleted from Traffic Vault.)

That is more of an issue with ds-server assignments, because you'd have to track all the caches it was originally assigned to, remove them, and save the list somewhere in case they need to be quickly re-added. With Topologies, this is as simple as saving the original topology name somewhere (perhaps in the DS description), and unassigning the topology. If the DS needs to be quickly reactivated, all you have to do is reassign the topology from the description.

@rob05c
Copy link
Copy Markdown
Member

rob05c commented Jun 15, 2021

It sounds like we have many ways to skin this cat already without needing the changes for DS Active Flags.

I generally agree, but it does still make me a little uncomfortable that it isn't obvious an "Inactive" DS is still going to Caches (but it needs to be).

We should at least add some more documentation making that as clear as possible, and also noting some of the options for not sending it to caches.

It'd also be really nice if we could rename the field to be more obvious; but that's obviously a breaking change, which is a pain, and I don't know what that name would be.

@rawlinp
Copy link
Copy Markdown
Contributor

rawlinp commented Jun 15, 2021

We could probably save some headaches by changing the field name in TP from active to active (routed) and providing a better description in the tooltip.

@ocket8888 ocket8888 closed this Nov 1, 2022
@ocket8888
Copy link
Copy Markdown
Contributor Author

obsoleted by #7099

@ocket8888 ocket8888 deleted the ds-active-flag branch November 1, 2022 15:54
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. Traffic Ops related to Traffic Ops Traffic Portal v1 related to Traffic Portal version 1

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants