Bug 1841556

Summary: OCPRHV-104: SDK - go-ovirt: NICs not displaying correctly
Product: Red Hat Enterprise Virtualization Manager Reporter: Douglas Schilling Landgraf <dougsland>
Component: distributionAssignee: Roberto Ciatti <rciatti>
Status: CLOSED UPSTREAM QA Contact: Lucie Leistnerova <lleistne>
Severity: unspecified Docs Contact:
Priority: unspecified    
Version: 4.3.11CC: abonilla, lsurette, mhicks, oliel, rbarry, rciatti, srevivo
Target Milestone: ovirt-4.3.11Keywords: ZStream
Target Release: ---   
Hardware: Unspecified   
OS: Unspecified   
Whiteboard:
Fixed In Version: Doc Type: If docs needed, set a value
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2020-06-08 15:21:14 UTC Type: Bug
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: Integration RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:
Bug Depends On:    
Bug Blocks: 1838559    

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).