Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Traffic Portal should filter out the "ALL" CDN option for snapshotting and queuing updates #6411

Closed
rawlinp opened this issue Dec 9, 2021 · 5 comments · Fixed by #6437
Closed
Labels
low difficulty the estimated level of effort to resolve this issue is low low impact affects only a small portion of a CDN, and cannot itself break one regression bug a bug in existing functionality introduced by a new version Traffic Portal v1 related to Traffic Portal version 1

Comments

@rawlinp
Copy link
Contributor

rawlinp commented Dec 9, 2021

This Improvement request (usability, performance, tech debt, etc.) affects these Traffic Control components:

  • Traffic Portal

Current behavior:

When snapshotting or queuing updates in Traffic Portal, the ALL CDN is listed as an option:
Screen Shot 2021-12-09 at 9 31 20 AM
Screen Shot 2021-12-09 at 9 31 30 AM

New behavior:

The ALL CDN should not be listed as an option in the snapshot and queue update modals, because it is never valid to snapshot or queue updates for the ALL CDN.

@rawlinp rawlinp added Traffic Portal v1 related to Traffic Portal version 1 low impact affects only a small portion of a CDN, and cannot itself break one improvement The functionality exists but it could be improved in some way. low difficulty the estimated level of effort to resolve this issue is low labels Dec 9, 2021
@mitchell852
Copy link
Member

that's odd. i don't recall it doing that...but indeed it does. feels like a regression bug.

@mitchell852
Copy link
Member

in addition. on the ALL cdn page (https://tp.domain.tld/#!/cdns/{{all_cdn_id}}), those options should not exist:

image

@ocket8888
Copy link
Contributor

If we really want to treat ALL specially, then we need to make that more formal. It should be immutable, it shouldn't be allowed to contain Delivery Services, etc. Currently, there are some places that treat ALL specially, and some that don't, and it can lead you to a partly correct configuration. It's also not documented anywhere that ALL is special and shouldn't be used for certain things.

IMO, we should actually just not treat ALL specially, and get rid of the notion of some special CDN that isn't really a CDN but sometimes is. If servers don't belong to a CDN, then they can just have null CDNs.

@mitchell852
Copy link
Member

mitchell852 commented Dec 17, 2021

IMO, we should actually just not treat ALL specially, and get rid of the notion of some special CDN that isn't really a CDN but sometimes is. If servers don't belong to a CDN, then they can just have null CDNs.

sure but that's a bigger effort and would require some thought on the plan. for now, i'll hide the ALL cdn from operations it was not designed to support as it was before. PR coming...this is a regression bug that was introduced when i added cdn notifications and decided to use the ALL cdn for that. i would definitely welcome the removal of the ALL cdn #3355

@mitchell852 mitchell852 added bug something isn't working as intended regression bug a bug in existing functionality introduced by a new version and removed improvement The functionality exists but it could be improved in some way. bug something isn't working as intended labels Dec 17, 2021
@rawlinp
Copy link
Contributor Author

rawlinp commented Dec 20, 2021

@ocket8888 I agree with your sentiments about the ALL CDN, but I do also think it's worth it to treat it specially in certain cases until such a time that we can separate cache servers from non-cache servers, especially since this does seem like a UI regression like @mitchell852 said.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
low difficulty the estimated level of effort to resolve this issue is low low impact affects only a small portion of a CDN, and cannot itself break one regression bug a bug in existing functionality introduced by a new version Traffic Portal v1 related to Traffic Portal version 1
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants