make pxe data include more relevant location data

Affects endpoints: GET /device/:id_or_serial/pxe GET /workspace/:id_or_name/pxe
joyent · Dec 11, 2019 · 9f1bafe · 9f1bafe
1 parent 39db637
commit 9f1bafe
Show file tree

Hide file tree

Showing 8 changed files with 50 additions and 231 deletions.
diff --git a/docs/json-schema/response.json b/docs/json-schema/response.json
@@ -1095,55 +1095,7 @@
               "type" : "null"
             },
             {
-              "additionalProperties" : false,
-              "properties" : {
-                "datacenter" : {
-                  "additionalProperties" : false,
-                  "properties" : {
-                    "name" : {
-                      "description" : "datacenter.region",
-                      "type" : "string"
-                    },
-                    "vendor_name" : {
-                      "description" : "the vendor's name for the datacenter",
-                      "oneOf" : [
-                        {
-                          "type" : "null"
-                        },
-                        {
-                          "type" : "string"
-                        }
-                      ]
-                    }
-                  },
-                  "required" : [
-                    "name",
-                    "vendor_name"
-                  ],
-                  "type" : "object"
-                },
-                "rack" : {
-                  "additionalProperties" : false,
-                  "properties" : {
-                    "name" : {
-                      "type" : "string"
-                    },
-                    "rack_unit_start" : {
-                      "$ref" : "common.json#/definitions/positive_integer"
-                    }
-                  },
-                  "required" : [
-                    "name",
-                    "rack_unit_start"
-                  ],
-                  "type" : "object"
-                }
-              },
-              "required" : [
-                "datacenter",
-                "rack"
-              ],
-              "type" : "object"
+              "$ref" : "/definitions/DeviceLocation"
             }
           ]
         },
@@ -2977,55 +2929,7 @@
             ]
           },
           "location" : {
-            "additionalProperties" : false,
-            "properties" : {
-              "datacenter" : {
-                "additionalProperties" : false,
-                "properties" : {
-                  "name" : {
-                    "description" : "datacenter.region",
-                    "type" : "string"
-                  },
-                  "vendor_name" : {
-                    "description" : "the vendor's name for the datacenter",
-                    "oneOf" : [
-                      {
-                        "type" : "null"
-                      },
-                      {
-                        "type" : "string"
-                      }
-                    ]
-                  }
-                },
-                "required" : [
-                  "name",
-                  "vendor_name"
-                ],
-                "type" : "object"
-              },
-              "rack" : {
-                "additionalProperties" : false,
-                "properties" : {
-                  "name" : {
-                    "type" : "string"
-                  },
-                  "rack_unit_start" : {
-                    "$ref" : "common.json#/definitions/positive_integer"
-                  }
-                },
-                "required" : [
-                  "name",
-                  "rack_unit_start"
-                ],
-                "type" : "object"
-              }
-            },
-            "required" : [
-              "datacenter",
-              "rack"
-            ],
-            "type" : "object"
+            "$ref" : "/definitions/DeviceLocation"
           },
           "phase" : {
             "$ref" : "common.json#/definitions/device_phase"

diff --git a/docs/modules/Conch::DB::ResultSet::Device.md b/docs/modules/Conch::DB::ResultSet::Device.md
@@ -73,7 +73,7 @@ Modifies the resultset to add the `sku` column.
 ## location\_data
 
 Returns a resultset that provides location data ([response.json#/definitions/DeviceLocation](../json-schema/response.json#/definitions/DeviceLocation)),
-optionally returned in a subhash using the provided key name.
+optionally returned under a hash using the provided key name.
 
 # LICENSING
 

diff --git a/json-schema/response.yaml b/json-schema/response.yaml
@@ -509,38 +509,7 @@ definitions:
       location:
         oneOf:
           - type: 'null'
-          - type: object
-            additionalProperties: false
-            required:
-              - datacenter
-              - rack
-            properties:
-              datacenter:
-                type: object
-                additionalProperties: false
-                required:
-                  - name
-                  - vendor_name
-                properties:
-                  name:
-                    description: datacenter.region
-                    type: string
-                  vendor_name:
-                    description: the vendor's name for the datacenter
-                    oneOf:
-                      - type: 'null'
-                      - type: string
-              rack:
-                type: object
-                additionalProperties: false
-                required:
-                  - name
-                  - rack_unit_start
-                properties:
-                  name:
-                    type: string
-                  rack_unit_start:
-                    $ref: common.yaml#/definitions/positive_integer
+          - $ref: /definitions/DeviceLocation
       ipmi:
         oneOf:
           - type: 'null'
@@ -622,38 +591,7 @@ definitions:
         phase:
           $ref: common.yaml#/definitions/device_phase
         location:
-          type: object
-          additionalProperties: false
-          required:
-            - datacenter
-            - rack
-          properties:
-            datacenter:
-              type: object
-              additionalProperties: false
-              required:
-                - name
-                - vendor_name
-              properties:
-                name:
-                  description: datacenter.region
-                  type: string
-                vendor_name:
-                  description: the vendor's name for the datacenter
-                  oneOf:
-                    - type: 'null'
-                    - type: string
-            rack:
-              type: object
-              additionalProperties: false
-              required:
-                - name
-                - rack_unit_start
-              properties:
-                name:
-                  type: string
-                rack_unit_start:
-                  $ref: common.yaml#/definitions/positive_integer
+          $ref: /definitions/DeviceLocation
         ipmi:
           oneOf:
             - type: 'null'

diff --git a/lib/Conch/Controller/Device.pm b/lib/Conch/Controller/Device.pm
@@ -266,29 +266,23 @@ Response uses the DevicePXE json schema.
 
 sub get_pxe ($c) {
     my $device_rs = $c->stash('device_rs');
-    my ($device) = $device_rs->search(
-        undef,
-        {
-            columns => {
-                id => 'device.id',
-                phase => 'device.phase',
-                'location.datacenter.name' => 'datacenter.region',
-                'location.datacenter.vendor_name' => 'datacenter.vendor_name',
-                'location.rack.name' => 'rack.name',
-                'location.rack.rack_unit_start' => 'device_location.rack_unit_start',
-                # pxe = the first (sorted by name) interface that is status=up
-                'pxe.mac' => $device_rs->correlate('device_nics')->nic_pxe->as_query,
-                # ipmi = the (newest) interface named ipmi1.
-                ipmi_mac_ip => $device_rs->correlate('device_nics')->nic_ipmi->as_query,
-            },
-            join => { device_location => { rack => { datacenter_room => 'datacenter' } } },
-        })
-        ->hri
-        ->all;
 
-    if (Conch::DB::Result::Device->phase_cmp($device->{phase}, 'production') >= 0) {
-        delete $device->{location};
-    }
+    $device_rs = $device_rs
+        ->location_data('location')
+        ->add_columns({
+            id => 'device.id',
+            phase => 'device.phase',
+            # pxe = the first (sorted by name) interface that is status=up
+            'pxe.mac' => $device_rs->correlate('device_nics')->nic_pxe->as_query,
+            # ipmi = the (newest) interface named ipmi1.
+            ipmi_mac_ip => $device_rs->correlate('device_nics')->nic_ipmi->as_query,
+        });
+
+    my ($device) = $device_rs->all;
+    undef $device->{location} if not $device->{location}{rack};
+
+    delete $device->{location}
+        if Conch::DB::Result::Device->phase_cmp($device->{phase}, 'production') >= 0;
 
     my $ipmi = delete $device->{ipmi_mac_ip};
     $device->{ipmi} = $ipmi ? { mac => $ipmi->[0], ip => $ipmi->[1] } : undef;

diff --git a/lib/Conch/Controller/WorkspaceDevice.pm b/lib/Conch/Controller/WorkspaceDevice.pm
@@ -73,38 +73,32 @@ sub get_pxe_devices ($c) {
     # production devices do not consider location, interface data to be canonical
     my $bad_phase = $c->req->query_params->param('phase_earlier_than') // 'production';
 
-    my @devices = $c->stash('workspace_rs')
+    my $rack_ids_rs = $c->stash('workspace_rs')
         ->related_resultset('workspace_racks')
-        ->search_related('rack', undef, { join => { datacenter_room => 'datacenter' } })
-        ->related_resultset('device_locations')
-        # production devices do not consider location data to be canonical
-        ->search_related('device',
+        ->get_column('rack_id')
+        ->as_query;
+
+    my @devices = $c->db_devices
+        ->search(
+            # production devices do not consider location data to be canonical
             $bad_phase ? { 'device.phase' => { '<' => \[ '?::device_phase_enum', $bad_phase ] } } : ())
-        ->columns({
+        ->location_data('location')
+        ->add_columns({
             id => 'device.id',
             phase => 'device.phase',
-            'location.datacenter.name' => 'datacenter.region',
-            'location.datacenter.vendor_name' => 'datacenter.vendor_name',
-            'location.rack.name' => 'rack.name',
-            'location.rack.rack_unit_start' => 'device_locations.rack_unit_start',
             # pxe = the first (sorted by name) interface that is status=up
             'pxe.mac' => $c->db_devices->correlate('device_nics')->nic_pxe->as_query,
             # ipmi = the (newest) interface named ipmi1.
             ipmi_mac_ip => $c->db_devices->correlate('device_nics')->nic_ipmi->as_query,
         })
+        ->search({ 'rack.id' => { -in => $rack_ids_rs } })
         ->order_by('device.created')
         ->hri
         ->all;
 
     foreach my $device (@devices) {
-        if (Conch::DB::Result::Device->phase_cmp($device->{phase}, 'production') >= 0) {
-            delete $device->{location};
-        }
-        else {
-            # DBIC collapse is inconsistent here with handling the lack of a datacenter_room->datacenter
-            $device->{location}{datacenter} = undef
-                if $device->{location} and not defined $device->{location}{datacenter}{name};
-        }
+        delete $device->{location}
+            if Conch::DB::Result::Device->phase_cmp($device->{phase}, 'production') >= 0;
 
         my $ipmi = delete $device->{ipmi_mac_ip};
         $device->{ipmi} = $ipmi ? { mac => $ipmi->[0], ip => $ipmi->[1] } : undef;

diff --git a/lib/Conch/DB/ResultSet/Device.pm b/lib/Conch/DB/ResultSet/Device.pm
@@ -214,7 +214,7 @@ sub with_sku ($self) {
 =head2 location_data
 
 Returns a resultset that provides location data (F<response.yaml#/definitions/DeviceLocation>),
-optionally returned in a subhash using the provided key name.
+optionally returned under a hash using the provided key name.
 
 =cut
 

diff --git a/t/integration/crud/devices.t b/t/integration/crud/devices.t
@@ -1088,18 +1088,15 @@ subtest 'Device PXE' => sub {
     $t_ro->get_ok('/device/PXE_TEST/pxe')
         ->status_is(200)
         ->json_schema_is('DevicePXE')
-        ->json_is({
+        ->json_cmp_deeply({
             id => $device_pxe->id,
             phase => 'integration',
             location => {
-                datacenter => {
-                    name => $datacenter->region,
-                    vendor_name => $datacenter->vendor_name,
-                },
-                rack => {
-                    name => $layout->rack->name,
-                    rack_unit_start => $layout->rack_unit_start,
-                },
+                az => 'room-0a',
+                datacenter_room => 'room 0a',
+                rack => 'ROOM:0.A:rack.0a',
+                rack_unit_start => 3,
+                target_hardware_product => superhashof({ alias => $layout->hardware_product->alias }),
             },
             ipmi => {
                 mac => '00:00:00:00:00:cc',

diff --git a/t/integration/crud/workspace-devices.t b/t/integration/crud/workspace-devices.t
@@ -176,8 +176,6 @@ subtest 'Devices with PXE data' => sub {
         },
     );
 
-    my $datacenter = $t->load_fixture('datacenter_0');
-
     $t->get_ok('/workspace/'.$global_ws_id.'/device/pxe')
         ->status_is(200)
         ->json_schema_is('WorkspaceDevicePXEs')
@@ -186,14 +184,11 @@ subtest 'Devices with PXE data' => sub {
                 id => $devices[0]->id,
                 phase => 'integration',
                 location => {
-                    datacenter => {
-                        name => $datacenter->region,
-                        vendor_name => $datacenter->vendor_name,
-                    },
-                    rack => {
-                        name => $layouts[0]->rack->name,
-                        rack_unit_start => $layouts[0]->rack_unit_start,
-                    },
+                    az => 'room-0a',
+                    datacenter_room => 'room 0a',
+                    rack => 'ROOM:0.A:rack.0a',
+                    rack_unit_start => $layouts[0]->rack_unit_start,
+                    target_hardware_product => superhashof({ alias => $layouts[0]->hardware_product->alias }),
                 },
                 ipmi => {
                     mac => '00:00:00:00:00:cc',
@@ -207,14 +202,11 @@ subtest 'Devices with PXE data' => sub {
                 id => $devices[1]->id,
                 phase => 'integration',
                 location => {
-                    datacenter => {
-                        name => $datacenter->region,
-                        vendor_name => $datacenter->vendor_name,
-                    },
-                    rack => {
-                        name => $layouts[1]->rack->name,
-                        rack_unit_start => $layouts[1]->rack_unit_start,
-                    },
+                    az => 'room-0a',
+                    datacenter_room => 'room 0a',
+                    rack => 'ROOM:0.A:rack.0a',
+                    rack_unit_start => $layouts[1]->rack_unit_start,
+                    target_hardware_product => superhashof({ alias => $layouts[1]->hardware_product->alias }),
                 },
                 ipmi => undef,
                 pxe => undef,