Implement Sandbox status via conditions

[SHRUTI6991]

Fixes #119

Motivation

We need to expose status method for Sandboxes in the Python SDK: #280. The current outstanding implementation checks the Pod status instead of Sandbox and transforms the Pod status into the Sandbox status on the client side. This is not an ideal implementation. We should expose status of the Sandbox on the controller side as a first class field. To align with Kubernetes API standards and address the previous limitations of using phase for Sandbox as discussed in #121, this proposal replaces the deprecated status.phase field with status.conditions model. This model establishes three conditions: Initialized, Ready and Suspended.

Description

This PR includes the KEP and implementation of the Sandbox status via condition. The details for the condition types and reasons are added in the KEP.

Testing Strategy

1. Unit test across 2 (Initialized) X 2 (Suspended) X 3 (Ready) condition matrix.
2. Integration Test: https://paste.googleplex.com/4805827690102784

[netlify]

✅ Deploy Preview for agent-sandbox canceled.

Name	Link
🔨 Latest commit	9fe5a13
🔍 Latest deploy log	https://app.netlify.com/projects/agent-sandbox/deploys/69cd45efed829000081bf97c

[SHRUTI6991]

/assign @janetkuo

[SHRUTI6991]

/assign @barney-s

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: SHRUTI6991
Once this PR has been reviewed and has the lgtm label, please ask for approval from barney-s. For more information see the Code Review Process.

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

Details

Needs approval from an approver in each of these files:

• OWNERS

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

[SHRUTI6991]

/assign @justinsb

[aditya-shantanu]

/assign @barney-s

[justinsb]

We need to be more precise about what we're changing here: AIUI we already have a ready status.condition, and we want to add more conditions. I'm not yet clear about what problem those conditions are solving.

For "aggregation" objects, my rule of thumb is that Ready is true when all the child objects are applied to the cluster and are themselves Ready. I consider Sandbox an aggregation object.

There's some ambiguity about whether a suspended sandbox should be considered Ready. I think convention would say yes, a Deployment of size 0 is not generally treated as broken (I think).

A controller can observe that the pod should be suspended from spec, see in status.observedGeneration that the controller has acted on this spec, and then look at the status to understand whether the child objects have been applied. I think there's an argument to be made that the current setup makes it harder to know when the Sandbox is in the process of scaling down, and I think we need to focus on that argument.

[SHRUTI6991]

We need to be more precise about what we're changing here: AIUI we already have a ready status.condition, and we want to add more conditions. I'm not yet clear about what problem those conditions are solving.

For "aggregation" objects, my rule of thumb is that Ready is true when all the child objects are applied to the cluster and are themselves Ready. I consider Sandbox an aggregation object.

There's some ambiguity about whether a suspended sandbox should be considered Ready. I think convention would say yes, a Deployment of size 0 is not generally treated as broken (I think).

A controller can observe that the pod should be suspended from spec, see in status.observedGeneration that the controller has acted on this spec, and then look at the status to understand whether the child objects have been applied. I think there's an argument to be made that the current setup makes it harder to know when the Sandbox is in the process of scaling down, and I think we need to focus on that argument.

I have reworded the motivation and emphasized focus on suspension.

My main argument against using just a single "Ready" field is that we end up recreating the issues of using Phase field. The reasons within Ready condition cannot still differentiate between "SandboxSuspended" vs "SandboxPodInitializing" for example.

In addition to that, Initialized makes the first-time setup of persistent infrastructure observable separately from the Pod which is very similar to how Pod exposes it's conditions: https://kubernetes.io/docs/concepts/workloads/pods/pod-lifecycle/#pod-conditions

[k8s-ci-robot]

PR needs rebase.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.