{"action":"created","comment":{"url":"https://api.github.com/repos/Metaswitch/calico/pulls/comments/28723694","id":28723694,"diff_hunk":"@@ -495,3 +496,78 @@ def test_check_segment_for_agent(self):\n             {mech_calico.api.NETWORK_TYPE: 'vlan'},\n             mech_calico.constants.AGENT_TYPE_DHCP\n         ))\n+\n+    def test_neutron_rule_to_etcd_rule_icmp(self):\n+        # No type/code specified\n+        self.assertNeutronToEtcd(neutron_rule({\n+            \"ethertype\": \"IPv4\",\n+            \"protocol\": \"icmp\",\n+        }), {\n+            'ip_version': 4,\n+            'protocol': 'icmp',\n+            'src_net': '0.0.0.0/0',\n+        })\n+        # Type/code wildcarded, same as above.\n+        self.assertNeutronToEtcd(neutron_rule({\n+            \"ethertype\": \"IPv4\",\n+            \"protocol\": \"icmp\",\n+            \"port_range_min\": -1,\n+            \"port_range_max\": -1,\n+        }), {\n+            'ip_version': 4,\n+            'protocol': 'icmp',\n+            'src_net': '0.0.0.0/0',\n+        })\n+        # Type and code.\n+        self.assertNeutronToEtcd(neutron_rule({\n+            \"ethertype\": \"IPv4\",\n+            \"protocol\": \"icmp\",\n+            \"port_range_min\": 123,\n+            \"port_range_max\": 100,\n+        }), {\n+            'ip_version': 4,\n+            'protocol': 'icmp',\n+            'src_net': '0.0.0.0/0',\n+            'icmp_type': 123,\n+            'icmp_code': 100,\n+        })\n+        # Type and code, IPv6.\n+        self.assertNeutronToEtcd(neutron_rule({\n+            \"ethertype\": \"IPv6\",\n+            \"protocol\": \"icmp\",\n+            \"port_range_min\": 123,\n+            \"port_range_max\": 100,\n+        }), {\n+            'ip_version': 6,\n+            'protocol': 'icmpv6',\n+            'src_net': '::/0',\n+            'icmp_type': 123,\n+            'icmp_code': 100,\n+        })\n+\n+    def assertNeutronToEtcd(self, neutron_rule, exp_etcd_rule):\n+        etcd_rule = t_etcd._neutron_rule_to_etcd_rule(neutron_rule)\n+        self.assertEqual(etcd_rule, exp_etcd_rule)\n+\n+        # Check felix is happy with generated rule.\n+        if neutron_rule[\"direction\"] == \"ingress\":\n+            rules = {\"inbound_rules\": [neutron_rule],\n+                     \"outbound_rules\": []}\n+        else:\n+            rules = {\"outbound_rules\": [neutron_rule],\n+                     \"inbound_rules\": []}\n+        fetcd.validate_rules(rules)\n+\n+\n+def neutron_rule(overrides):\n+    rule = {\n+        \"ethertype\": \"IPv4\",\n+        \"protocol\": None,\n+        \"remote_ip_prefix\": None,\n+        \"remote_group_id\": None,\n+        \"direction\": \"ingress\",\n+        \"port_range_min\": None,\n+        \"port_range_max\": None,\n+    }\n+    rule.update(overrides)\n+    return rule","path":"calico/openstack/test/test_plugin_etcd.py","position":86,"original_position":86,"commit_id":"59665bab13987d8b07110df9363f32e960a612b0","original_commit_id":"59665bab13987d8b07110df9363f32e960a612b0","user":{"login":"robbrockbank","id":3227729,"avatar_url":"https://avatars.githubusercontent.com/u/3227729?v=3","gravatar_id":"","url":"https://api.github.com/users/robbrockbank","html_url":"https://github.com/robbrockbank","followers_url":"https://api.github.com/users/robbrockbank/followers","following_url":"https://api.github.com/users/robbrockbank/following{/other_user}","gists_url":"https://api.github.com/users/robbrockbank/gists{/gist_id}","starred_url":"https://api.github.com/users/robbrockbank/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/robbrockbank/subscriptions","organizations_url":"https://api.github.com/users/robbrockbank/orgs","repos_url":"https://api.github.com/users/robbrockbank/repos","events_url":"https://api.github.com/users/robbrockbank/events{/privacy}","received_events_url":"https://api.github.com/users/robbrockbank/received_events","type":"User","site_admin":false},"body":"missing newline at end of file","created_at":"2015-04-20T20:06:06Z","updated_at":"2015-04-20T20:06:06Z","html_url":"https://github.com/Metaswitch/calico/pull/421#discussion_r28723694","pull_request_url":"https://api.github.com/repos/Metaswitch/calico/pulls/421","_links":{"self":{"href":"https://api.github.com/repos/Metaswitch/calico/pulls/comments/28723694"},"html":{"href":"https://github.com/Metaswitch/calico/pull/421#discussion_r28723694"},"pull_request":{"href":"https://api.github.com/repos/Metaswitch/calico/pulls/421"}}},"pull_request":{"url":"https://api.github.com/repos/Metaswitch/calico/pulls/421","id":33699721,"html_url":"https://github.com/Metaswitch/calico/pull/421","diff_url":"https://github.com/Metaswitch/calico/pull/421.diff","patch_url":"https://github.com/Metaswitch/calico/pull/421.patch","issue_url":"https://api.github.com/repos/Metaswitch/calico/issues/421","number":421,"state":"open","locked":false,"title":"Fix ICMP rule generation in plugin; add missing support for ICMP code","user":{"login":"fasaxc","id":469264,"avatar_url":"https://avatars.githubusercontent.com/u/469264?v=3","gravatar_id":"","url":"https://api.github.com/users/fasaxc","html_url":"https://github.com/fasaxc","followers_url":"https://api.github.com/users/fasaxc/followers","following_url":"https://api.github.com/users/fasaxc/following{/other_user}","gists_url":"https://api.github.com/users/fasaxc/gists{/gist_id}","starred_url":"https://api.github.com/users/fasaxc/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/fasaxc/subscriptions","organizations_url":"https://api.github.com/users/fasaxc/orgs","repos_url":"https://api.github.com/users/fasaxc/repos","events_url":"https://api.github.com/users/fasaxc/events{/privacy}","received_events_url":"https://api.github.com/users/fasaxc/received_events","type":"User","site_admin":false},"body":"* Plugin was generating rules with source/dest port as well as ICMP type/code, which felix was asserting on.  Fixed by moving the ICMP code into an alternating branch with the source/dest port code.\r\n* Felix was missing support for the ICMP code.  I've added this in but there's a caveat: felix can only match on icmp type or icmp type and code; it can't match on code alone, which does seem to be allowed in OpenStack.  Need to discuss the correct approach here.  (Expanding to 255 rules seems overkill?)\r\n* Added in missing validation for ICMP type/code.\r\n* Added plugin UTs.\r\n\r\nNot to reviewer: the first commit in the pull req makes the change to the plugin.  \"Add UTs\" commit refactors the plugin code to make it easier to test the rule creation in isolation.\r\n\r\nFixes #414.","created_at":"2015-04-20T19:52:16Z","updated_at":"2015-04-20T20:06:06Z","closed_at":null,"merged_at":null,"merge_commit_sha":"1e16a9a44ac0541c588dcfe5e133d41991c0da60","assignee":{"login":"robbrockbank","id":3227729,"avatar_url":"https://avatars.githubusercontent.com/u/3227729?v=3","gravatar_id":"","url":"https://api.github.com/users/robbrockbank","html_url":"https://github.com/robbrockbank","followers_url":"https://api.github.com/users/robbrockbank/followers","following_url":"https://api.github.com/users/robbrockbank/following{/other_user}","gists_url":"https://api.github.com/users/robbrockbank/gists{/gist_id}","starred_url":"https://api.github.com/users/robbrockbank/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/robbrockbank/subscriptions","organizations_url":"https://api.github.com/users/robbrockbank/orgs","repos_url":"https://api.github.com/users/robbrockbank/repos","events_url":"https://api.github.com/users/robbrockbank/events{/privacy}","received_events_url":"https://api.github.com/users/robbrockbank/received_events","type":"User","site_admin":false},"milestone":{"url":"https://api.github.com/repos/Metaswitch/calico/milestones/2","html_url":"https://github.com/Metaswitch/calico/milestones/0.17","labels_url":"https://api.github.com/repos/Metaswitch/calico/milestones/2/labels","id":1071957,"number":2,"title":"0.17","description":"","creator":{"login":"Lukasa","id":1382556,"avatar_url":"https://avatars.githubusercontent.com/u/1382556?v=3","gravatar_id":"","url":"https://api.github.com/users/Lukasa","html_url":"https://github.com/Lukasa","followers_url":"https://api.github.com/users/Lukasa/followers","following_url":"https://api.github.com/users/Lukasa/following{/other_user}","gists_url":"https://api.github.com/users/Lukasa/gists{/gist_id}","starred_url":"https://api.github.com/users/Lukasa/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/Lukasa/subscriptions","organizations_url":"https://api.github.com/users/Lukasa/orgs","repos_url":"https://api.github.com/users/Lukasa/repos","events_url":"https://api.github.com/users/Lukasa/events{/privacy}","received_events_url":"https://api.github.com/users/Lukasa/received_events","type":"User","site_admin":false},"open_issues":26,"closed_issues":4,"state":"open","created_at":"2015-04-17T09:33:49Z","updated_at":"2015-04-20T19:52:20Z","due_on":"2015-04-26T23:00:00Z","closed_at":null},"commits_url":"https://api.github.com/repos/Metaswitch/calico/pulls/421/commits","review_comments_url":"https://api.github.com/repos/Metaswitch/calico/pulls/421/comments","review_comment_url":"https://api.github.com/repos/Metaswitch/calico/pulls/comments{/number}","comments_url":"https://api.github.com/repos/Metaswitch/calico/issues/421/comments","statuses_url":"https://api.github.com/repos/Metaswitch/calico/statuses/59665bab13987d8b07110df9363f32e960a612b0","head":{"label":"Metaswitch:smc-fix-icmp","ref":"smc-fix-icmp","sha":"59665bab13987d8b07110df9363f32e960a612b0","user":{"login":"Metaswitch","id":453759,"avatar_url":"https://avatars.githubusercontent.com/u/453759?v=3","gravatar_id":"","url":"https://api.github.com/users/Metaswitch","html_url":"https://github.com/Metaswitch","followers_url":"https://api.github.com/users/Metaswitch/followers","following_url":"https://api.github.com/users/Metaswitch/following{/other_user}","gists_url":"https://api.github.com/users/Metaswitch/gists{/gist_id}","starred_url":"https://api.github.com/users/Metaswitch/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/Metaswitch/subscriptions","organizations_url":"https://api.github.com/users/Metaswitch/orgs","repos_url":"https://api.github.com/users/Metaswitch/repos","events_url":"https://api.github.com/users/Metaswitch/events{/privacy}","received_events_url":"https://api.github.com/users/Metaswitch/received_events","type":"Organization","site_admin":false},"repo":{"id":21242831,"name":"calico","full_name":"Metaswitch/calico","owner":{"login":"Metaswitch","id":453759,"avatar_url":"https://avatars.githubusercontent.com/u/453759?v=3","gravatar_id":"","url":"https://api.github.com/users/Metaswitch","html_url":"https://github.com/Metaswitch","followers_url":"https://api.github.com/users/Metaswitch/followers","following_url":"https://api.github.com/users/Metaswitch/following{/other_user}","gists_url":"https://api.github.com/users/Metaswitch/gists{/gist_id}","starred_url":"https://api.github.com/users/Metaswitch/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/Metaswitch/subscriptions","organizations_url":"https://api.github.com/users/Metaswitch/orgs","repos_url":"https://api.github.com/users/Metaswitch/repos","events_url":"https://api.github.com/users/Metaswitch/events{/privacy}","received_events_url":"https://api.github.com/users/Metaswitch/received_events","type":"Organization","site_admin":false},"private":false,"html_url":"https://github.com/Metaswitch/calico","description":"Project Calico core repository.","fork":false,"url":"https://api.github.com/repos/Metaswitch/calico","forks_url":"https://api.github.com/repos/Metaswitch/calico/forks","keys_url":"https://api.github.com/repos/Metaswitch/calico/keys{/key_id}","collaborators_url":"https://api.github.com/repos/Metaswitch/calico/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/Metaswitch/calico/teams","hooks_url":"https://api.github.com/repos/Metaswitch/calico/hooks","issue_events_url":"https://api.github.com/repos/Metaswitch/calico/issues/events{/number}","events_url":"https://api.github.com/repos/Metaswitch/calico/events","assignees_url":"https://api.github.com/repos/Metaswitch/calico/assignees{/user}","branches_url":"https://api.github.com/repos/Metaswitch/calico/branches{/branch}","tags_url":"https://api.github.com/repos/Metaswitch/calico/tags","blobs_url":"https://api.github.com/repos/Metaswitch/calico/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/Metaswitch/calico/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/Metaswitch/calico/git/refs{/sha}","trees_url":"https://api.github.com/repos/Metaswitch/calico/git/trees{/sha}","statuses_url":"https://api.github.com/repos/Metaswitch/calico/statuses/{sha}","languages_url":"https://api.github.com/repos/Metaswitch/calico/languages","stargazers_url":"https://api.github.com/repos/Metaswitch/calico/stargazers","contributors_url":"https://api.github.com/repos/Metaswitch/calico/contributors","subscribers_url":"https://api.github.com/repos/Metaswitch/calico/subscribers","subscription_url":"https://api.github.com/repos/Metaswitch/calico/subscription","commits_url":"https://api.github.com/repos/Metaswitch/calico/commits{/sha}","git_commits_url":"https://api.github.com/repos/Metaswitch/calico/git/commits{/sha}","comments_url":"https://api.github.com/repos/Metaswitch/calico/comments{/number}","issue_comment_url":"https://api.github.com/repos/Metaswitch/calico/issues/comments{/number}","contents_url":"https://api.github.com/repos/Metaswitch/calico/contents/{+path}","compare_url":"https://api.github.com/repos/Metaswitch/calico/compare/{base}...{head}","merges_url":"https://api.github.com/repos/Metaswitch/calico/merges","archive_url":"https://api.github.com/repos/Metaswitch/calico/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/Metaswitch/calico/downloads","issues_url":"https://api.github.com/repos/Metaswitch/calico/issues{/number}","pulls_url":"https://api.github.com/repos/Metaswitch/calico/pulls{/number}","milestones_url":"https://api.github.com/repos/Metaswitch/calico/milestones{/number}","notifications_url":"https://api.github.com/repos/Metaswitch/calico/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/Metaswitch/calico/labels{/name}","releases_url":"https://api.github.com/repos/Metaswitch/calico/releases{/id}","created_at":"2014-06-26T14:08:43Z","updated_at":"2015-04-20T19:30:41Z","pushed_at":"2015-04-20T19:52:17Z","git_url":"git://github.com/Metaswitch/calico.git","ssh_url":"git@github.com:Metaswitch/calico.git","clone_url":"https://github.com/Metaswitch/calico.git","svn_url":"https://github.com/Metaswitch/calico","homepage":"http://projectcalico.org/","size":9893,"stargazers_count":42,"watchers_count":42,"language":"Python","has_issues":true,"has_downloads":true,"has_wiki":false,"has_pages":false,"forks_count":11,"mirror_url":null,"open_issues_count":68,"forks":11,"open_issues":68,"watchers":42,"default_branch":"master"}},"base":{"label":"Metaswitch:master","ref":"master","sha":"ab7476ddd0bb1a163921ab197a3817f56a3e0cfc","user":{"login":"Metaswitch","id":453759,"avatar_url":"https://avatars.githubusercontent.com/u/453759?v=3","gravatar_id":"","url":"https://api.github.com/users/Metaswitch","html_url":"https://github.com/Metaswitch","followers_url":"https://api.github.com/users/Metaswitch/followers","following_url":"https://api.github.com/users/Metaswitch/following{/other_user}","gists_url":"https://api.github.com/users/Metaswitch/gists{/gist_id}","starred_url":"https://api.github.com/users/Metaswitch/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/Metaswitch/subscriptions","organizations_url":"https://api.github.com/users/Metaswitch/orgs","repos_url":"https://api.github.com/users/Metaswitch/repos","events_url":"https://api.github.com/users/Metaswitch/events{/privacy}","received_events_url":"https://api.github.com/users/Metaswitch/received_events","type":"Organization","site_admin":false},"repo":{"id":21242831,"name":"calico","full_name":"Metaswitch/calico","owner":{"login":"Metaswitch","id":453759,"avatar_url":"https://avatars.githubusercontent.com/u/453759?v=3","gravatar_id":"","url":"https://api.github.com/users/Metaswitch","html_url":"https://github.com/Metaswitch","followers_url":"https://api.github.com/users/Metaswitch/followers","following_url":"https://api.github.com/users/Metaswitch/following{/other_user}","gists_url":"https://api.github.com/users/Metaswitch/gists{/gist_id}","starred_url":"https://api.github.com/users/Metaswitch/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/Metaswitch/subscriptions","organizations_url":"https://api.github.com/users/Metaswitch/orgs","repos_url":"https://api.github.com/users/Metaswitch/repos","events_url":"https://api.github.com/users/Metaswitch/events{/privacy}","received_events_url":"https://api.github.com/users/Metaswitch/received_events","type":"Organization","site_admin":false},"private":false,"html_url":"https://github.com/Metaswitch/calico","description":"Project Calico core repository.","fork":false,"url":"https://api.github.com/repos/Metaswitch/calico","forks_url":"https://api.github.com/repos/Metaswitch/calico/forks","keys_url":"https://api.github.com/repos/Metaswitch/calico/keys{/key_id}","collaborators_url":"https://api.github.com/repos/Metaswitch/calico/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/Metaswitch/calico/teams","hooks_url":"https://api.github.com/repos/Metaswitch/calico/hooks","issue_events_url":"https://api.github.com/repos/Metaswitch/calico/issues/events{/number}","events_url":"https://api.github.com/repos/Metaswitch/calico/events","assignees_url":"https://api.github.com/repos/Metaswitch/calico/assignees{/user}","branches_url":"https://api.github.com/repos/Metaswitch/calico/branches{/branch}","tags_url":"https://api.github.com/repos/Metaswitch/calico/tags","blobs_url":"https://api.github.com/repos/Metaswitch/calico/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/Metaswitch/calico/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/Metaswitch/calico/git/refs{/sha}","trees_url":"https://api.github.com/repos/Metaswitch/calico/git/trees{/sha}","statuses_url":"https://api.github.com/repos/Metaswitch/calico/statuses/{sha}","languages_url":"https://api.github.com/repos/Metaswitch/calico/languages","stargazers_url":"https://api.github.com/repos/Metaswitch/calico/stargazers","contributors_url":"https://api.github.com/repos/Metaswitch/calico/contributors","subscribers_url":"https://api.github.com/repos/Metaswitch/calico/subscribers","subscription_url":"https://api.github.com/repos/Metaswitch/calico/subscription","commits_url":"https://api.github.com/repos/Metaswitch/calico/commits{/sha}","git_commits_url":"https://api.github.com/repos/Metaswitch/calico/git/commits{/sha}","comments_url":"https://api.github.com/repos/Metaswitch/calico/comments{/number}","issue_comment_url":"https://api.github.com/repos/Metaswitch/calico/issues/comments{/number}","contents_url":"https://api.github.com/repos/Metaswitch/calico/contents/{+path}","compare_url":"https://api.github.com/repos/Metaswitch/calico/compare/{base}...{head}","merges_url":"https://api.github.com/repos/Metaswitch/calico/merges","archive_url":"https://api.github.com/repos/Metaswitch/calico/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/Metaswitch/calico/downloads","issues_url":"https://api.github.com/repos/Metaswitch/calico/issues{/number}","pulls_url":"https://api.github.com/repos/Metaswitch/calico/pulls{/number}","milestones_url":"https://api.github.com/repos/Metaswitch/calico/milestones{/number}","notifications_url":"https://api.github.com/repos/Metaswitch/calico/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/Metaswitch/calico/labels{/name}","releases_url":"https://api.github.com/repos/Metaswitch/calico/releases{/id}","created_at":"2014-06-26T14:08:43Z","updated_at":"2015-04-20T19:30:41Z","pushed_at":"2015-04-20T19:52:17Z","git_url":"git://github.com/Metaswitch/calico.git","ssh_url":"git@github.com:Metaswitch/calico.git","clone_url":"https://github.com/Metaswitch/calico.git","svn_url":"https://github.com/Metaswitch/calico","homepage":"http://projectcalico.org/","size":9893,"stargazers_count":42,"watchers_count":42,"language":"Python","has_issues":true,"has_downloads":true,"has_wiki":false,"has_pages":false,"forks_count":11,"mirror_url":null,"open_issues_count":68,"forks":11,"open_issues":68,"watchers":42,"default_branch":"master"}},"_links":{"self":{"href":"https://api.github.com/repos/Metaswitch/calico/pulls/421"},"html":{"href":"https://github.com/Metaswitch/calico/pull/421"},"issue":{"href":"https://api.github.com/repos/Metaswitch/calico/issues/421"},"comments":{"href":"https://api.github.com/repos/Metaswitch/calico/issues/421/comments"},"review_comments":{"href":"https://api.github.com/repos/Metaswitch/calico/pulls/421/comments"},"review_comment":{"href":"https://api.github.com/repos/Metaswitch/calico/pulls/comments{/number}"},"commits":{"href":"https://api.github.com/repos/Metaswitch/calico/pulls/421/commits"},"statuses":{"href":"https://api.github.com/repos/Metaswitch/calico/statuses/59665bab13987d8b07110df9363f32e960a612b0"}}}}