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.

Sign up for GitHub

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

Jump to bottom

⚠️ Use ironic inventory API instead of calling ironic-inspector #1355

Merged
merged 6 commits into from
Sep 28, 2023
Merged

⚠️ Use ironic inventory API instead of calling ironic-inspector #1355

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
e03450f
Update gophercloud to 2.0 development branch
dtantsur Sep 13, 2023
ee69555
Use ironic inventory API instead of calling ironic-inspector
dtantsur Sep 13, 2023
79ea7c9
Remove any traces of inspector access
dtantsur Sep 18, 2023
df0e91b
Backward compatibility for get-hardware-details
dtantsur Sep 20, 2023
aa31c74
Update the required API version
dtantsur Sep 28, 2023
d8b09e7
Explain the compatibility logic in get-hardware-details
dtantsur Sep 28, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
15 changes: 8 additions & 7 deletions cmd/get-hardware-details/main.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,8 @@ import (
"os"
"strings"

"github.com/gophercloud/gophercloud/openstack/baremetalintrospection/v1/introspection"
"github.com/gophercloud/gophercloud/openstack/baremetal/v1/nodes"
"k8s.io/klog/v2"

"github.com/metal3-io/baremetal-operator/pkg/provisioner/ironic/clients"
"github.com/metal3-io/baremetal-operator/pkg/provisioner/ironic/hardwaredetails"
Expand All @@ -34,20 +35,20 @@ func main() {
InsecureSkipVerify: ironicInsecure,
}

inspector, err := clients.InspectorClient(opts.Endpoint, opts.AuthConfig, tlsConf)
ironic, err := clients.IronicClient(opts.Endpoint, opts.AuthConfig, tlsConf)
if err != nil {
fmt.Printf("could not get inspector client: %s", err)
fmt.Printf("could not get ironic client: %s", err)
os.Exit(1)
}

introData := introspection.GetIntrospectionData(inspector, opts.NodeID)
introData := nodes.GetInventory(ironic, opts.NodeID)
data, err := introData.Extract()
if err != nil {
fmt.Printf("could not get introspection data: %s", err)
fmt.Printf("could not get inspection data: %s", err)
os.Exit(1)
}

json, err := json.MarshalIndent(hardwaredetails.GetHardwareDetails(data), "", "\t")
json, err := json.MarshalIndent(hardwaredetails.GetHardwareDetails(data, klog.NewKlogr()), "", "\t")
if err != nil {
fmt.Printf("could not convert introspection data: %s", err)
os.Exit(1)
Expand All @@ -58,7 +59,7 @@ func main() {

func getOptions() (o options) {
if len(os.Args) != 3 {
fmt.Println("Usage: get-hardware-details <inspector URI> <node UUID>")
fmt.Println("Usage: get-hardware-details <ironic URI> <node UUID>")
dtantsur marked this conversation as resolved.
Show resolved Hide resolved
os.Exit(1)
}

Expand Down
4 changes: 2 additions & 2 deletions go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ go 1.20
require (
github.com/go-logr/logr v1.2.4
github.com/google/safetext v0.0.0-20230106111101-7156a760e523
github.com/gophercloud/gophercloud v1.6.0
github.com/gophercloud/gophercloud v1.5.1-0.20230912084133-c79ed6d3b371
Copy link
Member

Choose a reason for hiding this comment

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

I'm a bit uncomfortable with this. Is the goal to merge it with the development commit or wait for gophercloud v2 to be released?

Copy link
Member Author

Choose a reason for hiding this comment

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

V2 may take more time to get releases. What's the cause of your discomfort? For better or worse, using git commits is a normal thing in the Go world. The last time I tried to hurry gophercloud folks about a release they went "wtf, just use a git hash".

Copy link
Member

Choose a reason for hiding this comment

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

Would this be landing in v2 only? Do you have any guess when would that be? AFAIK when we need to test commits that has landed in dev branches but not ready for release yet, we use pseudo-versions with git hashes. Use of pseudo version signals module is still under development and not stable. But since go has now stricter rules now to generate pseudo-numbers, it can be used incase we cannot wait for proper release to exist.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yep, this is going to v2 only because this work involved breaking changes. I don't know when that happens, we can ask on slack.

I think I'm using what you're describing here. Since the last release before splitting the 1.* branch was 1.5.0, it assumes the new version will 1.5.1-something (which is wrong, but go tell go!)

Copy link
Member

Choose a reason for hiding this comment

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

What's the cause of your discomfort?

I'm mainly worried about the upgrade path and the potential for API changes that would impact us. It is probably not an issue but I would not want to just skip over it without thinking it through. So how do we expect the upgrade will look like?

We start with a scenario where a user is running an older version of BMO/Ironic relying on the older API and gophercloud. What would be required to upgrade? My understanding is something like this:

  1. Upgrade Ironic to a version that supports microversion 1.81. (Is something else needed here to "refresh" the inventory, I guess not?)
  2. Upgrade BMO.

This seems easy enough, but definitely worth a mention in the release notes for BMO.

Now my worry is that there would be some migration steps needed to go from gophercloud v1 to v2 and we end up somewhere in the middle. Do you see any situation where some kind of patch would be needed to support the bump to v2? (I know of projects where bugs have made it impossible to upgrade from e.g. 0.3.1 to 0.4.0. Instead users were required to first upgrade to the latest patch release, e.g. 0.3.4 and from there on to 0.4.0.) For example, could there be a required upgrade path for gophercloud to do v1.7.1 -> v2.0.0 and we miss it because we do not have the v1.7.1 patch?

Copy link
Member Author

Choose a reason for hiding this comment

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

Hmm, you seem to be conflating the Ironic upgrade and the Gophercloud upgrade.

Gophercloud is just an HTTP library. 2.0.0 will see some Go API changes, but these are easy to address on any bump. For better or worse, Go does not update anything automatically (we will need to keep an eye on dependabot, of course). This is why I'm not sure what you mean by "we don't have the v1.7.1 patch". Gophercloud has no local state.

Ironic upgrade is a bit different. Historically, we have taken bumping microversions very lightly. Now Steven has raised a valid point that some of our consumers may not always use the latest-and-greatest versions. Fair. We need to start having some sort of upgrade docs per release (which I assume don't exist now).

In any case, what are the suggested action items here?

Copy link
Member

Choose a reason for hiding this comment

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

Thanks for the explanation, that makes sense!
It seems like there is not much reason to worry. We just need to document the Ironic version bump so users notice it.
Perhaps we can bring this up at the community meeting today also to make sure developers are aware of it, in case there are any concerns?

github.com/metal3-io/baremetal-operator/apis v0.2.0
github.com/metal3-io/baremetal-operator/pkg/hardwareutils v0.2.0
github.com/onsi/gomega v1.27.10
Expand Down Expand Up @@ -62,7 +62,7 @@ require (
golang.org/x/exp v0.0.0-20230522175609-2e198f4a06a1 // indirect
golang.org/x/net v0.14.0 // indirect
golang.org/x/oauth2 v0.5.0 // indirect
golang.org/x/sys v0.11.0 // indirect
golang.org/x/sys v0.12.0 // indirect
golang.org/x/term v0.11.0 // indirect
golang.org/x/text v0.12.0 // indirect
golang.org/x/time v0.3.0 // indirect
Expand Down
13 changes: 4 additions & 9 deletions go.sum
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,8 +72,8 @@ github.com/google/safetext v0.0.0-20230106111101-7156a760e523 h1:i4NsbmB9pD5+Ggp
github.com/google/safetext v0.0.0-20230106111101-7156a760e523/go.mod h1:mJNEy0r5YPHC7ChQffpOszlGB4L1iqjXWpIEKcFpr9s=
github.com/google/uuid v1.3.1 h1:KjJaJ9iWZ3jOFZIf1Lqf4laDRCasjl0BCmnEGxkdLb4=
github.com/google/uuid v1.3.1/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo=
github.com/gophercloud/gophercloud v1.6.0 h1:JwJN1bauRnWPba5ueWs9IluONHteXPWjjK+MvfM4krY=
github.com/gophercloud/gophercloud v1.6.0/go.mod h1:aAVqcocTSXh2vYFZ1JTvx4EQmfgzxRcNupUfxZbBNDM=
github.com/gophercloud/gophercloud v1.5.1-0.20230912084133-c79ed6d3b371 h1:/4TDrmUM3o52/9J3Pn4mZ3VP7CYiFkDKRNuWyxLkpp4=
github.com/gophercloud/gophercloud v1.5.1-0.20230912084133-c79ed6d3b371/go.mod h1:vYRGjRkTeMAwdvQFTVuPGfRWv4dc3J2QHy9H3bHD6Pc=
github.com/imdario/mergo v0.3.12 h1:b6R2BslTbIEToALKP7LxUvijTsNI9TAe80pLWN2g/HU=
github.com/imdario/mergo v0.3.12/go.mod h1:jmQim1M+e3UYxmgPu/WyfjB3N3VflVyUjjjwH0dnCYA=
github.com/inconshreveable/mousetrap v1.0.1 h1:U3uMjPSQEBMNp1lFxmllqCPM6P5u/Xq7Pgzkat/bFNc=
Expand Down Expand Up @@ -156,7 +156,6 @@ go.uber.org/zap v1.25.0/go.mod h1:JIAUzQIH94IC4fOJQm7gMmBJP5k7wQfdcnYdPoEXJYk=
golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w=
golang.org/x/crypto v0.0.0-20191011191535-87dc89f01550/go.mod h1:yigFU9vqHzYiE8UmvKecakEJjdnWj3jj499lnFckfCI=
golang.org/x/crypto v0.0.0-20200622213623-75b288015ac9/go.mod h1:LzIPMQfyMNhhGPhUkYOs5KpL4U8rLKemX1yGLhDgUto=
golang.org/x/crypto v0.0.0-20220829220503-c86fa9a7ed90/go.mod h1:IxCIyHEi3zRg3s0A5j5BB6A9Jmi73HwBIUl50j+osU4=
golang.org/x/exp v0.0.0-20190121172915-509febef88a4/go.mod h1:CJ0aWSM057203Lf6IL+f9T1iT9GByDxfZKAQTCR3kQA=
golang.org/x/exp v0.0.0-20230522175609-2e198f4a06a1 h1:k/i9J1pBpvlfR+9QsetwPyERsqu1GIbi967PQMq3Ivc=
golang.org/x/exp v0.0.0-20230522175609-2e198f4a06a1/go.mod h1:V1LtkGg67GoY2N1AnLN78QLrzxkLyJw7RJb1gzOOz9w=
Expand All @@ -177,7 +176,6 @@ golang.org/x/net v0.0.0-20190620200207-3b0461eec859/go.mod h1:z5CRVTTTmAJ677TzLL
golang.org/x/net v0.0.0-20200226121028-0de0cce0169b/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s=
golang.org/x/net v0.0.0-20201021035429-f5854403a974/go.mod h1:sp8m0HH+o8qH0wwXwYZr8TS3Oi6o0r6Gce1SSxlDquU=
golang.org/x/net v0.0.0-20210405180319-a5a99cb37ef4/go.mod h1:p54w0d4576C0XHj96bSt6lcn1PtDYWL6XObtHCRCNQM=
golang.org/x/net v0.0.0-20211112202133-69e39bad7dc2/go.mod h1:9nx3DQGgdP8bBQD5qxJ1jj9UTztislL4KSBs9R2vV5Y=
golang.org/x/net v0.14.0 h1:BONx9s002vGdD9umnlX1Po8vOZmrgH34qlHcD1MfK14=
golang.org/x/net v0.14.0/go.mod h1:PpSgVXXLK0OxS0F31C1/tv6XNguvCrnXIDrFMspZIUI=
golang.org/x/oauth2 v0.0.0-20180821212333-d2e6202438be/go.mod h1:N/0e6XlmueqKjAGxoOufVs8QHGRruUQn6yWY3a++T0U=
Expand All @@ -196,19 +194,16 @@ golang.org/x/sys v0.0.0-20190412213103-97732733099d/go.mod h1:h1NjWce9XRLGQEsW7w
golang.org/x/sys v0.0.0-20200930185726-fdedc70b468f/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
golang.org/x/sys v0.0.0-20201119102817-f84b799fce68/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
golang.org/x/sys v0.0.0-20210330210617-4fbd30eecc44/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
golang.org/x/sys v0.0.0-20210423082822-04245dca01da/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
golang.org/x/sys v0.0.0-20210510120138-977fb7262007/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
golang.org/x/sys v0.0.0-20210615035016-665e8c7367d1/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
golang.org/x/sys v0.0.0-20220908164124-27713097b956/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
golang.org/x/sys v0.11.0 h1:eG7RXZHdqOJ1i+0lgLgCpSXAp6M3LYlAo6osgSi0xOM=
golang.org/x/sys v0.11.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
golang.org/x/sys v0.12.0 h1:CM0HF96J0hcLAwsHPJZjfdNzs0gftsLfgKt57wWHJ0o=
golang.org/x/sys v0.12.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo=
golang.org/x/term v0.11.0 h1:F9tnn/DA/Im8nCwm+fX+1/eBwi4qFjRT++MhtVC4ZX0=
golang.org/x/term v0.11.0/go.mod h1:zC9APTIj3jG3FdV/Ons+XE1riIZXG4aZ4GTHiPZJPIU=
golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ=
golang.org/x/text v0.3.2/go.mod h1:bEr9sfX3Q8Zfm5fL9x+3itogRgK3+ptLWKqgva+5dAk=
golang.org/x/text v0.3.3/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ=
golang.org/x/text v0.3.6/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ=
golang.org/x/text v0.12.0 h1:k+n5B8goJNdU7hSvEtMUz3d1Q6D/XW4COJSJR6fN0mc=
golang.org/x/text v0.12.0/go.mod h1:TvPlkZtksWOMsz7fbANvkp4WM8x/WCo/om8BMLbz+aE=
golang.org/x/time v0.3.0 h1:rg5rLMjNzMS1RkNLzCG38eapWhnYLFYXDXj2gOlr8j4=
Expand Down
4 changes: 2 additions & 2 deletions pkg/provisioner/ironic/clients/client.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,8 +95,8 @@ func IronicClient(ironicEndpoint string, auth AuthConfig, tls TLSConfig) (client

// Ensure we have a microversion high enough to get the features
// we need. Update docs/configuration.md when updating the version.
// Version 1.74 allows retrival of the BIOS Registry
client.Microversion = "1.74"
// Version 1.81 allows retrival of Node inventory
client.Microversion = "1.81"
dtantsur marked this conversation as resolved.
Show resolved Hide resolved

return updateHTTPClient(client, tls)
}
Expand Down
26 changes: 17 additions & 9 deletions pkg/provisioner/ironic/hardwaredetails/hardwaredetails.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,18 +6,26 @@ import (
"sort"
"strings"

"github.com/go-logr/logr"
"github.com/gophercloud/gophercloud/openstack/baremetal/inventory"
"github.com/gophercloud/gophercloud/openstack/baremetal/v1/nodes"
"github.com/gophercloud/gophercloud/openstack/baremetalintrospection/v1/introspection"

metal3api "github.com/metal3-io/baremetal-operator/apis/metal3.io/v1alpha1"
)

// GetHardwareDetails converts Ironic introspection data into BareMetalHost HardwareDetails.
func GetHardwareDetails(data *introspection.Data) *metal3api.HardwareDetails {
func GetHardwareDetails(data *nodes.InventoryData, logger logr.Logger) *metal3api.HardwareDetails {
inspectorData, err := data.PluginData.AsInspectorData()
if err != nil {
logger.Error(err, "cannot get plugin data from inventory, some fields will not be available")
}

details := new(metal3api.HardwareDetails)
details.Firmware = getFirmwareDetails(data.Inventory.SystemVendor.Firmware)
details.SystemVendor = getSystemVendorDetails(data.Inventory.SystemVendor)
details.RAMMebibytes = data.MemoryMB
details.NIC = getNICDetails(data.Inventory.Interfaces, data.AllInterfaces)
details.RAMMebibytes = data.Inventory.Memory.PhysicalMb
details.NIC = getNICDetails(data.Inventory.Interfaces, inspectorData.AllInterfaces)
details.Storage = getStorageDetails(data.Inventory.Disks)
details.CPU = getCPUDetails(&data.Inventory.CPU)
details.Hostname = data.Inventory.Hostname
Expand Down Expand Up @@ -47,7 +55,7 @@ func getVLANs(intf introspection.BaseInterfaceType) (vlans []metal3api.VLAN, vla
return
}

func getNICDetails(ifdata []introspection.InterfaceType,
func getNICDetails(ifdata []inventory.InterfaceType,
basedata map[string]introspection.BaseInterfaceType) []metal3api.NIC {
var nics []metal3api.NIC
for _, intf := range ifdata {
Expand Down Expand Up @@ -85,7 +93,7 @@ func getNICDetails(ifdata []introspection.InterfaceType,
return nics
}

func getDiskType(diskdata introspection.RootDiskType) metal3api.DiskType {
func getDiskType(diskdata inventory.RootDiskType) metal3api.DiskType {
if diskdata.Rotational {
return metal3api.HDD
}
Expand All @@ -97,7 +105,7 @@ func getDiskType(diskdata introspection.RootDiskType) metal3api.DiskType {
return metal3api.SSD
}

func getStorageDetails(diskdata []introspection.RootDiskType) []metal3api.Storage {
func getStorageDetails(diskdata []inventory.RootDiskType) []metal3api.Storage {
storage := make([]metal3api.Storage, len(diskdata))
for i, disk := range diskdata {
device := disk.Name
Expand All @@ -121,15 +129,15 @@ func getStorageDetails(diskdata []introspection.RootDiskType) []metal3api.Storag
return storage
}

func getSystemVendorDetails(vendor introspection.SystemVendorType) metal3api.HardwareSystemVendor {
func getSystemVendorDetails(vendor inventory.SystemVendorType) metal3api.HardwareSystemVendor {
return metal3api.HardwareSystemVendor{
Manufacturer: vendor.Manufacturer,
ProductName: vendor.ProductName,
SerialNumber: vendor.SerialNumber,
}
}

func getCPUDetails(cpudata *introspection.CPUType) metal3api.CPU {
func getCPUDetails(cpudata *inventory.CPUType) metal3api.CPU {
var freq float64
fmt.Sscanf(cpudata.Frequency, "%f", &freq)
freq = math.Round(freq) // Ensure freq has no fractional part
Expand All @@ -145,7 +153,7 @@ func getCPUDetails(cpudata *introspection.CPUType) metal3api.CPU {
return cpu
}

func getFirmwareDetails(firmwaredata introspection.SystemFirmwareType) metal3api.Firmware {
func getFirmwareDetails(firmwaredata inventory.SystemFirmwareType) metal3api.Firmware {
return metal3api.Firmware{
BIOS: metal3api.BIOS{
Vendor: firmwaredata.Vendor,
Expand Down
5 changes: 3 additions & 2 deletions pkg/provisioner/ironic/hardwaredetails/hardwaredetails_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"reflect"
"testing"

"github.com/gophercloud/gophercloud/openstack/baremetal/inventory"
"github.com/gophercloud/gophercloud/openstack/baremetalintrospection/v1/introspection"

metal3api "github.com/metal3-io/baremetal-operator/apis/metal3.io/v1alpha1"
Expand Down Expand Up @@ -150,7 +151,7 @@ func TestGetVLANsMalformed(t *testing.T) {

func TestGetNICDetails(t *testing.T) {
nics := getNICDetails(
[]introspection.InterfaceType{
[]inventory.InterfaceType{
{
Name: "eth0",
IPV4Address: "192.0.2.1",
Expand Down Expand Up @@ -231,7 +232,7 @@ func TestGetNICDetails(t *testing.T) {

func TestGetFirmwareDetails(t *testing.T) {
// Test full (known) firmware payload
firmware := getFirmwareDetails(introspection.SystemFirmwareType{
firmware := getFirmwareDetails(inventory.SystemFirmwareType{
Vendor: "foobar",
Version: "1.2.3",
BuildDate: "2019-07-10",
Expand Down
30 changes: 12 additions & 18 deletions pkg/provisioner/ironic/inspecthardware_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@ import (
"testing"
"time"

"github.com/gophercloud/gophercloud/openstack/baremetal/inventory"
"github.com/gophercloud/gophercloud/openstack/baremetal/v1/nodes"
"github.com/gophercloud/gophercloud/openstack/baremetalintrospection/v1/introspection"
"github.com/stretchr/testify/assert"

metal3api "github.com/metal3-io/baremetal-operator/apis/metal3.io/v1alpha1"
Expand Down Expand Up @@ -54,8 +54,7 @@ func TestInspectHardware(t *testing.T) {
ironic: testserver.NewIronic(t).WithDefaultResponses().Node(nodes.Node{
UUID: nodeUUID,
ProvisionState: "manageable",
}),
inspector: testserver.NewInspector(t).Ready().WithIntrospectionDataFailed(nodeUUID, http.StatusNotFound),
}).WithInventoryFailed(nodeUUID, http.StatusNotFound),

expectedStarted: true,
expectedDirty: true,
Expand All @@ -67,13 +66,11 @@ func TestInspectHardware(t *testing.T) {
ironic: testserver.NewIronic(t).WithDefaultResponses().Node(nodes.Node{
UUID: nodeUUID,
ProvisionState: "manageable",
}).WithInventory(nodeUUID, nodes.InventoryData{
Inventory: inventory.InventoryType{
Hostname: "node-0",
},
}),
inspector: testserver.NewInspector(t).Ready().
WithIntrospectionData(nodeUUID, introspection.Data{
Inventory: introspection.InventoryType{
Hostname: "node-0",
},
}),

refresh: true,

Expand All @@ -87,10 +84,9 @@ func TestInspectHardware(t *testing.T) {
ironic: testserver.NewIronic(t).WithDefaultResponses().Node(nodes.Node{
UUID: nodeUUID,
ProvisionState: "manageable",
}),
inspector: testserver.NewInspector(t).Ready().WithIntrospectionDataFailed(nodeUUID, http.StatusBadRequest),
}).WithInventoryFailed(nodeUUID, http.StatusBadRequest),

expectedError: "failed to retrieve hardware introspection data: Bad request with: \\[GET http://127.0.0.1:.*/v1/introspection/33ce8659-7400-4c68-9535-d10766f07a58/data\\], error message: An error\\\n",
expectedError: "failed to retrieve hardware introspection data: Bad request with: \\[GET http://127.0.0.1:.*/v1/nodes/33ce8659-7400-4c68-9535-d10766f07a58/inventory\\], error message: An error\\\n",
},
{
name: "introspection-status-retry-on-wait",
Expand Down Expand Up @@ -192,13 +188,11 @@ func TestInspectHardware(t *testing.T) {
ironic: testserver.NewIronic(t).Ready().Node(nodes.Node{
UUID: nodeUUID,
ProvisionState: string(nodes.Manageable),
}).WithInventory(nodeUUID, nodes.InventoryData{
Inventory: inventory.InventoryType{
Hostname: "node-0",
},
}),
inspector: testserver.NewInspector(t).Ready().
WithIntrospectionData(nodeUUID, introspection.Data{
Inventory: introspection.InventoryType{
Hostname: "node-0",
},
}),

expectedDirty: false,
expectedDetailsHost: "node-0",
Expand Down
7 changes: 3 additions & 4 deletions pkg/provisioner/ironic/ironic.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ import (
"github.com/gophercloud/gophercloud"
"github.com/gophercloud/gophercloud/openstack/baremetal/v1/nodes"
"github.com/gophercloud/gophercloud/openstack/baremetal/v1/ports"
"github.com/gophercloud/gophercloud/openstack/baremetalintrospection/v1/introspection"

metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/util/intstr"
Expand Down Expand Up @@ -844,8 +843,8 @@ func (p *ironicProvisioner) InspectHardware(data provisioner.InspectData, restar
return
}

// TODO(dtantsur): change this to use Ironic native inspection data API.
response := introspection.GetIntrospectionData(p.inspector, ironicNode.UUID)
p.log.Info("getting hardware details from inspection")
response := nodes.GetInventory(p.client, ironicNode.UUID)
introData, err := response.Extract()
if err != nil {
if _, isNotFound := err.(gophercloud.ErrDefault404); isNotFound {
Expand All @@ -860,7 +859,7 @@ func (p *ironicProvisioner) InspectHardware(data provisioner.InspectData, restar
// Introspection is done
p.log.Info("inspection finished successfully", "data", response.Body)

details = hardwaredetails.GetHardwareDetails(introData)
details = hardwaredetails.GetHardwareDetails(introData, p.log)
p.publisher("InspectionComplete", "Hardware inspection completed")
result, err = operationComplete()
return
Expand Down
18 changes: 0 additions & 18 deletions pkg/provisioner/ironic/testserver/inspector.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,21 +44,3 @@ func (m *InspectorMock) WithIntrospection(nodeUUID string, status introspection.
m.ResponseJSON("/v1/introspection/"+nodeUUID, status)
return m
}

// WithIntrospectionFailed configures the server with an error response for /v1/introspection/<node>
func (m *InspectorMock) WithIntrospectionFailed(nodeUUID string, errorCode int) *InspectorMock {
m.ErrorResponse("/v1/introspection/"+nodeUUID, errorCode)
return m
}

// WithIntrospectionData configures the server with a valid response for /v1/introspection/<node>/data
func (m *InspectorMock) WithIntrospectionData(nodeUUID string, data introspection.Data) *InspectorMock {
m.ResponseJSON("/v1/introspection/"+nodeUUID+"/data", data)
return m
}

// WithIntrospectionDataFailed configures the server with an error response for /v1/introspection/<node>/data
func (m *InspectorMock) WithIntrospectionDataFailed(nodeUUID string, errorCode int) *InspectorMock {
m.ErrorResponse("/v1/introspection/"+nodeUUID+"/data", errorCode)
return m
}
12 changes: 12 additions & 0 deletions pkg/provisioner/ironic/testserver/ironic.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -446,3 +446,15 @@ func (m *IronicMock) BIOSDetailSettings(nodeUUID string) *IronicMock {
func (m *IronicMock) NoBIOS(nodeUUID string) *IronicMock {
return m.NodeError(nodeUUID, http.StatusNotFound)
}

// WithInventory configures the server with a valid response for /v1/nodes/<node>/inventory
func (m *IronicMock) WithInventory(nodeUUID string, data nodes.InventoryData) *IronicMock {
m.ResponseJSON("/v1/nodes/"+nodeUUID+"/inventory", data)
return m
}

// WithInventoryFailed configures the server with an error response for /v1/nodes/<node>/inventory
func (m *IronicMock) WithInventoryFailed(nodeUUID string, errorCode int) *IronicMock {
m.ErrorResponse("/v1/nodes/"+nodeUUID+"/inventory", errorCode)
return m
}