Use fmt.Sprintf format a ginkgo Should #688

qinqon · 2021-02-01T07:33:26Z

Is this a BUG FIX or a FEATURE ?:

Uncomment only one, leave it on its own line:

/kind bug

/kind enhancement

What this PR does / why we need it:
Ginkgo Should description allow to pass a string and some arguments but
the string has to follow Sprintf format. This change fix one test that
was not using "%s" expansion token at the string, also use the lazy
initialization mechanism to get the current after eventually is
finished.

Special notes for your reviewer:

Release note:

NONE

rhrazdil

/lgtm

oshoval

Type in PR description Sptinf
also maybe rephrase Ginkgo Should description allow ?

qinqon · 2021-02-01T09:37:19Z

current state diff is not working as expected

/tmp/knmstate/kubernetes-nmstate/test/e2e/handler/nns_update_timestamp_test.go:31
Failed after 58.520s.
currentState diff: -dns-resolver:
-  config: {}
-  running:
-    search: []
-    server:
-    - 192.168.66.2
-    - fe80::c8ff:b9ff:fe0b:4b0d%eth0
-    - fd00::1
-interfaces:
-- bridge:
-    options:
-      group-addr: 01:80:C2:00:00:00
-      group-forward-mask: 0
-      hash-max: 512
-      mac-ageing-time: 300
-      multicast-last-member-count: 2
-      multicast-last-member-interval: 100
-      multicast-querier: false
-      multicast-querier-interval: 25500
-      multicast-query-interval: 12500
-      multicast-query-response-interval: 1000
-      multicast-query-use-ifaddr: false
-      multicast-router: 1
-      multicast-snooping: true
-      multicast-startup-query-count: 2
-      multicast-startup-query-interval: 3125
-      stp:
-        enabled: false
-        forward-delay: 15
-        hello-time: 2
-        max-age: 20
-        priority: 32768
-    port: []
-  ipv4:
-    address:
-    - ip: 172.17.0.1
-      prefix-length: 16
-    dhcp: false
-    enabled: true
-  ipv6:
-    address:
-    - ip: 2001:db9:1::1
-      prefix-length: 64
-    - ip: fe80::1
-      prefix-length: 64
-    autoconf: false
-    dhcp: false
-    enabled: true
-  lldp:
-    enabled: false
-  mac-address: 02:42:34:4C:BE:FC
-  mtu: 1500
-  name: docker0
-  state: up
-  type: linux-bridge
-- ipv4:
-    address:
-    - ip: 192.169.1.50
-      prefix-length: 24
-    dhcp: false
-    enabled: true
-  ipv6:
-    address:
-    - ip: fe80::d344:4151:f6ba:ef92
-      prefix-length: 64
-    auto-dns: true
-    auto-gateway: true
-    auto-route-table-id: 0
-    auto-routes: true
-    autoconf: true
-    dhcp: true
-    enabled: true
-  lldp:
-    enabled: false
-  mac-address: 2E:48:B0:EA:12:B0
-  mtu: 1500
-  name: dummy0
-  state: up
-  type: dummy
-- ipv4:
-    address:
-    - ip: 192.168.66.103
-      prefix-length: 24
-    auto-dns: true
-    auto-gateway: true
-    auto-route-table-id: 0
-    auto-routes: true
-    dhcp: true
-    enabled: true
-  ipv6:
-    address:
-    - ip: fd00::103
-      prefix-length: 128
-    - ip: fe80::4f15:9e7f:b6e2:2870
-      prefix-length: 64
-    auto-dns: true
-    auto-gateway: true
-    auto-route-table-id: 0
-    auto-routes: true
-    autoconf: true
-    dhcp: true
-    enabled: true
-  lldp:
-    enabled: false
-  mac-address: 52:55:00:D1:55:03
-  mtu: 1500
-  name: eth0
-  state: up
-  type: ethernet
-- ipv4:
-    address: []
-    enabled: false
-  ipv6:
-    address: []
-    enabled: false
-  lldp:
-    enabled: false
-  mac-address: 52:55:00:D1:56:04
-  mtu: 1500
-  name: eth1
-  state: down
-  type: ethernet
-- ipv4:
-    address: []
-    enabled: false
-  ipv6:
-    address: []
-    enabled: false
-  lldp:
-    enabled: false
-  mac-address: 52:55:00:D1:56:05
-  mtu: 1500
-  name: eth2
-  state: down
-  type: ethernet
-- ipv4:
-    address:
-    - ip: 127.0.0.1
-      prefix-length: 8
-    enabled: true
-  ipv6:
-    address:
-    - ip: ::1
-      prefix-length: 128
-    enabled: true
-  mac-address: "00:00:00:00:00:00"
-  mtu: 65536
-  name: lo
-  state: up
-  type: unknown
-- ipv4:
-    address:
-    - ip: 10.244.186.192
-      prefix-length: 32
-    enabled: true
-  ipv6:
-    address: []
-    enabled: false
-  mac-address: "00:00:00:00"
-  mtu: 1440
-  name: tunl0
-  state: up
-  type: unknown
-route-rules:
-  config: []
-routes:
-  config: []
-  running:
-  - destination: ::1/128
-    metric: 256
-    next-hop-address: ""
-    next-hop-interface: lo
-    table-id: 254
-  - destination: 2001:db9:1::/64
-    metric: 256
-    next-hop-address: ""
-    next-hop-interface: docker0
-    table-id: 254
-  - destination: 2001:db9:1::/64
-    metric: 1024
-    next-hop-address: ""
-    next-hop-interface: docker0
-    table-id: 254
-  - destination: fd00::103/128
-    metric: 100
-    next-hop-address: ""
-    next-hop-interface: eth0
-    table-id: 254
-  - destination: fd00::/64
-    metric: 100
-    next-hop-address: ""
-    next-hop-interface: eth0
-    table-id: 254
-  - destination: fd10:244::8c40/122
-    metric: 1024
-    next-hop-address: fd00::102
-    next-hop-interface: eth0
-    table-id: 254
-  - destination: fd10:244::bac0/122
-    metric: 1024
-    next-hop-address: ""
-    next-hop-interface: lo
-    table-id: 254
-  - destination: fd10:244::c480/122
-    metric: 1024
-    next-hop-address: fd00::101
-    next-hop-interface: eth0
-    table-id: 254
-  - destination: fe80::/64
-    metric: 100
-    next-hop-address: ""
-    next-hop-interface: eth0
-    table-id: 254
-  - destination: fe80::/64
-    metric: 256
-    next-hop-address: ""
-    next-hop-interface: docker0
-    table-id: 254
-  - destination: fe80::/64
-    metric: 550
-    next-hop-address: ""
-    next-hop-interface: dummy0
-    table-id: 254
-  - destination: ::/0
-    metric: 100
-    next-hop-address: fe80::c8ff:b9ff:fe0b:4b0d
-    next-hop-interface: eth0
-    table-id: 254
-  - destination: ::1/128
-    metric: 0
-    next-hop-address: ""
-    next-hop-interface: lo
-    table-id: 255
-  - destination: fd00::103/128
-    metric: 0
-    next-hop-address: ""
-    next-hop-interface: eth0
-    table-id: 255
-  - destination: fe80::/128
-    metric: 0
-    next-hop-address: ""
-    next-hop-interface: eth0
-    table-id: 255
-  - destination: fe80::4f15:9e7f:b6e2:2870/128
-    metric: 0
-    next-hop-address: ""
-    next-hop-interface: eth0
-    table-id: 255
-  - destination: fe80::d344:4151:f6ba:ef92/128
-    metric: 0
-    next-hop-address: ""
-    next-hop-interface: dummy0
-    table-id: 255
-  - destination: ff00::/8
-    metric: 256
-    next-hop-address: ""
-    next-hop-interface: eth0
-    table-id: 255
-  - destination: ff00::/8
-    metric: 256
-    next-hop-address: ""
-    next-hop-interface: tunl0
-    table-id: 255
-  - destination: ff00::/8
-    metric: 256
-    next-hop-address: ""
-    next-hop-interface: eth1
-    table-id: 255
-  - destination: ff00::/8
-    metric: 256
-    next-hop-address: ""
-    next-hop-interface: eth2
-    table-id: 255
-  - destination: ff00::/8
-    metric: 256
-    next-hop-address: ""
-    next-hop-interface: dummy0
-    table-id: 255
-  - destination: 0.0.0.0/0
-    metric: 100
-    next-hop-address: 192.168.66.2
-    next-hop-interface: eth0
-    table-id: 254
-  - destination: 10.244.140.64/26
-    metric: 0
-    next-hop-address: 192.168.66.102
-    next-hop-interface: tunl0
-    table-id: 254
-  - destination: 10.244.186.192/26
-    metric: 0
-    next-hop-address: ""
-    next-hop-interface: ""
-    table-id: 254
-  - destination: 10.244.196.128/26
-    metric: 0
-    next-hop-address: 192.168.66.101
-    next-hop-interface: tunl0
-    table-id: 254
Expected
    <string>: NodeNetworkStateStatus
to match fields: {
.CurrentState:
	Expected
	    <string>: "...92.168.66.1..."
	to equal               |
	    <string>: "...92.169.1.50..."
.LastSuccessfulUpdateTime:
	Expected
	    <v1.Time>: {
	        Time: 2021-02-01T07:52:48Z,
	    }
	to equal
	    <v1.Time>: {
	        Time: 2021-02-01T07:51:44Z,
	    }
}

qinqon · 2021-02-01T09:46:42Z

force-push: Do lazy evaluation for the currentState so we get the latest value.

test/e2e/handler/nns_update_timestamp_test.go

oshoval · 2021-02-01T13:04:39Z

Please remove the "how to hints" from the PR desc, commit message has the typo as well (up to you),
beside that conceptual lgtm (just waiting for Radim comments).

Thanks

Ginkgo Should description allow to pass a string and some arguments but the string has to follow Sprintf format. This change fix one test that was not using "%s" expansion token at the string, also use the lazy initialization mechanism to get the current after eventually is finished. Signed-off-by: Quique Llorente <[email protected]>

rhrazdil · 2021-02-08T09:30:46Z

Thanks
/lgtm

phoracek

/approve

kubevirt-bot · 2021-02-08T09:44:37Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: phoracek

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:

~~OWNERS~~ [phoracek]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

qinqon · 2021-02-08T09:56:49Z

We have being "lucky" and the flaky appears here.

/tmp/knmstate/kubernetes-nmstate/test/e2e/handler/nns_update_timestamp_test.go:32
Failed after 42.279s.
currentState diff: 
 dns-resolver:
   config:
     search: []
     server: []
   running:
     search: []
     server:
     - 192.168.66.2
     - fe80::840:62ff:fe70:ff1e%eth0
     - fd00::1
 interfaces:
 - bridge:
     options:
       group-addr: 01:80:C2:00:00:00
       group-forward-mask: 0
       hash-max: 4096
       mac-ageing-time: 300
       multicast-last-member-count: 2
       multicast-last-member-interval: 100
       multicast-querier: false
       multicast-querier-interval: 25500
       multicast-query-interval: 12500
       multicast-query-response-interval: 1000
       multicast-query-use-ifaddr: false
       multicast-router: 1
       multicast-snooping: true
       multicast-startup-query-count: 2
       multicast-startup-query-interval: 3125
       stp:
         enabled: false
         forward-delay: 15
         hello-time: 2
         max-age: 20
         priority: 32768
     port: []
   ipv4:
     address:
     - ip: 172.17.0.1
       prefix-length: 16
     dhcp: false
     enabled: true
   ipv6:
     address:
     - ip: 2001:db9:1::1
       prefix-length: 64
     - ip: fe80::1
       prefix-length: 64
     autoconf: false
     dhcp: false
     enabled: true
   lldp:
     enabled: false
   mac-address: 02:42:2D:4B:F0:B4
   mtu: 1500
   name: docker0
   state: up
   type: linux-bridge
 - ipv4:
     address:
     - ip: 192.168.66.101
       prefix-length: 24
     auto-dns: true
     auto-gateway: true
     auto-routes: true
     dhcp: true
     enabled: true
   ipv6:
     address:
     - ip: fd00::101
       prefix-length: 128
     - ip: fe80::43d6:b934:3fc:1dca
       prefix-length: 64
     auto-dns: true
     auto-gateway: true
     auto-routes: true
     autoconf: true
     dhcp: true
     enabled: true
   lldp:
     enabled: false
   mac-address: 52:55:00:D1:55:01
   mtu: 1500
   name: eth0
   state: up
   type: ethernet
 - ipv4:
     enabled: false
   ipv6:
     enabled: false
   lldp:
     enabled: false
   mac-address: 52:55:00:D1:56:00
   mtu: 1500
   name: eth1
   state: down
   type: ethernet
 - ipv4:
     enabled: false
   ipv6:
     enabled: false
   lldp:
     enabled: false
   mac-address: 52:55:00:D1:56:01
   mtu: 1500
   name: eth2
   state: down
   type: ethernet
 - ipv4:
     enabled: false
   ipv6:
     enabled: false
   lldp:
     enabled: false
   mtu: 65536
   name: lo
   state: down
   type: unknown
 - ipv4:
     enabled: false
   ipv6:
     enabled: false
   lldp:
     enabled: false
   mac-address: "00:00:00:00"
   mtu: 1440
   name: tunl0
   state: down
   type: unknown
 route-rules:
   config: []
 routes:
   config: []
   running:
   - destination: 172.17.0.0/16
     metric: 0
     next-hop-address: ""
     next-hop-interface: docker0
     table-id: 254
   - destination: 0.0.0.0/0
     metric: 100
     next-hop-address: 192.168.66.2
     next-hop-interface: eth0
     table-id: 254
   - destination: 192.168.66.0/24
     metric: 100
     next-hop-address: ""
     next-hop-interface: eth0
     table-id: 254
   - destination: 2001:db9:1::/64
     metric: 256
     next-hop-address: ""
     next-hop-interface: docker0
     table-id: 254
   - destination: 2001:db9:1::/64
     metric: 1024
     next-hop-address: ""
     next-hop-interface: docker0
     table-id: 254
   - destination: fe80::/64
     metric: 256
     next-hop-address: ""
     next-hop-interface: docker0
     table-id: 254
   - destination: ::/0
     metric: 100
     next-hop-address: fe80::840:62ff:fe70:ff1e
     next-hop-interface: eth0
     table-id: 254
   - destination: fd00::/64
     metric: 100
     next-hop-address: ""
     next-hop-interface: eth0
     table-id: 254
   - destination: fd00::101/128
     metric: 100
     next-hop-address: ""
     next-hop-interface: eth0
     table-id: 254
-  - destination: fd10:244::bac0/122
-    metric: 1024
-    next-hop-address: fd00::103
-    next-hop-interface: eth0
-    table-id: 254
   - destination: fe80::/64
     metric: 100
     next-hop-address: ""
     next-hop-interface: eth0
     table-id: 254
   - destination: ff00::/8
     metric: 256
     next-hop-address: ""
     next-hop-interface: eth0
     table-id: 255
Expected
    <string>: NodeNetworkStateStatus
to match fields: {
.CurrentState:
	Expected
	    <string>: "...on: fe80::/..."
	to equal               |
	    <string>: "...on: fd10:24..."
.LastSuccessfulUpdateTime:
	Expected
	    <v1.Time>: {
	        Time: 2021-02-08T09:38:02Z,
	    }
	to equal
	    <v1.Time>: {
	        Time: 2021-02-08T09:36:58Z,
	    }
}

So looks like the diff thing works fine.

qinqon · 2021-02-08T10:00:19Z

/retest

kubevirt-bot · 2021-02-08T10:07:43Z

@qinqon: The following test failed, say /retest to rerun all failed tests:

Test name	Commit	Details	Rerun command
pull-kubernetes-nmstate-e2e-handler-k8s-future	`c9f2a83`	link	`/test pull-kubernetes-nmstate-e2e-handler-k8s-future`

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

kubevirt-bot added release-note-none Denotes a PR that doesn't merit a release note. dco-signoff: yes Indicates the PR's author has DCO signed all their commits. kind/bug labels Feb 1, 2021

kubevirt-bot requested review from AlonaKaplan and oshoval February 1, 2021 07:33

qinqon requested review from RamLavi and phoracek and removed request for AlonaKaplan and oshoval February 1, 2021 07:33

kubevirt-bot added the size/XS label Feb 1, 2021

qinqon requested a review from rhrazdil February 1, 2021 07:33

rhrazdil approved these changes Feb 1, 2021

View reviewed changes

kubevirt-bot assigned rhrazdil Feb 1, 2021

kubevirt-bot added the lgtm Indicates that a PR is ready to be merged. label Feb 1, 2021

oshoval reviewed Feb 1, 2021

View reviewed changes

qinqon force-pushed the use-sprintf-format-at-should branch from 890a438 to 9938bad Compare February 1, 2021 09:43

kubevirt-bot removed the lgtm Indicates that a PR is ready to be merged. label Feb 1, 2021

qinqon force-pushed the use-sprintf-format-at-should branch from 9938bad to 09aac3d Compare February 1, 2021 09:45

qinqon requested review from rhrazdil and oshoval February 1, 2021 09:47

qinqon force-pushed the use-sprintf-format-at-should branch from 09aac3d to 38e5beb Compare February 1, 2021 10:15

rhrazdil requested changes Feb 1, 2021

View reviewed changes

test/e2e/handler/nns_update_timestamp_test.go Outdated Show resolved Hide resolved

qinqon force-pushed the use-sprintf-format-at-should branch from 38e5beb to c9f2a83 Compare February 8, 2021 08:55

qinqon requested a review from rhrazdil February 8, 2021 08:56

kubevirt-bot added the lgtm Indicates that a PR is ready to be merged. label Feb 8, 2021

phoracek reviewed Feb 8, 2021

View reviewed changes

kubevirt-bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Feb 8, 2021

qinqon mentioned this pull request Feb 8, 2021

[flaky CI] [nns] NNS LastSuccessfulUpdateTime when network configuration hasn't change should not be updated #670

Closed

kubevirt-bot merged commit 687cead into nmstate:master Feb 8, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use fmt.Sprintf format a ginkgo Should #688

Use fmt.Sprintf format a ginkgo Should #688

qinqon commented Feb 1, 2021 •

edited

Loading

rhrazdil left a comment

oshoval left a comment

qinqon commented Feb 1, 2021

qinqon commented Feb 1, 2021

oshoval commented Feb 1, 2021

rhrazdil commented Feb 8, 2021

phoracek left a comment

kubevirt-bot commented Feb 8, 2021

qinqon commented Feb 8, 2021

qinqon commented Feb 8, 2021

kubevirt-bot commented Feb 8, 2021 •

edited

Loading

Use fmt.Sprintf format a ginkgo Should #688

Use fmt.Sprintf format a ginkgo Should #688

Conversation

qinqon commented Feb 1, 2021 • edited Loading

rhrazdil left a comment

Choose a reason for hiding this comment

oshoval left a comment

Choose a reason for hiding this comment

qinqon commented Feb 1, 2021

qinqon commented Feb 1, 2021

oshoval commented Feb 1, 2021

rhrazdil commented Feb 8, 2021

phoracek left a comment

Choose a reason for hiding this comment

kubevirt-bot commented Feb 8, 2021

qinqon commented Feb 8, 2021

qinqon commented Feb 8, 2021

kubevirt-bot commented Feb 8, 2021 • edited Loading

qinqon commented Feb 1, 2021 •

edited

Loading

kubevirt-bot commented Feb 8, 2021 •

edited

Loading