payload: {"action":"created","issue":{"url":"https://api.github.com/repos/ceph/ceph-iscsi/issues/63","repository_url":"https://api.github.com/repos/ceph/ceph-iscsi","labels_url":"https://api.github.com/repos/ceph/ceph-iscsi/issues/63/labels{/name}","comments_url":"https://api.github.com/repos/ceph/ceph-iscsi/issues/63/comments","events_url":"https://api.github.com/repos/ceph/ceph-iscsi/issues/63/events","html_url":"https://github.com/ceph/ceph-iscsi/pull/63","id":429407881,"node_id":"MDExOlB1bGxSZXF1ZXN0MjY3NTUxODY5","number":63,"title":"Fix deletion of gateways with multiple IPs","user":{"login":"ricardoasmarques","id":14297426,"node_id":"MDQ6VXNlcjE0Mjk3NDI2","avatar_url":"https://avatars2.githubusercontent.com/u/14297426?v=4","gravatar_id":"","url":"https://api.github.com/users/ricardoasmarques","html_url":"https://github.com/ricardoasmarques","followers_url":"https://api.github.com/users/ricardoasmarques/followers","following_url":"https://api.github.com/users/ricardoasmarques/following{/other_user}","gists_url":"https://api.github.com/users/ricardoasmarques/gists{/gist_id}","starred_url":"https://api.github.com/users/ricardoasmarques/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/ricardoasmarques/subscriptions","organizations_url":"https://api.github.com/users/ricardoasmarques/orgs","repos_url":"https://api.github.com/users/ricardoasmarques/repos","events_url":"https://api.github.com/users/ricardoasmarques/events{/privacy}","received_events_url":"https://api.github.com/users/ricardoasmarques/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":5,"created_at":"2019-04-04T17:54:04Z","updated_at":"2019-04-11T19:43:52Z","closed_at":null,"author_association":"MEMBER","pull_request":{"url":"https://api.github.com/repos/ceph/ceph-iscsi/pulls/63","html_url":"https://github.com/ceph/ceph-iscsi/pull/63","diff_url":"https://github.com/ceph/ceph-iscsi/pull/63.diff","patch_url":"https://github.com/ceph/ceph-iscsi/pull/63.patch"},"body":"PR https://github.com/ceph/ceph-iscsi/pull/51 introduced the support for deleting a gateway but doesn't consider the case where the gateway was created with multiple IPs.\r\n\r\nThis PRs will guarantee that when deleting a gateway, all IPs of that gateway are removed.\r\n\r\n### How to reproduce:\r\n\r\nCreate a target:\r\n`curl --insecure --user admin:admin -X PUT https://192.168.100.201:5001/api/target/iqn.2003-01.com.redhat.iscsi-gw0`\r\n\r\nCreate one gateway with a single IP:\r\n`curl --insecure --user admin:admin -d ip_address=192.168.100.201 -dskipchecks=true -X PUT https://192.168.100.201:5001/api/gateway/iqn.2003-01.com.redhat.iscsi-gw0/node1`\r\n\r\nCreate another gateway with **two IPs** separated by a ',':\r\n`curl --insecure --user admin:admin -d ip_address=192.168.100.202,192.168.121.120 -dskipchecks=true -X PUT https://192.168.100.201:5001/api/gateway/iqn.2003-01.com.redhat.iscsi-gw0/node2`\r\n\r\nDelete the second gateway:\r\n`curl --insecure --user admin:admin -X DELETE https://192.168.100.201:5001/api/gateway/iqn.2003-01.com.redhat.iscsi-gw0/node2`\r\n\r\nOne of the IPs from the second gateway will remain in the configuration:\r\n\r\n```\r\n{\r\n  \"created\": \"2019/04/04 17:32:35\",\r\n  \"discovery_auth\": {\r\n    \"mutual_password\": \"\",\r\n    \"mutual_password_encryption_enabled\": false,\r\n    \"mutual_username\": \"\",\r\n    \"password\": \"\",\r\n    \"password_encryption_enabled\": false,\r\n    \"username\": \"\"\r\n  },\r\n  \"disks\": {},\r\n  \"epoch\": 4,\r\n  \"gateways\": {\r\n    \"node1\": {\r\n      \"active_luns\": 0,\r\n      \"created\": \"2019/04/04 17:34:51\",\r\n      \"updated\": \"2019/04/04 17:34:51\"\r\n    }\r\n  },\r\n  \"targets\": {\r\n    \"iqn.2003-01.com.redhat.iscsi-gw0\": {\r\n      \"acl_enabled\": true,\r\n      \"clients\": {},\r\n      \"controls\": {},\r\n      \"created\": \"2019/04/04 17:34:46\",\r\n      \"disks\": [],\r\n      \"groups\": {},\r\n      \"ip_list\": [\r\n        \"192.168.100.201\",\r\n        \"192.168.100.202\"\r\n      ],\r\n      \"portals\": {\r\n        \"node1\": {\r\n          \"gateway_ip_list\": [\r\n            \"192.168.100.201\",\r\n            \"192.168.100.202\"\r\n          ],\r\n          \"inactive_portal_ips\": [\r\n            \"192.168.100.202\"\r\n          ],\r\n          \"portal_ip_address\": \"192.168.100.201\",\r\n          \"tpgs\": 2\r\n        }\r\n      },\r\n      \"updated\": \"2019/04/04 17:35:05\"\r\n    }\r\n  },\r\n  \"updated\": \"2019/04/04 17:35:06\",\r\n  \"version\": 8\r\n}\r\n```\r\n\r\nAnd if we also delete the first gateway:\r\n\r\n`curl --insecure --user admin:admin -X DELETE https://192.168.100.201:5001/api/gateway/iqn.2003-01.com.redhat.iscsi-gw0/node1`\r\n\r\n\r\nThe \"lost\" IP from the second gateway still remains in the configuration:\r\n\r\n```\r\n{\r\n  \"created\": \"2019/04/04 17:32:35\",\r\n  \"discovery_auth\": {\r\n    \"mutual_password\": \"\",\r\n    \"mutual_password_encryption_enabled\": false,\r\n    \"mutual_username\": \"\",\r\n    \"password\": \"\",\r\n    \"password_encryption_enabled\": false,\r\n    \"username\": \"\"\r\n  },\r\n  \"disks\": {},\r\n  \"epoch\": 5,\r\n  \"gateways\": {},\r\n  \"targets\": {\r\n    \"iqn.2003-01.com.redhat.iscsi-gw0\": {\r\n      \"acl_enabled\": true,\r\n      \"clients\": {},\r\n      \"controls\": {},\r\n      \"created\": \"2019/04/04 17:34:46\",\r\n      \"disks\": [],\r\n      \"groups\": {},\r\n      \"ip_list\": [\r\n        \"192.168.100.202\"\r\n      ],\r\n      \"portals\": {},\r\n      \"updated\": \"2019/04/04 17:35:29\"\r\n    }\r\n  },\r\n  \"updated\": \"2019/04/04 17:35:29\",\r\n  \"version\": 8\r\n}\r\n```\r\n\r\nSigned-off-by: Ricardo Marques <rimarques@suse.com>"},"comment":{"url":"https://api.github.com/repos/ceph/ceph-iscsi/issues/comments/482278020","html_url":"https://github.com/ceph/ceph-iscsi/pull/63#issuecomment-482278020","issue_url":"https://api.github.com/repos/ceph/ceph-iscsi/issues/63","id":482278020,"node_id":"MDEyOklzc3VlQ29tbWVudDQ4MjI3ODAyMA==","user":{"login":"mikechristie","id":1063316,"node_id":"MDQ6VXNlcjEwNjMzMTY=","avatar_url":"https://avatars2.githubusercontent.com/u/1063316?v=4","gravatar_id":"","url":"https://api.github.com/users/mikechristie","html_url":"https://github.com/mikechristie","followers_url":"https://api.github.com/users/mikechristie/followers","following_url":"https://api.github.com/users/mikechristie/following{/other_user}","gists_url":"https://api.github.com/users/mikechristie/gists{/gist_id}","starred_url":"https://api.github.com/users/mikechristie/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/mikechristie/subscriptions","organizations_url":"https://api.github.com/users/mikechristie/orgs","repos_url":"https://api.github.com/users/mikechristie/repos","events_url":"https://api.github.com/users/mikechristie/events{/privacy}","received_events_url":"https://api.github.com/users/mikechristie/received_events","type":"User","site_admin":false},"created_at":"2019-04-11T19:43:52Z","updated_at":"2019-04-11T19:43:52Z","author_association":"CONTRIBUTOR","body":"> > I didn't know multiple IPs was supported. What commit was it added in? Did it come in with the multiple target support?\r\n> > I think some other stuff is broken or might just be sort-of working by accident.\r\n> \r\n> This was already available before the support for multiple targets, but it's curious that you cannot do this using `gwcli`, you have to call the API directly so maybe this \"feature\" was not intentionally implemented.\r\n> \r\n\r\nIt was not intentional for what we currently support so that is why we never added gwcli support. It was probably something we did early on when we were doing single node non target_core_user based support originally. We then later dropped that, but we never cleaned up code like this.\r\n\r\nThe problem is that some SCSI commands require the port info, but target_core_user and tcmu-runner do not have it since they are too far up the stack. We were limiting it to one enabled tpg with one portal per node for target_core_user/runner setups, because we then know the command could only come from the enabled one.\r\n\r\n> Shouldn't we change the `gwcli` gateway creation command to allow the user to specify multiple IPs, for consistency with the API?\r\n\r\nIt might be a little more difficult. Other changes might be needed to fully support it with target_core_user.\r\n\r\nFor multiple enabled tpgs support we need kernel and tcmu-runner changes.\r\n\r\nMultiple portals per tpg might work, because we do not support and initiators have not required support for any type of command that would require us to return or use info for a specific portal in a tpg. For example, the relative port id would be the same for all portals in that tpg so we would be ok for that command. There are commands that might specify/require the ip address of a portal, but we do not support them. I am still checking the specs. And, there also might be a bug with some versions of windows where if you return the same relative port id for a LUN accessed through different iscsi sessions, then windows will crash."}}
other: {"actor":{"display_login":"mikechristie"}}