fix ansible version check #1611

ceclinux · 2020-12-04T02:15:23Z

Using string comparsion may cause trouble in version check. For instance, I have ansible 2.10.3 on my laptop and caused

TASK [common : Checking that Ansible version >= '2.4.0'] ***********************
fatal: [k8s-node-master]: FAILED! => {
    "assertion": "ansible_version.full >= \"2.4.0\"",
    "changed": false,
    "evaluated_to": false,
    "msg": "Assertion failed"
}

when runing ./infra/vagrant/provision.sh

vmwclabot · 2020-12-04T02:15:29Z

@ceclinux, you must sign our contributor license agreement before your changes are merged. Click here to sign the agreement. If you are a VMware employee, read this for further instruction.

antrea-bot · 2020-12-04T02:15:39Z

Thanks for your PR.
Unit tests and code linters are run automatically every time the PR is updated.
E2e, conformance and network policy tests can only be triggered by a member of the vmware-tanzu organization. Regular contributors to the project should join the org.

The following commands are available:

/test-e2e: to trigger e2e tests.
/skip-e2e: to skip e2e tests.
/test-conformance: to trigger conformance tests.
/skip-conformance: to skip conformance tests.
/test-all-features-conformance: to trigger conformance tests with all alpha features enabled.
/skip-all-features-conformance: to skip conformance tests with all alpha features enabled.
/test-whole-conformance: to trigger all conformance tests on linux.
/skip-whole-conformance: to skip all conformance tests on linux.
/test-networkpolicy: to trigger networkpolicy tests.
/skip-networkpolicy: to skip networkpolicy tests.
/test-windows-conformance: to trigger windows conformance tests.
/skip-windows-conformance: to skip windows conformance tests.
/test-windows-networkpolicy: to trigger windows networkpolicy tests.
/skip-windows-networkpolicy: to skip windows networkpolicy tests.
/test-hw-offload: to trigger ovs hardware offload test.
/skip-hw-offload: to skip ovs hardware offload test.
/test-all: to trigger all tests (except whole conformance).
/skip-all: to skip all tests (except whole conformance).

codecov-io · 2020-12-04T02:19:33Z

Codecov Report

Merging #1611 (e459be4) into master (9d3d10b) will decrease coverage by 1.51%.
The diff coverage is 53.93%.

@@            Coverage Diff             @@
##           master    #1611      +/-   ##
==========================================
- Coverage   63.31%   61.80%   -1.52%     
==========================================
  Files         170      181      +11     
  Lines       14250    15351    +1101     
==========================================
+ Hits         9023     9488     +465     
- Misses       4292     4854     +562     
- Partials      935     1009      +74

Flag	Coverage Δ
kind-e2e-tests	`52.53% <51.49%> (-2.86%)`	⬇️
unit-tests	`40.47% <12.87%> (-0.81%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
cmd/antrea-agent/agent.go	`0.00% <0.00%> (ø)`
cmd/antrea-agent/options.go	`21.69% <0.00%> (+0.97%)`	⬆️
...gent/controller/noderoute/node_route_controller.go	`46.06% <ø> (-0.41%)`	⬇️
.../agent/flowexporter/connections/conntrack_linux.go	`28.37% <0.00%> (-0.79%)`	⬇️
pkg/agent/stats/collector.go	`97.72% <ø> (ø)`
pkg/controller/networkpolicy/tier.go	`100.00% <ø> (+10.00%)`	⬆️
pkg/features/antrea_features.go	`16.66% <ø> (ø)`
pkg/ovs/openflow/ofctrl_builder.go	`60.58% <0.00%> (-1.59%)`	⬇️
pkg/ovs/openflow/ofctrl_group.go	`48.57% <0.00%> (-4.56%)`	⬇️
pkg/ovs/openflow/ofctrl_action.go	`51.26% <5.71%> (-10.10%)`	⬇️
... and 63 more

antoninbas

LGTM - tested it locally and it works fine with my version of Ansible (2.9.13)

tnqn · 2020-12-04T02:25:15Z

test/e2e/infra/vagrant/playbook/roles/common/tasks/main.yml

@@ -1,6 +1,6 @@
 - name: "Checking that Ansible version >= '2.4.0'"
  assert:
-    that: ansible_version.full >= "2.4.0"


I see the doc says version_compared was renamed to version in 2.5.0. So does it still work if using 2.5.0+?
https://docs.ansible.com/ansible/latest/user_guide/playbooks_tests.html#comparing-versions

cc @antoninbas if he has any idea.

Just saw @antoninbas has tested this with 2.9.13. Then was the doc wrong? or it keeps both version and version_compared?

@tnqn the proposed change works for me with 2.9. It seems that as of today, even top-of-tree supports both: https://github.com/ansible/ansible/blob/221c50b57c347d6f8382523c48e869cb21b8c010/lib/ansible/plugins/test/core.py#L256-L257

My local machine is ansible 2.10.3 and it seems ansible still support version_compared at 2.10.3

Thanks @antoninbas and @ceclinux for confirming. Then it's a doc thing, this patch added that statement but just added an alias "version", not renaming it.
https://github.com/ansible/ansible/pull/32361/files#diff-64ad78caca92056d49f0e3f1a0a3498ea24f9af508981cca7c7dfa21de4004b5R149

tnqn

LGTM

tnqn · 2020-12-04T02:35:27Z

test/e2e/infra/vagrant/playbook/roles/common/tasks/main.yml

@@ -1,6 +1,6 @@
 - name: "Checking that Ansible version >= '2.4.0'"
  assert:
-    that: ansible_version.full >= "2.4.0"


Thanks @antoninbas and @ceclinux for confirming. Then it's a doc thing, this patch added that statement but just added an alias "version", not renaming it.
https://github.com/ansible/ansible/pull/32361/files#diff-64ad78caca92056d49f0e3f1a0a3498ea24f9af508981cca7c7dfa21de4004b5R149

tnqn · 2020-12-04T02:39:33Z

/skip-all as it updates vagrant script only.

antoninbas · 2020-12-04T02:53:15Z

@ceclinux are you a VMware employee or have you signed the CLA?

1. refactor parseMetricFlow to map format 2. add AntreaNetworkPolicyStats test with drop action 3. add extra ansible version check, forgotten by(#1611) 4. change metric to stats in terms of NetworkPolicyStats

…a-io#1606) 1. refactor parseMetricFlow to map format 2. add AntreaNetworkPolicyStats test with drop action 3. add extra ansible version check, forgotten by(antrea-io#1611) 4. change metric to stats in terms of NetworkPolicyStats

1. refactor parseMetricFlow to map format 2. add AntreaNetworkPolicyStats test with drop action 3. add extra ansible version check, forgotten by(#1611) 4. change metric to stats in terms of NetworkPolicyStats

fix ansible version check

e459be4

antoninbas approved these changes Dec 4, 2020

View reviewed changes

tnqn reviewed Dec 4, 2020

View reviewed changes

tnqn approved these changes Dec 4, 2020

View reviewed changes

tnqn merged commit f888369 into antrea-io:master Dec 4, 2020

antoninbas pushed a commit that referenced this pull request Dec 23, 2020

fix ansible version check (#1611)

86777f3

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix ansible version check #1611

fix ansible version check #1611

ceclinux commented Dec 4, 2020

vmwclabot commented Dec 4, 2020

antrea-bot commented Dec 4, 2020

codecov-io commented Dec 4, 2020 •

edited

Loading

antoninbas left a comment

tnqn Dec 4, 2020

tnqn Dec 4, 2020

antoninbas Dec 4, 2020

ceclinux Dec 4, 2020

tnqn Dec 4, 2020

tnqn left a comment

tnqn Dec 4, 2020

tnqn commented Dec 4, 2020

antoninbas commented Dec 4, 2020

fix ansible version check #1611

fix ansible version check #1611

Conversation

ceclinux commented Dec 4, 2020

vmwclabot commented Dec 4, 2020

antrea-bot commented Dec 4, 2020

codecov-io commented Dec 4, 2020 • edited Loading

Codecov Report

antoninbas left a comment

Choose a reason for hiding this comment

tnqn Dec 4, 2020

Choose a reason for hiding this comment

tnqn Dec 4, 2020

Choose a reason for hiding this comment

antoninbas Dec 4, 2020

Choose a reason for hiding this comment

ceclinux Dec 4, 2020

Choose a reason for hiding this comment

tnqn Dec 4, 2020

Choose a reason for hiding this comment

tnqn left a comment

Choose a reason for hiding this comment

tnqn Dec 4, 2020

Choose a reason for hiding this comment

tnqn commented Dec 4, 2020

antoninbas commented Dec 4, 2020

codecov-io commented Dec 4, 2020 •

edited

Loading