{"action":"created","issue":{"url":"https://api.github.com/repos/kubernetes/kubernetes/issues/39164","repository_url":"https://api.github.com/repos/kubernetes/kubernetes","labels_url":"https://api.github.com/repos/kubernetes/kubernetes/issues/39164/labels{/name}","comments_url":"https://api.github.com/repos/kubernetes/kubernetes/issues/39164/comments","events_url":"https://api.github.com/repos/kubernetes/kubernetes/issues/39164/events","html_url":"https://github.com/kubernetes/kubernetes/pull/39164","id":197243974,"number":39164,"title":"Move NetworkPolicy to v1","user":{"login":"danwinship","id":96040,"avatar_url":"https://avatars3.githubusercontent.com/u/96040?v=3","gravatar_id":"","url":"https://api.github.com/users/danwinship","html_url":"https://github.com/danwinship","followers_url":"https://api.github.com/users/danwinship/followers","following_url":"https://api.github.com/users/danwinship/following{/other_user}","gists_url":"https://api.github.com/users/danwinship/gists{/gist_id}","starred_url":"https://api.github.com/users/danwinship/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/danwinship/subscriptions","organizations_url":"https://api.github.com/users/danwinship/orgs","repos_url":"https://api.github.com/users/danwinship/repos","events_url":"https://api.github.com/users/danwinship/events{/privacy}","received_events_url":"https://api.github.com/users/danwinship/received_events","type":"User","site_admin":false},"labels":[{"id":414883982,"url":"https://api.github.com/repos/kubernetes/kubernetes/labels/approved","name":"approved","color":"0ffa16","default":false},{"id":477397086,"url":"https://api.github.com/repos/kubernetes/kubernetes/labels/cncf-cla:%20yes","name":"cncf-cla: yes","color":"bfe5bf","default":false},{"id":261005360,"url":"https://api.github.com/repos/kubernetes/kubernetes/labels/kind/api-change","name":"kind/api-change","color":"c7def8","default":false},{"id":261005424,"url":"https://api.github.com/repos/kubernetes/kubernetes/labels/kind/new-api","name":"kind/new-api","color":"c7def8","default":false},{"id":349530192,"url":"https://api.github.com/repos/kubernetes/kubernetes/labels/release-note-label-needed","name":"release-note-label-needed","color":"db5a64","default":false},{"id":253450978,"url":"https://api.github.com/repos/kubernetes/kubernetes/labels/size/L","name":"size/L","color":"ee9900","default":false}],"state":"open","locked":false,"assignee":{"login":"erictune","id":8105590,"avatar_url":"https://avatars0.githubusercontent.com/u/8105590?v=3","gravatar_id":"","url":"https://api.github.com/users/erictune","html_url":"https://github.com/erictune","followers_url":"https://api.github.com/users/erictune/followers","following_url":"https://api.github.com/users/erictune/following{/other_user}","gists_url":"https://api.github.com/users/erictune/gists{/gist_id}","starred_url":"https://api.github.com/users/erictune/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/erictune/subscriptions","organizations_url":"https://api.github.com/users/erictune/orgs","repos_url":"https://api.github.com/users/erictune/repos","events_url":"https://api.github.com/users/erictune/events{/privacy}","received_events_url":"https://api.github.com/users/erictune/received_events","type":"User","site_admin":false},"assignees":[{"login":"thockin","id":5595220,"avatar_url":"https://avatars3.githubusercontent.com/u/5595220?v=3","gravatar_id":"","url":"https://api.github.com/users/thockin","html_url":"https://github.com/thockin","followers_url":"https://api.github.com/users/thockin/followers","following_url":"https://api.github.com/users/thockin/following{/other_user}","gists_url":"https://api.github.com/users/thockin/gists{/gist_id}","starred_url":"https://api.github.com/users/thockin/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/thockin/subscriptions","organizations_url":"https://api.github.com/users/thockin/orgs","repos_url":"https://api.github.com/users/thockin/repos","events_url":"https://api.github.com/users/thockin/events{/privacy}","received_events_url":"https://api.github.com/users/thockin/received_events","type":"User","site_admin":false},{"login":"erictune","id":8105590,"avatar_url":"https://avatars0.githubusercontent.com/u/8105590?v=3","gravatar_id":"","url":"https://api.github.com/users/erictune","html_url":"https://github.com/erictune","followers_url":"https://api.github.com/users/erictune/followers","following_url":"https://api.github.com/users/erictune/following{/other_user}","gists_url":"https://api.github.com/users/erictune/gists{/gist_id}","starred_url":"https://api.github.com/users/erictune/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/erictune/subscriptions","organizations_url":"https://api.github.com/users/erictune/orgs","repos_url":"https://api.github.com/users/erictune/repos","events_url":"https://api.github.com/users/erictune/events{/privacy}","received_events_url":"https://api.github.com/users/erictune/received_events","type":"User","site_admin":false}],"milestone":null,"comments":14,"created_at":"2016-12-22T18:57:08Z","updated_at":"2017-04-07T13:56:13Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/kubernetes/kubernetes/pulls/39164","html_url":"https://github.com/kubernetes/kubernetes/pull/39164","diff_url":"https://github.com/kubernetes/kubernetes/pull/39164.diff","patch_url":"https://github.com/kubernetes/kubernetes/pull/39164.patch"},"body":"Here's an initial attempt at moving NetworkPolicy to v1... this still needs work.\r\n\r\nAs discussed in the SIG, this changes the Ports and From arrays to be wrapped in an optional struct (which will be reflected in the json/yaml as well). That change is made in the unversioned and v1 versions of NetworkPolicy, but v1beta1 is left as-is (although with its comments fixed to reflect reality).\r\n\r\n(No generated files have been regenerated yet, to keep the diff small.)\r\n\r\nSome remaining questions:\r\n - [x] Do the v1beta1 comment changes correctly describe the way that existing NetworkPolicy implementations work?\r\n   - Yes\r\n - [x] ~~What should the unversioned→v1beta1 conversion do with policies containing an \"allow no ports\" or \"allow no peers\" rule? Currently it tries to represent that using a nil-vs-[] distinction, but that's obviously wrong since we're not documenting any such distinction any more (and the conversion to/from json won't respect that distinction). Should it return an error instead? (Presumably that would mean v1beta1 clients would get errors if they tried to fetch such policies?)~~\r\n   - (We dropped the API changes, so this no longer applies)\r\n - [ ] If we're breaking API (*although maybe we aren't any more*), are there any other changes to the structs that we want now that we've had some experience implementing it?\r\n   - eg, I found it weird that NetworkPolicyIngressRule.Ports describes destinations but NetworkPolicyIngressRule.From describes sources, particularly given that NetworkPolicySpec.PodSelector also describes destinations, meaning that destination information is split across two different parts of the policy, one of which can only be specified once, the other of which can be specified multiple times and is specified together with source information. (*I guess the explanation/counter-argument is that NetworkPolicySpec.PodSelector describes the pods that the policy applies to, and NetworkPolicySpec.Ingress describes the traffic that is allowed to those pods.*)\r\n   - It appears that no one implements named NetworkPolicyPort.Port values\r\n   - Does anyone implement the feature where you can specify a NetworkPolicyPort with a Protocol but no Port? (Meaning either \"allow connections to any TCP port, but not to UDP\" or \"allow connections to any UDP port, but not to TCP\"). (If we were going to drop this presumably we'd want to switch NetworkPolicyPort.Port to be non-optional and not a pointer.)\r\n - [ ] How does migration between the v1beta1 net.beta.kubernetes.io/network-policy Namespace annotation and the v1 Namespace.NetworkPolicy field work, given that there is not actually a version number change for Namespace? That is, when an existing NetworkPolicy user upgrades to a version of kubernetes with the v1 NetworkPolicy API, they will have Namespace objects that have a net.beta.kubernetes.io/network-policy annotation but a nil NetworkPolicy field. Should the network plugin check both the field and the annotation? Or should the kubernetes core \"default\" the value of the annotation into the new field? (There's no \"conversion\" here because the version of the Namespace object hasn't changed.) And if so, should we copy the value in the other direction (field to annotation) as well, so that v1beta1 NetworkPolicy clients and v1 NetworkPolicy clients will always agree on whether NetworkPolicy is in use for a Namespace? And if we do any of these things, when, if ever, can we stop doing them?\r\n- [ ] Should NetworkPolicy stay in pkg/apis/extensions? If not, where should it move? (Does the infrastructure support moving APIs between versions? Eg, conversions.)\r\n - [x] Do we want to simplify NamespaceNetworkPolicy at all? eg, it currently distinguishes between \"default network policy\" (`{ \"networkPolicy\": nil }`) \"default ingress network policy (but possibly non-default egress policy)\" (`{ \"networkPolicy\": { \"ingress\": nil } }`), and \"default ingress isolation policy (but possibly non-default policy for other aspects of ingress)\" (`{ \"networkPolicy\": { \"ingress\": { \"isolation\": nil } } }`). Do we still think NetworkPolicy will eventually describe things other than traffic isolation?\r\n   - consensus seems to be \"no, we don't want to simplify\"\r\n - [x] Where do we move docs/proposals/network-policy.md now that it's no longer a \"proposal\"? ~~Also, presumably we update it to refer only to the v1 API? (Do we need to worry about documenting the v1beta1 Namespace annotation somewhere since it won't show up anywhere in the generated API docs?)~~\r\n   - We leave the original proposal there, but add v1 docs somewhere else appropriate\r\n\r\n@kubernetes/sig-network-misc "},"comment":{"url":"https://api.github.com/repos/kubernetes/kubernetes/issues/comments/292543178","html_url":"https://github.com/kubernetes/kubernetes/pull/39164#issuecomment-292543178","issue_url":"https://api.github.com/repos/kubernetes/kubernetes/issues/39164","id":292543178,"user":{"login":"k8s-ci-robot","id":20407524,"avatar_url":"https://avatars3.githubusercontent.com/u/20407524?v=3","gravatar_id":"","url":"https://api.github.com/users/k8s-ci-robot","html_url":"https://github.com/k8s-ci-robot","followers_url":"https://api.github.com/users/k8s-ci-robot/followers","following_url":"https://api.github.com/users/k8s-ci-robot/following{/other_user}","gists_url":"https://api.github.com/users/k8s-ci-robot/gists{/gist_id}","starred_url":"https://api.github.com/users/k8s-ci-robot/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/k8s-ci-robot/subscriptions","organizations_url":"https://api.github.com/users/k8s-ci-robot/orgs","repos_url":"https://api.github.com/users/k8s-ci-robot/repos","events_url":"https://api.github.com/users/k8s-ci-robot/events{/privacy}","received_events_url":"https://api.github.com/users/k8s-ci-robot/received_events","type":"User","site_admin":false},"created_at":"2017-04-07T13:56:13Z","updated_at":"2017-04-07T13:56:13Z","body":"@danwinship: The following test(s) **failed**:\n\nTest name | Commit | Details | Rerun command\n--- | --- | --- | ---\nJenkins verification | edd71202da2b88aa69bc53b6ffabb18e7547c3d0 | [link](https://k8s-gubernator.appspot.com/build/kubernetes-jenkins/pr-logs/pull/39164/pull-kubernetes-verify/21835/) | `@k8s-bot verify test this`\nJenkins Bazel Build | e4a3b0f68072614562079a89265391c4e8278773 | [link](https://k8s-gubernator.appspot.com/build/kubernetes-jenkins/pr-logs/pull/39164/pull-kubernetes-bazel/12631/) | `@k8s-bot bazel test this`\nJenkins GCE e2e | e4a3b0f68072614562079a89265391c4e8278773 | [link](https://k8s-gubernator.appspot.com/build/kubernetes-jenkins/pr-logs/pull/39164/pull-kubernetes-e2e-gce/25532/) | `@k8s-bot cvm gce e2e test this`\nJenkins unit/integration | e4a3b0f68072614562079a89265391c4e8278773 | [link](https://k8s-gubernator.appspot.com/build/kubernetes-jenkins/pr-logs/pull/39164/pull-kubernetes-unit/25533/) | `@k8s-bot unit test this`\nJenkins GCE etcd3 e2e | e4a3b0f68072614562079a89265391c4e8278773 | [link](https://k8s-gubernator.appspot.com/build/kubernetes-jenkins/pr-logs/pull/39164/pull-kubernetes-e2e-gce-etcd3/25102/) | `@k8s-bot gce etcd3 e2e test this`\nJenkins kops AWS e2e | e4a3b0f68072614562079a89265391c4e8278773 | [link](https://k8s-gubernator.appspot.com/build/kubernetes-jenkins/pr-logs/pull/39164/pull-kubernetes-e2e-kops-aws/19951/) | `@k8s-bot kops aws e2e test this`\nJenkins GCI GCE e2e | e4a3b0f68072614562079a89265391c4e8278773 | [link](https://k8s-gubernator.appspot.com/build/kubernetes-jenkins/pr-logs/pull/39164/pull-kubernetes-e2e-gce-gci/24866/) | `@k8s-bot gci gce e2e test this`\n\n[Full PR test history](https://k8s-gubernator.appspot.com/pr/39164). [Your PR dashboard](https://k8s-gubernator.appspot.com/pr/danwinship). Please help us cut down on flakes by linking to an [open issue](https://github.com/kubernetes/kubernetes/issues?q=is:issue+is:open) when you hit one in your PR.\n\n<details>\n\nInstructions for interacting with me using PR comments are available [here](https://github.com/kubernetes/community/blob/master/contributors/devel/pull-request-commands.md).  If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository. I understand the commands that are listed [here](https://github.com/kubernetes/test-infra/blob/master/prow/commands.md).\n</details>\n<!-- test report -->"}}
{"actor":{"display_login":"k8s-ci-robot"}}