this was an instance of the problems with "derived defaults" described in https://github.com/kubernetes/community/blob/master/contributors/devel/sig-architecture/api-conventions.md#static-defaults:

In some cases, these values may be conditional on or deterministically derived from other fields (e.g. "if otherField is X then this field defaults to 0" or "this field defaults to the value of otherField"). Note that such derived defaults present a hazard in the face of updates - if the "other" field changes, the derived field may have to change, too. The static defaulting logic is unaware of updates and has no concept of "previous value", which means this inter-field relationship becomes the user's problem - they must update both the field they care about and the "other" field.

so changing it (by moving the defaulting to the pod, which is immutable on update) seems useful if we can swing it

The immediately mechanical issue I see is that it looks like validation requires hostPort == containerPort when hostNetwork == true, even at the deployment, etc, level:

• ValidatePodTemplateSpec → ValidatePodSpec → ValidatePodSecurityContext → validateHostNetwork:

  kubernetes/pkg/apis/core/validation/validation.go

  Lines 3484 to 3492 in f871d5f

  	func validateHostNetwork(hostNetwork bool, containers []core.Container, fldPath *field.Path) field.ErrorList {
  	allErrors := field.ErrorList{}
  	if hostNetwork {
  	for i, container := range containers {
  	portsPath := fldPath.Index(i).Child("ports")
  	for i, port := range container.Ports {
  	idxPath := portsPath.Index(i)
  	if port.HostPort != port.ContainerPort {
  	allErrors = append(allErrors, field.Invalid(idxPath.Child("containerPort"), port.ContainerPort, "must match `hostPort` when `hostNetwork` is true"))

• How is this validation passing on deployments that set hostNetwork: true but rely on defaulting to populate hostPort in all the container ports? Do we not have tests for that?

More esoteric implications of this:

1. a client that sends Update requests for deployments, etc, will no longer get hostPort fields defaulted inside containers. That means an update to an object persisted in a previous release you would expect to be a no-op will now trigger a generation change and rollout, even though the resulting pods would be identical.
2. pod-creating controllers (replicaset, job, cronjob, statefulset, daemonset) have to tolerate defaulting differences between the pod-spawning object and the pods without triggering rollouts to try to make the pods exactly match the workload spec. I think that already has to be generally true since we already have pod-specific defaulting, but I've been surprised before, which led to Add tests to detect default changes to podspec and podspectemplate defaults #78914, which I don't think we've ever shored up this aspect to verify controllers are well-behaved.

it looks like validation requires hostPort == containerPort when hostNetwork == true

And it seems there are no tests for this case. I can fix that.

a client that sends Update requests for deployments, etc, will no longer get hostPort fields defaulted inside containers. That means an update to an object persisted in a previous release you would expect to be a no-op will now trigger a generation change and rollout, even though the resulting pods would be identical.

Not sure I follow this one. A Deployment which already exists has these fields set. We won't unset them.

have to tolerate defaulting differences between the pod-spawning object and the pods without triggering rollouts to try to make the pods exactly match the workload spec

I'll look into this, too. I kicked this PR off to see if it was easy enough to squeeze into free time. It may not be :)

a client that sends Update requests for deployments, etc, will no longer get hostPort fields defaulted inside containers. That means an update to an object persisted in a previous release you would expect to be a no-op will now trigger a generation change and rollout, even though the resulting pods would be identical.

Not sure I follow this one. A Deployment which already exists has these fields set. We won't unset them.

When the client sends an update of {kind:Deployment, spec: {template: {hostNetwork: true, ...}}}, if we no longer default hostPort inside all the container ports, the update would not have those fields populated in the new object on an update. DeepEqual(newDeployment.spec.template, oldDeployment.spec.template) would show a diff, generation would get bumped, a new replicaset/etc would roll out

the update would not have those fields populated in the new object on an update

If they did a full replace (PUT) then yes, you are right. But I am not sure on PATCH - we're into the area of patch/apply where I get fuzzy.

If user sent an update which changed (e.g.) Deployment labels (which should not trigger a redeploy), wouldn't the patch/apply logic carry the hostPort values forward?

yes, a patch not touching .spec.template... should leave the defaulted fields alone

Pushed with validation tests. Will need to look at the rest later

I think that already has to be generally true since we already have pod-specific defaulting, but I've been surprised before, which led to #78914, which I don't think we've ever shored up this aspect to verify controllers are well-behaved.

Yeah That test doesn't handle conditional defaults. I'll have to comprehend the tangle behind that thing.

oof. I think I get it, but it's tricky - it detects a slice with a value as "already defaulted". I'm pushing an update now - PTAL. Still need to verify manually that controllers don't hotloop on this.

I verified by inspection DaemonSet, ReplicaSet, StatefulSet, and Job. I also ran a Deployment under this config and verified that Deployment's hostPort is not set, the pod's hostPort is set, and pods were not being recreated.

oh, this is much better than current method we were doing

I see #117696 (comment)

Yeah, I didn't realize this was an option either :)