NETOBSERV-924 adapt to new ovn annotation format #404
Conversation
The format of this annotation has changed between ocp 4.12 and 4.13 This kind of IPs are typically used in host-network traffic
Codecov Report
@@ Coverage Diff @@
## main #404 +/- ##
=========================================
+ Coverage 0 63.84% +63.84%
=========================================
Files 0 92 +92
Lines 0 6491 +6491
=========================================
+ Hits 0 4144 +4144
- Misses 0 2107 +2107
- Partials 0 240 +240
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 91 files with indirect coverage changes Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
| } | ||
| return "", fmt.Errorf("unexpected content for annotation %s: %s", ovnSubnetAnnotation, subnetsJSON) | ||
| return fmt.Sprintf("%d.%d.%d.2", ip4[0], ip4[1], ip4[2]), nil |
There was a problem hiding this comment.
what is .2 ? maybe u can use fmt.Sprintf("%s",net.IPv4(ip4))
There was a problem hiding this comment.
The IP here is the first one of the range provided by the subnet that is read above; e.g. we read the subnet 10.128.0.0/23, from that we get first-in-range IP 10.128.0.0 ; then it's a tacit rule with ovn-k that the .2 ip from that subnet is used for the mp0 interface, which is the one we want to track here. So we need somehow to convert 10.128.0.0/23 into 10.128.0.2.
There was a problem hiding this comment.
well, we can also write ip4[3] = 2 and return just ip4.String(), to not involve fmt.
There was a problem hiding this comment.
That will work only if you can assume the suffix is never greater than 24. It's better to add 2 or | 2 to ip4[3].
There was a problem hiding this comment.
From past discussions, this is an assumption that we can make - although there's no guarantee that it won't change.
| if subnet, ok := subnetsAsString["default"]; ok { | ||
| return subnet, nil | ||
| } | ||
| return "", fmt.Errorf("unexpected content for annotation %s: %s", ovnSubnetAnnotation, annot) |
There was a problem hiding this comment.
(Minor) Wouldn't it be better to try the new way first and then if it fails, try legacy?
| } | ||
| return "", fmt.Errorf("unexpected content for annotation %s: %s", ovnSubnetAnnotation, subnetsJSON) | ||
| return fmt.Sprintf("%d.%d.%d.2", ip4[0], ip4[1], ip4[2]), nil |
There was a problem hiding this comment.
That will work only if you can assume the suffix is never greater than 24. It's better to add 2 or | 2 to ip4[3].
|
/ok-to-test |
openshift-ci
bot
added
the
ok-to-test
|
New image: quay.io/netobserv/flowlogs-pipeline:202938e. It will expire after two weeks. |
|
/label qe-approved |
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: jotak The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
The format of this annotation has changed between ocp 4.12 and 4.13 This kind of IPs are typically used in host-network traffic