From 9f1bafeab02fd610c5d04ac8a3c0d8f5166713e8 Mon Sep 17 00:00:00 2001 From: Karen Etheridge Date: Fri, 6 Dec 2019 12:07:20 -0800 Subject: [PATCH] make pxe data include more relevant location data Affects endpoints: GET /device/:id_or_serial/pxe GET /workspace/:id_or_name/pxe --- docs/json-schema/response.json | 100 +------------------ docs/modules/Conch::DB::ResultSet::Device.md | 2 +- json-schema/response.yaml | 66 +----------- lib/Conch/Controller/Device.pm | 38 +++---- lib/Conch/Controller/WorkspaceDevice.pm | 30 +++--- lib/Conch/DB/ResultSet/Device.pm | 2 +- t/integration/crud/devices.t | 15 ++- t/integration/crud/workspace-devices.t | 28 ++---- 8 files changed, 50 insertions(+), 231 deletions(-) diff --git a/docs/json-schema/response.json b/docs/json-schema/response.json index cda62f415..0194831f6 100644 --- 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 index bed2aa0db..c19d397fe 100644 --- 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 index ac0fb4ffe..1428fc987 100644 --- 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 index da5fc4087..74a56ae2c 100644 --- 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 index a1832281c..80d99d449 100644 --- 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 index d04fee0ce..e31c5775f 100644 --- 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), -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 index e43b73e1d..4cb0adb75 100644 --- 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 index 9f05e6a42..de3c9e45b 100644 --- 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,