{"action":"created","issue":{"url":"https://api.github.com/repos/Mirantis/virtlet/issues/261","repository_url":"https://api.github.com/repos/Mirantis/virtlet","labels_url":"https://api.github.com/repos/Mirantis/virtlet/issues/261/labels{/name}","comments_url":"https://api.github.com/repos/Mirantis/virtlet/issues/261/comments","events_url":"https://api.github.com/repos/Mirantis/virtlet/issues/261/events","html_url":"https://github.com/Mirantis/virtlet/pull/261","id":222223952,"number":261,"title":"Update Virtlet and CRI proxy for k8s 1.6","user":{"login":"ivan4th","id":190448,"avatar_url":"https://avatars1.githubusercontent.com/u/190448?v=3","gravatar_id":"","url":"https://api.github.com/users/ivan4th","html_url":"https://github.com/ivan4th","followers_url":"https://api.github.com/users/ivan4th/followers","following_url":"https://api.github.com/users/ivan4th/following{/other_user}","gists_url":"https://api.github.com/users/ivan4th/gists{/gist_id}","starred_url":"https://api.github.com/users/ivan4th/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/ivan4th/subscriptions","organizations_url":"https://api.github.com/users/ivan4th/orgs","repos_url":"https://api.github.com/users/ivan4th/repos","events_url":"https://api.github.com/users/ivan4th/events{/privacy}","received_events_url":"https://api.github.com/users/ivan4th/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":1,"created_at":"2017-04-17T20:33:53Z","updated_at":"2017-04-20T12:45:36Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/Mirantis/virtlet/pulls/261","html_url":"https://github.com/Mirantis/virtlet/pull/261","diff_url":"https://github.com/Mirantis/virtlet/pull/261.diff","patch_url":"https://github.com/Mirantis/virtlet/pull/261.patch"},"body":"- [x] Make it possible to start Virtlet on k8s 1.6 in semi-manual mode (1)\r\n- [x] Add hack for `\\\"enforceNodeAllocatable\\\":[]` in case of DIND (2) \r\n- [x] Fix `nodeAffinity` - it's being ignored in daemonset definition\r\n- [x] Add RBAC defs to yamls (non-permissive version)\r\n- [x] Fix DNS in Virtlet pod\r\n- [x] Fix kubelet restart during criproxy bootstrap (3)\r\n- [x] Add test for ImageRef in PullImage response (4)\r\n- [x] Add test for Metadata.Name in ContainerStatus response (5)\r\n- [x] Fix flexvolume\r\n- [x] Fix tests\r\n- [x] Fix CI\r\n\r\nDetails\r\n\r\n1. After initial impl the following extra steps need to be done, besides ones described in `running-local-environment.md`:\r\n```\r\n# after bringing up the cluster, before applying Virtlet yaml\r\n# set up RBAC\r\nkubectl create clusterrolebinding permissive-binding \\\r\n   --clusterrole=cluster-admin \\\r\n   --user=admin \\\r\n   --user=kubelet \\\r\n   --group=system:serviceaccounts\r\n# delete kube-node-2 because of nodeAffinity problems\r\nkubectl delete node kube-node-2\r\n```\r\nAfter creating Virtlet daemonset, enter `kube-node-1` container and kill `sleep Infinity` there. After starting imager-server/image-service, take service IP from `kubectl describe -n kube-system image-service` and put it instead of `image-service.kube-system` in cirros-vm.yaml.\r\n\r\n2. There's bug in kubelet 1.6, it doesn't serialize empty `enforceNodeAllocatable` array currently employed in DIND cgroup workaround for 1.6 when responding to `/configz` request. Currently setting empty `enforceNodeAllocatable` is hardcoded in CRI proxy bootstrap but it should only be done for DIND.\r\n\r\n3. As of now kubelet defaults to CRI, so CRI proxy bootstrap container doesn't die during kubelet restart, so it keeps sitting in `sleep Infinity`.  It should detect kubelet restart properly instead.\r\n\r\n4. That's an API change in k8s 1.6, the code is already fixed but tests are not updated yet. This should be tested for both criproxy and Virtlet itself.\r\n\r\n5. This part of API didn't really change but extra validation was added on kubelet side, so Virtlet started failing checks during `CreateContainer`. The code is already fixed but need to update tests.\r\n\r\n<!-- Reviewable:start -->\r\n---\r\nThis change is [<img src=\"https://reviewable.io/review_button.svg\" height=\"34\" align=\"absmiddle\" alt=\"Reviewable\"/>](https://reviewable.io/reviews/mirantis/virtlet/261)\r\n<!-- Reviewable:end -->\r\n"},"comment":{"url":"https://api.github.com/repos/Mirantis/virtlet/issues/comments/295723948","html_url":"https://github.com/Mirantis/virtlet/pull/261#issuecomment-295723948","issue_url":"https://api.github.com/repos/Mirantis/virtlet/issues/261","id":295723948,"user":{"login":"ivan4th","id":190448,"avatar_url":"https://avatars1.githubusercontent.com/u/190448?v=3","gravatar_id":"","url":"https://api.github.com/users/ivan4th","html_url":"https://github.com/ivan4th","followers_url":"https://api.github.com/users/ivan4th/followers","following_url":"https://api.github.com/users/ivan4th/following{/other_user}","gists_url":"https://api.github.com/users/ivan4th/gists{/gist_id}","starred_url":"https://api.github.com/users/ivan4th/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/ivan4th/subscriptions","organizations_url":"https://api.github.com/users/ivan4th/orgs","repos_url":"https://api.github.com/users/ivan4th/repos","events_url":"https://api.github.com/users/ivan4th/events{/privacy}","received_events_url":"https://api.github.com/users/ivan4th/received_events","type":"User","site_admin":false},"created_at":"2017-04-20T12:45:36Z","updated_at":"2017-04-20T12:45:36Z","body":"\n\n\n\nReview status: all files reviewed at latest revision, 4 unresolved discussions, some commit checks failed.\n\n---\n\n*[pkg/criproxy/bootstrap.go, line 43 at r1](https://reviewable.io:443/reviews/mirantis/virtlet/261#-Ki9NUbNW9V7fNeDb1M5:-KiANz9DmnAayfE4L-yU:bkhcd3h) ([raw file](https://github.com/mirantis/virtlet/blob/8e2732adb97ea88b184d6c815cd72c541edabb4c/pkg/criproxy/bootstrap.go#L43)):*\n<details><summary><i>Previously, jellonek (Piotr Skamruk) wrote…</i></summary><blockquote>\n\nThis comment seems to be outdated ;)\n</blockquote></details>\n\nWill remove\n\n---\n\n*[pkg/criproxy/bootstrap.go, line 169 at r1](https://reviewable.io:443/reviews/mirantis/virtlet/261#-Ki9Nk_rfgvGR90AQVVM:-KiAO2VZMFIKt7GAI4Pm:bhy9q7g) ([raw file](https://github.com/mirantis/virtlet/blob/8e2732adb97ea88b184d6c815cd72c541edabb4c/pkg/criproxy/bootstrap.go#L169)):*\n<details><summary><i>Previously, jellonek (Piotr Skamruk) wrote…</i></summary><blockquote>\n\nMessage should start from capital letter.\n</blockquote></details>\n\nWill fix\n\n---\n\n*[pkg/criproxy/proxy.go, line 329 at r1](https://reviewable.io:443/reviews/mirantis/virtlet/261#-Ki9OoKEaC0p5LBK3g-M:-KiAO4Q3VDMPk2-em0iu:b97cpr1) ([raw file](https://github.com/mirantis/virtlet/blob/8e2732adb97ea88b184d6c815cd72c541edabb4c/pkg/criproxy/proxy.go#L329)):*\n<details><summary><i>Previously, jellonek (Piotr Skamruk) wrote…</i></summary><blockquote>\n\nIf now it's graceful - is above comment still correct?\n</blockquote></details>\n\nWill remove the comment\n\n---\n\n*[pkg/criproxy/proxy_test.go, line 1145 at r1](https://reviewable.io:443/reviews/mirantis/virtlet/261#-Ki9PbPko_Tv0gJl0oAL:-KiAO6CPPe1H0VhDXWUu:b-wdoj4k) ([raw file](https://github.com/mirantis/virtlet/blob/8e2732adb97ea88b184d6c815cd72c541edabb4c/pkg/criproxy/proxy_test.go#L1145)):*\n<details><summary><i>Previously, jellonek (Piotr Skamruk) wrote…</i></summary><blockquote>\n\nIf it's race, can we wait for criproxy longer, instead of skipping these tests?\n</blockquote></details>\n\nNo, it's more complex than that. Waiting doesn't help, the problem is that `ListContainers()` should never fail when 2nd CRI impl is not connected yet, but under some circumstances it still may fail when it's being connected to. We'll need to fix this at some point -- will need to make an issue for this.\n\n---\n\n\n*Comments from [Reviewable](https://reviewable.io:443/reviews/mirantis/virtlet/261)*\n<!-- Sent from Reviewable.io -->\n"}}
{"actor":{"display_login":"ivan4th"}}