KEP-3085: promote to beta in 1.29

[kannon92]

• One-line PR description: Promote KEP 3085 to beta in 1.29.

• Issue link: [KEP-3085] Add condition for sandbox creation (xposted from original issue) #4138

• Other comments:

[bart0sh]

/lgtm
/assign @mrunalp @dchen1107 @derekwaynecarr

[kannon92]

/assign @ddebroy

[kannon92]

/cc @rphillips @haircommander

[BenTheElder]

/assign

[jeremyrickard]

The PRR for this needs to be updated a little bit for beta. There is one section defined under scalability that needs to be added to the KEP (https://github.com/kubernetes/enhancements/blob/master/keps/NNNN-kep-template/README.md#can-enabling--using-this-feature-result-in-resource-exhaustion-of-some-node-resources-pids-sockets-inodes-etc).

While reviewing the previous PRR template, I had a few questions. I can't comment since you didn't update the PRR template, so I'll ask below:

How can a rollout or rollback fail? Can it impact already running workloads?

This flag is only relevant for the Kubelet. Therefore, the new condition will be
reported for pods scheduled on nodes that have the feature enabled.
A controller or service that consumes the new pod condition should be enabled
only after rollout of the new condition has succeeded on all nodes. Similarly,
the controller or service that consumes the new pod condition should be disabled
before the rollback. This helps prevent a controller/service consuming the
condition getting the data from pods running in a subset of the nodes in the
middle of a rollout or rollback.

What happens if someone doesn't do the "should" statements above? When controllers that consume the condition get data in the middle of a rollout or rollback is there a way to determine this or are there any ways to help determine this?

What specific metrics should inform a rollback?

A sharp increase in the number of PATCH requests to API Server from Kubelets
after enabling this feature is a sign of potential problem and can inform a
rollback. A cluster operator may monitor

apiserver_request_total{verb="PATCH", resource="pods", subresource="status"}

for this.
This may be the case in clusters that use a special runtime environment like
microVM/Kata, where the sandbox may crash repeatedly (without ever getting a
chance to start containers) resulting in lots of potential updates due to the
new condition "flapping". However, in such environments, this may already be the
case with existing pod conditions like ContainersReady and Ready (unless the
sandbox environment/VM crashes very early before a single container is run).
Batching of pod status updates from the Kubelet status manager will also help.

Could we better define a sharp increase? This seems like a bad metric for those other enviroments. For people operating clusters with those special runtime environments, is there any way to know that this feature is functioning properly?

How can someone using this feature know that it is working for their instance?

The check of PodReadyToStartContainers seems good for the case where things are working correctly. If this is not set, is this only because there are failures updating etcd? The troubleshooting section below doesn't call out any additional failure modes, are there any other ways that could fail that were identified while this was in the alpha state?

[kannon92]

The PRR for this needs to be updated a little bit for beta. There is one section defined under scalability that needs to be added to the KEP (https://github.com/kubernetes/enhancements/blob/master/keps/NNNN-kep-template/README.md#can-enabling--using-this-feature-result-in-resource-exhaustion-of-some-node-resources-pids-sockets-inodes-etc).

Ah. I will add that.

While reviewing the previous PRR template, I had a few questions. I can't comment since you didn't update the PRR template, so I'll ask below:

What happens if someone doesn't do the "should" statements above? When controllers that consume the condition get data in the middle of a rollout or rollback is there a way to determine this or are there any ways to help determine this?

I'll add an update for this. Basically new pods will add this condition if the feature gate is on. If the feature gate is turned off, that condition will not be updated.

If a service/controller relies on this condition and it doesn't existing or is not being updated, then it will most likely not report anything related to this condition.

What specific metrics should inform a rollback?

A sharp increase in the number of PATCH requests to API Server from Kubelets
after enabling this feature is a sign of potential problem and can inform a
rollback. A cluster operator may monitor

apiserver_request_total{verb="PATCH", resource="pods", subresource="status"}

for this.
This may be the case in clusters that use a special runtime environment like
microVM/Kata, where the sandbox may crash repeatedly (without ever getting a
chance to start containers) resulting in lots of potential updates due to the
new condition "flapping". However, in such environments, this may already be the
case with existing pod conditions like ContainersReady and Ready (unless the
sandbox environment/VM crashes very early before a single container is run).
Batching of pod status updates from the Kubelet status manager will also help.

Could we better define a sharp increase? This seems like a bad metric for those other enviroments. For people operating clusters with those special runtime environments, is there any way to know that this feature is functioning properly?

I don't know the answer to this one. I think it depends on what is the baseline for this metric for their instance. The main idea of this question/metric will be to report that there could be way more patches than what is expected. On a chatty cluster, I could see this being large. So it'd expect problems could be cases where the sandbox is condition is being set to true and then back to false and repeat.

How can someone using this feature know that it is working for their instance?

The check of PodReadyToStartContainers seems good for the case where things are working correctly. If this is not set, is this only because there are failures updating etcd? The troubleshooting section below doesn't call out any additional failure modes, are there any other ways that could fail that were identified while this was in the alpha state?

Yes. We didn't see any other failures in the alpha state.

[haircommander]

/lgtm

[SergeyKanzhelev]

/lgtm
/approve

[kannon92]

@jeremyrickard You bring up an interesting point about mid rollouts.

If a user disables a feature, should we remove the condition entirely from the pod?

I can add that has something we add for beta if need be. I hope this doesn't block the promotion.

[johnbelamaric]

@jeremyrickard You bring up an interesting point about mid rollouts.

If a user disables a feature, should we remove the condition entirely from the pod?

I can add that has something we add for beta if need be. I hope this doesn't block the promotion.

So, you have already documented that the conditions will be "frozen" after disablement. We obviously do have a timestamp on conditions. We can also patch them manually. What I suggest is that you document how you would "clear" the conditions after disablement. We should not have special clean up code that runs automatically on disablement (because it might be broken too!). Instead, just document how to do the cleanup. If it seems really necessary, some script or small client side binary could be provided to do clean up.

Please do this in a follow up PR (I won't hold up merge for this).

/approve
/lgtm

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: johnbelamaric, kannon92, mrunalp, SergeyKanzhelev

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• keps/prod-readiness/OWNERS [johnbelamaric]
• keps/sig-node/OWNERS [johnbelamaric,mrunalp]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment