kubernetes / sample-controller

Repository for sample controller. Complements sample-apiserver
Apache License 2.0
3.14k stars 1.08k forks source link

Did you mean "Spec" instead of "Status" #53

Closed mysterytony closed 5 years ago

mysterytony commented 5 years ago

https://github.com/kubernetes/sample-controller/blob/325dc0a18ed9c672755662fdcf9f0e71120ac30e/controller.go#L325-L330

Did you mean on L327, the "Spec"? But in the end you are not updating the Spec anyways, I suppose?

devdattakulkarni commented 5 years ago

@mysterytony No, the comment is correct. What it is referring to is the line 330. In that 'Update' method is used. The comment is saying that, instead of using the 'Update' method, more accurate would be to use the 'UpdateStatus' method since it is only the Status of the Custom Resource that we want to update. 'UpdateStatus' will update only the status and nothing else. With 'Update' it is programmer's responsibility to ensure that only the Status fields are updated and not any part of the Spec, even accidentally. However, for using 'UpdateStatus' the feature gate mentioned in the comment on line 326 needs to be enabled on the API Server. Ideally, the code could be written such that it checks if the gate is enabled and then decides whether to use 'Update' or 'UpdateStatus'. For now, the comment is referring to all of this and leaving the potential for improvement open.

mysterytony commented 5 years ago

@devdattakulkarni I see. Thank you!

Closing now.