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

Topologies support for assigning ORG servers to Delivery Services #5164

Merged
merged 11 commits into from
Oct 19, 2020

Conversation

zrhoffman
Copy link
Member

What does this PR (Pull Request) do?

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?

  • Run the Traffic Ops API tests
  • Run the Traffic Portal UI tests
  • Try to assign an Origin server to a Topology-based Delivery Service in Traffic Portal
  • Make sure that Origin server is not included in GET /servers?dsId=# unless it is assigned

The following criteria are ALL met by this PR

zrhoffman and others added 4 commits October 15, 2020 14:14
…ry services (#38)

* adds the ability to assign ORG servers to topology-based ds's

* adds check for enabled 'assign org servers' button

* topologies need cache groups with servers in them now
@zrhoffman zrhoffman added Traffic Ops related to Traffic Ops Traffic Portal v1 related to Traffic Portal version 1 cdn-in-a-box related to the Docker-based CDN-in-a-Box system labels Oct 16, 2020
@ocket8888 ocket8888 added this to the 5.0.0 milestone Oct 16, 2020
@zrhoffman
Copy link
Member Author

Reran API tests and they passed.

@zrhoffman
Copy link
Member Author

API tests failed before due to #5165 which is not caused by this PR.

@mitchell852
Copy link
Member

mitchell852 commented Oct 18, 2020

Seeing this when calling GET /servers?dsId=X where X has a topology:

ERROR: api.go:209: 2020-10-18T17:14:43.163577-06:00: 127.0.0.1:57234 found more than one server with ID #1134

My dataset has only ONE server with ID=1134 fyi...

@zrhoffman
Copy link
Member Author

Seeing this when calling GET /servers?dsId=X where X has a topology:

ERROR: api.go:209: 2020-10-18T17:14:43.163577-06:00: 127.0.0.1:57234 found more than one server with ID #1134

My dataset has only ONE server with ID=1134 fyi...

A join was not specific enough, so delivery service servers could come from other delivery services. Fixed in 982bac1704

Copy link
Member

@mitchell852 mitchell852 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems to work the way I'd expect:

A bit confusing but isn't everything MSO-related? :P

@mitchell852 mitchell852 added the new feature A new feature, capability or behavior label Oct 19, 2020
@rawlinp rawlinp self-assigned this Oct 19, 2020
@rawlinp
Copy link
Contributor

rawlinp commented Oct 19, 2020

I can now assign ONLY ORG servers to the ds however, those org servers will not show up at https://tp.domain.tld/#!/delivery-services/{id}/servers?type={type} UNTIL the org cache group is added to the topology.

Indeed, I believe that is how it should work. If the ORG cachegroup isn't in the topology, we don't know how to parent up to the ORG servers.

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.

Works as designed, just one suggestion in the code

traffic_ops/traffic_ops_golang/server/servers.go Outdated Show resolved Hide resolved
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.

LGTM, will merge once the checks finish running

@zrhoffman
Copy link
Member Author

Looks like we hit #5165 again, rebased onto master since the fix is merged.

@rawlinp rawlinp merged commit 1be7d6d into apache:master Oct 19, 2020
@zrhoffman zrhoffman deleted the top-server-dsid-origin branch October 23, 2020 20:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cdn-in-a-box related to the Docker-based CDN-in-a-Box system new feature A new feature, capability or behavior Traffic Ops related to Traffic Ops Traffic Portal v1 related to Traffic Portal version 1
Projects
None yet
4 participants