{"action":"created","issue":{"url":"https://api.github.com/repos/kubernetes/kubernetes/issues/16543","labels_url":"https://api.github.com/repos/kubernetes/kubernetes/issues/16543/labels{/name}","comments_url":"https://api.github.com/repos/kubernetes/kubernetes/issues/16543/comments","events_url":"https://api.github.com/repos/kubernetes/kubernetes/issues/16543/events","html_url":"https://github.com/kubernetes/kubernetes/issues/16543","id":114146928,"number":16543,"title":"Determine how apply annotation should be serialized","user":{"login":"jackgr","id":5607565,"avatar_url":"https://avatars.githubusercontent.com/u/5607565?v=3","gravatar_id":"","url":"https://api.github.com/users/jackgr","html_url":"https://github.com/jackgr","followers_url":"https://api.github.com/users/jackgr/followers","following_url":"https://api.github.com/users/jackgr/following{/other_user}","gists_url":"https://api.github.com/users/jackgr/gists{/gist_id}","starred_url":"https://api.github.com/users/jackgr/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/jackgr/subscriptions","organizations_url":"https://api.github.com/users/jackgr/orgs","repos_url":"https://api.github.com/users/jackgr/repos","events_url":"https://api.github.com/users/jackgr/events{/privacy}","received_events_url":"https://api.github.com/users/jackgr/received_events","type":"User","site_admin":false},"labels":[{"url":"https://api.github.com/repos/kubernetes/kubernetes/labels/area/app-config-deployment","name":"area/app-config-deployment","color":"fb6420"},{"url":"https://api.github.com/repos/kubernetes/kubernetes/labels/priority/P2","name":"priority/P2","color":"fbca04"},{"url":"https://api.github.com/repos/kubernetes/kubernetes/labels/team/ux","name":"team/ux","color":"d2b48c"}],"state":"open","locked":false,"assignee":null,"milestone":null,"comments":15,"created_at":"2015-10-29T21:04:54Z","updated_at":"2015-11-16T17:57:57Z","closed_at":null,"body":"Forked from #16441, starting at [this comment](https://github.com/kubernetes/kubernetes/pull/16441#issuecomment-151935611).\r\n\r\nTo summarize, when creating the apply annotation in [pkg/kubectl/cmd/apply.go](https://github.com/kubernetes/kubernetes/blob/master/pkg/kubectl/cmd/apply.go), we are serializing the object client side using the mapping codec. We are also deserializing it client side in [pkg/kubectl/apply.go](https://github.com/kubernetes/kubernetes/blob/master/pkg/kubectl/apply.go) to retrieve the previous version for the three way diff. \r\n\r\nThe question being discussed is whether or not this code is serializing and deserializing the object in a way that deals correctly with the api version and defaults.\r\n\r\ncc @bgrant0607, @liggitt, @janetkuo"},"comment":{"url":"https://api.github.com/repos/kubernetes/kubernetes/issues/comments/157119762","html_url":"https://github.com/kubernetes/kubernetes/issues/16543#issuecomment-157119762","issue_url":"https://api.github.com/repos/kubernetes/kubernetes/issues/16543","id":157119762,"user":{"login":"liggitt","id":980082,"avatar_url":"https://avatars.githubusercontent.com/u/980082?v=3","gravatar_id":"","url":"https://api.github.com/users/liggitt","html_url":"https://github.com/liggitt","followers_url":"https://api.github.com/users/liggitt/followers","following_url":"https://api.github.com/users/liggitt/following{/other_user}","gists_url":"https://api.github.com/users/liggitt/gists{/gist_id}","starred_url":"https://api.github.com/users/liggitt/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/liggitt/subscriptions","organizations_url":"https://api.github.com/users/liggitt/orgs","repos_url":"https://api.github.com/users/liggitt/repos","events_url":"https://api.github.com/users/liggitt/events{/privacy}","received_events_url":"https://api.github.com/users/liggitt/received_events","type":"User","site_admin":false},"created_at":"2015-11-16T17:57:57Z","updated_at":"2015-11-16T17:57:57Z","body":"> When the patch in step 3 is applied, host will be deleted. IIUC, it will be set some time later, based on the value the user set in requestedHost. Is this an appropriate outcome, or would it cause problems (e.g., scheduling problems due to the removal of a required field)?\r\n\r\nIt's a hypothetical example, so I'm not sure :) The main point was that `apply` has to be version-aware to correctly determine which fields are user-specified when working across versions.\r\n\r\n> When #17238 is implemented, kubectl apply will pass false for the force flag argument to CreateStrategicMergePatch. As a result, the change submitted by the user in step 3 will fail with a conflict, since it deletes host, which was populated by a controller.\r\n\r\nHow does `apply` know `host` was populated by a controller?\r\n\r\n> If the new host and port properties populated by a controller are mandatory in the api server, then any attempt to delete them will fail. IMO, we should prevent the user from deleting properties required for correct function.\r\n\r\nAgain, hypothetical example, but host/port probably wouldn't be required if Foo could be created without them set and they got populated later by a controller.\r\n\r\n> An alternate solution would be to always delete and recreate when the version changes.\r\n\r\nIf you mean delete/recreate the apply annotation, that's actually what I originally thought was being proposed (see https://github.com/kubernetes/kubernetes/issues/16543#issuecomment-156768784)\r\n\r\n> It seems like a bad idea to define new fields maintained by controllers, in this case host and port, using now deprecated field names once exposed to users.\r\n\r\nVersioning the API lets us make changes like that safely, using defined conversions. Putting constraints on field name interactions between API versions, to allow other components to ignore API versioning does not seem like a good precedent. In cases where a single field was serving two purposes, and a future API version wants to address those purposes using two distinct fields, using the old field name for one of the purposes seems very reasonable.\r\n\r\n> perhaps to always delete and recreate when the version changes as an additional measure of assurance\r\n\r\njust to clarify, you're thinking of deleting/recreating the apply annotation, not the whole object, right?"}}