ocket8888 commented on a change in pull request #5450:
URL: https://github.com/apache/trafficcontrol/pull/5450#discussion_r563971707



##########
File path: docs/source/api/v3/about.rst
##########
@@ -13,7 +13,7 @@
 .. limitations under the License.
 ..
 
-.. _to-api-v3-about:
+.. _to-api-v3-v3-about:

Review comment:
       This one apparently already had a `-v3-` segment, so now it's got two

##########
File path: docs/source/api/v3/ping.rst
##########
@@ -13,7 +13,7 @@
 .. limitations under the License.
 ..
 
-.. _to-api-v3-ping:
+.. _to-api-v3-v3-ping:

Review comment:
       Same as above RE: two`v3` segments

##########
File path: docs/source/api/v3/federations_all.rst
##########
@@ -13,7 +13,7 @@
 .. limitations under the License.
 ..
 
-.. _to-api-v3-federations-all:
+.. _to-api-v3-v3-federations-all:

Review comment:
       Same as above RE: two`v3` segments

##########
File path: docs/source/api/v3/servers.rst
##########
@@ -39,7 +39,7 @@ Request Structure
        | cachegroupName | no       | Return only those servers within the 
:term:`Cache Group` that has this :ref:`cache-group-name`                    |
        
+----------------+----------+-------------------------------------------------------------------------------------------------------------------+
        | dsId           | no       | Return only those servers assigned to the 
:term:`Delivery Service` identified by this integral, unique identifier.|
-       |                |          | If the Delivery Service has a 
:term:`Topology` assigned to it, the :ref:`to-api-servers` endpoint will return 
    |
+       |                |          | If the Delivery Service has a 
:term:`Topology` assigned to it, the :ref:`to-api-v3-servers` endpoint will 
return     |

Review comment:
       Malformed table

##########
File path: docs/source/api/v3/steering.rst
##########
@@ -13,7 +13,7 @@
 .. limitations under the License.
 ..
 
-.. _to-api-v3-steering:
+.. _to-api-v3-v3-steering:

Review comment:
       Same as above RE: two`v3` segments

##########
File path: docs/source/api/v3/deliveryservice_requests_id_assign.rst
##########
@@ -13,7 +13,7 @@
 .. limitations under the License.
 ..
 
-.. _to-api-v3-deliveryservice_requests-id-assign:
+.. _to-api-v3-v3-deliveryservice_requests-id-assign:

Review comment:
       Same as above RE: two`v3` segments

##########
File path: docs/source/api/v3/cdns_dnsseckeys_generate.rst
##########
@@ -13,7 +13,7 @@
 .. limitations under the License.
 ..
 
-.. _to-api-v3-cdns-dnsseckeys-generate:
+.. _to-api-v3-v3-cdns-dnsseckeys-generate:

Review comment:
       same as above RE: double-segment.

##########
File path: docs/source/api/v3/deliveryservice_requests_id_status.rst
##########
@@ -13,7 +13,7 @@
 .. limitations under the License.
 ..
 
-.. _to-api-v3-deliveryservice_requests-id-status:
+.. _to-api-v3-v3-deliveryservice_requests-id-status:

Review comment:
       Same as above RE: two`v3` segments

##########
File path: lib/go-tc/servers.go
##########
@@ -63,12 +72,22 @@ type ServersV1Response struct {
 type ServerDetailV11 struct {
        ServerDetail
        LegacyInterfaceDetails
+       RouterHostName *string `json:"routerHostName" db:"router_host_name"`
+       RouterPortName *string `json:"routerPortName" db:"router_port_name"`
 }
 
 // ServerDetailV30 is the details for a server for API v3
 type ServerDetailV30 struct {
        ServerDetail
        ServerInterfaces *[]ServerInterfaceInfo `json:"interfaces"`
+       RouterHostName   *string                `json:"routerHostName" 
db:"router_host_name"`
+       RouterPortName   *string                `json:"routerPortName" 
db:"router_port_name"`
+}
+
+// ServerDetailV40 is the details for a server for API v4
+type ServerDetailV40 struct {
+       ServerDetail
+       ServerInterfaces *[]ServerInterfaceInfoV40 `json:"interfaces"`

Review comment:
       Does this need to be a pointer? Slices are already reference types so 
you can already do
   ```go
   type Foo {
       Slice []int
   }
   
   a := Foo{Slice: nil}
   ```

##########
File path: lib/go-tc/servers.go
##########
@@ -99,6 +124,13 @@ type ServerInterfaceInfo struct {
        Name         string            `json:"name" db:"name"`
 }
 
+// ServerInterfaceInfoV40 is the data associated with a V40 server's interface.
+type ServerInterfaceInfoV40 struct {
+       ServerInterfaceInfo
+       RouterHostName string `json:"routerHostName" db:"router_host_name"`
+       RouterPortName string `json:"routerPortName" db:"router_port_name"`

Review comment:
       The database and the existing API implementation for these server fields 
allows them to be null - if they're still allowed to be `null` then these need 
to be pointers, otherwise we need to enforce that these can't be null in the 
database, because that could cause TO to return data that can't be used in 
subsequent requests.
   
   Honestly I don't really care which, just make sure existing data is 
`COALESCE`'d if you go the db constraint route

##########
File path: 
traffic_portal/app/src/common/modules/table/servers/TableServersController.js
##########
@@ -220,7 +220,7 @@ var TableServersController = function(tableName, servers, 
filter, $scope, $state
                },
                {
                        headerName: "Router Port Name",
-                       field: "routerPortName",
+                       field: "routerPort",

Review comment:
       This field won't work anymore, since there's no field on the input data 
named `routerPort` (or `routerPortName`). I think we may just have to remove it.

##########
File path: docs/source/api/v4/servers.rst
##########
@@ -0,0 +1,522 @@
+..
+..
+.. Licensed under the Apache License, Version 2.0 (the "License");
+.. you may not use this file except in compliance with the License.
+.. You may obtain a copy of the License at
+..
+..     http://www.apache.org/licenses/LICENSE-2.0
+..
+.. Unless required by applicable law or agreed to in writing, software
+.. distributed under the License is distributed on an "AS IS" BASIS,
+.. WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+.. See the License for the specific language governing permissions and
+.. limitations under the License.
+..
+
+.. _to-api-servers:
+
+***********
+``servers``
+***********
+
+``GET``
+=======
+Retrieves properties of all servers across all CDNs.
+
+:Auth. Required: Yes
+:Roles Required: None
+:Response Type:  Array
+
+Request Structure
+-----------------
+.. table:: Request Query Parameters
+
+       
+----------------+----------+-------------------------------------------------------------------------------------------------------------------+
+       | Name           | Required | Description                               
                                                                        |
+       
+================+==========+===================================================================================================================+
+       | cachegroup     | no       | Return only those servers within the 
:term:`Cache Group` that has this :ref:`cache-group-id`                      |
+       
+----------------+----------+-------------------------------------------------------------------------------------------------------------------+
+       | cachegroupName | no       | Return only those servers within the 
:term:`Cache Group` that has this :ref:`cache-group-name`                    |
+       
+----------------+----------+-------------------------------------------------------------------------------------------------------------------+
+       | dsId           | no       | Return only those servers assigned to the 
:term:`Delivery Service` identified by this integral, unique identifier.|
+       |                |          | If the Delivery Service has a 
:term:`Topology` assigned to it, the :ref:`to-api-servers` endpoint will return 
    |
+       |                |          | each server whose :term:`Cache Group` is 
associated with a :term:`Topology Node` of that Topology and has the     |
+       |                |          | :term:`Server Capabilities` that are      
                                                                        |
+       |                |          | :term:`required by the Delivery Service 
<Delivery Service required capabilities>` but excluding                   |
+       |                |          | :term:`Origin Servers` that are not 
assigned to the Delivery Service. For more information, see                   |
+       |                |          | :ref:`multi-site-origin-qht`.             
                                                                        |
+       
+----------------+----------+-------------------------------------------------------------------------------------------------------------------+
+       | hostName       | no       | Return only those servers that have this 
(short) hostname                                                         |
+       
+----------------+----------+-------------------------------------------------------------------------------------------------------------------+
+       | id             | no       | Return only the server with this 
integral, unique identifier                                                     
 |
+       
+----------------+----------+-------------------------------------------------------------------------------------------------------------------+
+       | profileId      | no       | Return only those servers that are using 
the :term:`Profile` that has this :ref:`profile-id`                      |
+       
+----------------+----------+-------------------------------------------------------------------------------------------------------------------+
+       | status         | no       | Return only those servers with this 
status - see :ref:`health-proto`                                              |
+       
+----------------+----------+-------------------------------------------------------------------------------------------------------------------+
+       | type           | no       | Return only servers of this :term:`Type`  
                                                                        |
+       
+----------------+----------+-------------------------------------------------------------------------------------------------------------------+
+       | topology       | no       | Return only servers who belong to 
cachegroups assigned to the :term:`Topology` identified by this name            
|
+       
+----------------+----------+-------------------------------------------------------------------------------------------------------------------+
+       | sortOrder      | no       | Changes the order of sorting. Either 
ascending (default or "asc") or descending ("desc")                          |
+       
+----------------+----------+-------------------------------------------------------------------------------------------------------------------+
+       | limit          | no       | Choose the maximum number of results to 
return                                                                    |
+       
+----------------+----------+-------------------------------------------------------------------------------------------------------------------+
+       | offset         | no       | The number of results to skip before 
beginning to return results. Must use in conjunction with limit              |
+       
+----------------+----------+-------------------------------------------------------------------------------------------------------------------+
+       | page           | no       | Return the n\ :sup:`th` page of results, 
where "n" is the value of this parameter, pages are ``limit`` long and   |
+       |                |          | the first page is 1. If ``offset`` was 
defined, this query parameter has no effect. ``limit`` must be defined to  |
+       |                |          | make use of ``page``.                     
                                                                        |
+       
+----------------+----------+-------------------------------------------------------------------------------------------------------------------+
+
+.. code-block:: http
+       :caption: Request Example
+
+       GET /api/4.0/servers?hostName=mid HTTP/1.1
+       Host: trafficops.infra.ciab.test
+       User-Agent: curl/7.47.0
+       Accept: */*
+       Cookie: mojolicious=...
+
+Response Structure
+------------------
+:cachegroup:   A string that is the :ref:`name of the Cache Group 
<cache-group-name>` to which the server belongs
+:cachegroupId: An integer that is the :ref:`ID of the Cache Group 
<cache-group-id>` to which the server belongs
+:cdnId:        The integral, unique identifier of the CDN to which the server 
belongs
+:cdnName:      Name of the CDN to which the server belongs
+:domainName:   The domain part of the server's :abbr:`FQDN (Fully Qualified 
Domain Name)`
+:guid:         An identifier used to uniquely identify the server
+
+       .. note:: This is a legacy key which only still exists for 
compatibility reasons - it should always be ``null``
+
+:hostName:     The (short) hostname of the server
+:httpsPort:    The port on which the server listens for incoming HTTPS 
connections/requests
+:id:           An integral, unique identifier for this server
+:iloIpAddress: The IPv4 address of the server's :abbr:`ILO (Integrated 
Lights-Out)` service\ [#ilo]_
+:iloIpGateway: The IPv4 gateway address of the server's :abbr:`ILO (Integrated 
Lights-Out)` service\ [#ilo]_
+:iloIpNetmask: The IPv4 subnet mask of the server's :abbr:`ILO (Integrated 
Lights-Out)` service\ [#ilo]_
+:iloPassword:  The password of the of the server's :abbr:`ILO (Integrated 
Lights-Out)` service user\ [#ilo]_ - displays as simply ``******`` if the 
currently logged-in user does not have the 'admin' or 'operations' 
:term:`Role(s) <Role>`
+:iloUsername:  The user name for the server's :abbr:`ILO (Integrated 
Lights-Out)` service\ [#ilo]_
+:interfaces:   A set of the network interfaces in use by the server. In most 
scenarios, only one will be present, but it is illegal for this set to be an 
empty collection.
+
+       :ipAddresses:       A set of objects representing IP Addresses assigned 
to this network interface. In most scenarios, only one or two (usually one IPv4 
address and one IPv6 address) will be present, but it is illegal for this set 
to be an empty collection.
+
+               :address:        The actual IP address, including any mask as a 
CIDR-notation suffix
+               :gateway:        Either the IP address of the network gateway 
for this address, or ``null`` to signify that no such gateway exists
+               :serviceAddress: A boolean that describes whether or not the 
server's main service is available at this IP address. When this property is 
``true``, the IP address is referred to as a "service address". It is illegal 
for a server to not have at least one service address. It is also illegal for a 
server to have more than one service address of the same address family (i.e. 
more than one IPv4 service address and/or more than one IPv6 address). Finally, 
all service addresses for a server must be contained within one interface - 
which is therefore sometimes referred to as the "service interface" for the 
server.
+
+       :maxBandwidth:      The maximum healthy bandwidth allowed for this 
interface. If bandwidth exceeds this limit, Traffic Monitors will consider the 
entire server unhealthy - which includes *all* configured network interfaces. 
If this is ``null``, it has the meaning "no limit". It has no effect if 
``monitor`` is not true for this interface.
+
+               .. seealso:: :ref:`health-proto`
+
+       :monitor:           A boolean which describes whether or not this 
interface should be monitored by Traffic Monitor for statistics and health 
consideration.
+       :mtu:               The :abbr:`MTU (Maximum Transmission Unit)` of this 
interface. If it is ``null``, it may be assumed that the information is either 
not available or not applicable for this interface.
+       :name:              The name of the interface. No two interfaces of the 
same server may share a name. It is the same as the network interface's device 
name on the server, e.g. ``eth0``.
+    :routerHostName:    The human-readable name of the router responsible for 
reaching this server's interface.
+    :routerPort:        The human-readable name of the port used by the router 
responsible for reaching this server's interface.

Review comment:
       This is still indented with spaces instead of tab characters.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[email protected]


Reply via email to