Bug 1841556 - OCPRHV-104: SDK - go-ovirt: NICs not displaying correctly
Summary: OCPRHV-104: SDK - go-ovirt: NICs not displaying correctly
Keywords:
Status: CLOSED UPSTREAM
Alias: None
Product: Red Hat Enterprise Virtualization Manager
Classification: Red Hat
Component: distribution
Version: 4.3.11
Hardware: Unspecified
OS: Unspecified
unspecified
unspecified
Target Milestone: ovirt-4.3.11
: ---
Assignee: Roberto Ciatti
QA Contact: Lucie Leistnerova
URL:
Whiteboard:
Depends On:
Blocks: 1838559
TreeView+ depends on / blocked
 
Reported: 2020-05-29 13:06 UTC by Douglas Schilling Landgraf
Modified: 2020-07-17 07:12 UTC (History)
7 users (show)

Fixed In Version:
Doc Type: If docs needed, set a value
Doc Text:
Clone Of:
Environment:
Last Closed: 2020-06-08 15:21:14 UTC
oVirt Team: Integration
Target Upstream Version:
Embargoed:


Attachments (Terms of Use)


Links
System ID Private Priority Status Summary Last Updated
Github oVirt ovirt-engine-sdk-go pull 206 0 None closed Networks parsing problem 2020-07-17 07:02:47 UTC

Description Douglas Schilling Landgraf 2020-05-29 13:06:58 UTC
Description of problem:

Note:
------------
This is a summary provided from mmartinv/Olimp/Alejandro discussion from: https://bugzilla.redhat.com/show_bug.cgi?id=1838559
https://bugzilla.redhat.com/show_bug.cgi?id=1829153

Test Scenario
===========================

Engine versions tested:
----------------------------------
oVirt Engine: Software Version:4.3.9.4-1.el7
RHVM: Software Version:4.3.7.2-0.1.el7
RHVM: Software Version:4.3.9.4-1.el7


How I have created the nics?
--------------------------------
- Engine Administrator portal -> Network -> Networks -> New
             Name: nic${number}

  * I have repeated that for 15 times. 

- Engine Administrator portal -> Network -> Networks
        * On NIC 10 -> Edit -> Set nameserver to 8.8.8.8  (or multiple nameservers if you prefer)


Run the tool:
-----------------------
$ go get github.com/ovirt/go-ovirt
$ go run reproducer.go (bellow the code)

<snip>
2020/05/29 08:59:05 Parsed network list:
2020/05/29 08:59:05 nic1
2020/05/29 08:59:05 nic10            <---------- only two
    

Workaround
-----------------------------------
$ vi ~/go/src/github.com/ovirt/go-ovirt/readers.go 
<line>14234

-     case "name_servers":
+     case "name_server":


$  go build github.com/ovirt/go-ovirt
>>>>>>
2020/05/29 09:03:17 Parsed network list:
2020/05/29 09:03:17 nic1
2020/05/29 09:03:17 nic10
2020/05/29 09:03:17 nic11
2020/05/29 09:03:17 nic12
2020/05/29 09:03:17 nic13
2020/05/29 09:03:17 nic14
2020/05/29 09:03:17 nic15
2020/05/29 09:03:17 nic2
2020/05/29 09:03:17 nic3
2020/05/29 09:03:17 nic4
2020/05/29 09:03:17 nic5
2020/05/29 09:03:17 nic6
2020/05/29 09:03:17 nic7
2020/05/29 09:03:17 nic8
2020/05/29 09:03:17 nic9
2020/05/29 09:03:17 ovirtmgmt



// reproducer.go
package main

import (
    "log"
    "time"

    ovirtsdk4 "github.com/ovirt/go-ovirt"
)

func main() {
    inputRawURL := "https://engine.medogz.home/ovirt-engine/api"
    conn, err := ovirtsdk4.NewConnectionBuilder().
        URL(inputRawURL).
        Username("admin@internal").
        Password("mysuperpassword").
        Insecure(true).
        Compress(true).
        LogFunc(log.Printf).
        Timeout(time.Second * 10).
        Build()
    if err != nil {
        log.Printf("Make connection failed, reason: %v\n", err)
        return
    }
    defer conn.Close()

    defer func() {
        if err := recover(); err != nil {
            log.Printf("Panics occurs, try the non-Must methods to find the reason")
        }
    }()

    networksService := conn.SystemService().NetworksService()

    networksResponse, err := networksService.List().Send()
    if err != nil {
        log.Printf("Failed to get network list, reason: %v\n", err)
        return
    }

    log.Printf("Parsed network list:")
    if networks, ok := networksResponse.Networks(); ok {
        for _, network := range networks.Slice() {
            if networkName, ok := network.Name(); ok {
                log.Printf("%v\n", networkName)
            }
        }
    }
}

Comment 1 Roberto Ciatti 2020-05-29 14:35:03 UTC
Hi,
  working together with Douglas we discovered a problem in the 'go-ovirt' and probably at

https://github.com/oVirt/go-ovirt/blob/master/readers.go#L14234

where the reader is gonna parse the name_server section of the XML coming from the ovirt engine REST api. Probably worth to look at the 'ovirt-engine-sdk-go' ReadersGenerator.

Trying with another sdk (e.g.: the 'ovirt-engine-sdk-ruby') all the nics are reported correctly (using the code below)

require 'ovirtsdk4'

BASE_OPTS = {
  url: 'https://<engine-fqdn>/ovirt-engine/api',
  username: 'admin@internal',
  password: 'pass',
  timeout: 3600,
  insecure: true,
  connect_timeout: 60
}.freeze

conn = OvirtSDK4::Connection.new(BASE_OPTS)

sys_srv = conn.system_service
cls_srv = sys_srv.clusters_service
cls_srv.list.each do |c|
  puts "Cluster: #{c.name}, #{c.id}"
  cl_service = cls_srv.cluster_service(c.id)
  cl_net_srv = cl_service.networks_service
  nics = cl_net_srv.list
  puts nics.map(&:name)
end

Comment 2 Roberto Ciatti 2020-05-29 16:57:02 UTC
The problem in the ovirt-go readers.go parser 

https://github.com/oVirt/go-ovirt/blob/9bcc4fd4e6c0921d07d328036c73abbea369e023/readers.go#L14202

parsing the sub tag 'dns_resolver_configuration' the is completely compromising the tag hierarchy.

Need some modification to the ovirt-engine-sdk-go generator for this section

Comment 3 Douglas Schilling Landgraf 2020-05-31 12:18:04 UTC
Roberto is working in this bug, setting the Github WIP patch as attachment and setting owner.

Comment 4 Roberto Ciatti 2020-06-01 10:21:23 UTC
Proposed a solution to solve the problem and tests added to cover the case. 
All the other tests are passing too.

Comment 5 Sandro Bonazzola 2020-06-08 15:04:57 UTC
Current patch is merged on master, do we need it backported to 4.3? or SDK 4.4 will be ok?

Comment 6 Roberto Ciatti 2020-06-08 15:16:08 UTC
The problem is also on the sdk 4.3.
According to me, backporting the patch would allow clients that are using the 4.3 to work properly, but I'll leave the decision to the maintainer.

IMHO I'd close this BZ as a consequence of the upstream merge.

Comment 7 Douglas Schilling Landgraf 2020-06-08 15:21:14 UTC
As the reporter for this bug, I am okay to close this bug as upstream merged and verified the patch (with Robert's testing patch as well).


Note You need to log in before you can comment on or make changes to this bug.