Fill up KEP

Signed-off-by: Itamar Holder <iholder@redhat.com>

Author: iholder101

keps/sig-node/5365-ImageVolume-with-image-digest/README.md

@@ -58,7 +58,7 @@ If none of those approvers are still appropriate, then changes to that list
 should be approved by the remaining approvers and/or the owning SIG (or
 SIG Architecture for cross-cutting KEPs).
 -->
-# KEP-NNNN: Your short, descriptive title
+# KEP-5365: ImageVolume with an image digest
 
 <!--
 This is the title of your KEP. Keep it short, simple, and descriptive. A good
@@ -86,15 +86,19 @@ tags, and then generate with `hack/update-toc.sh`.
   - [User Stories (Optional)](#user-stories-optional)
     - [Story 1](#story-1)
     - [Story 2](#story-2)
+    - [Story 3](#story-3)
   - [Notes/Constraints/Caveats (Optional)](#notesconstraintscaveats-optional)
   - [Risks and Mitigations](#risks-and-mitigations)
 - [Design Details](#design-details)
+  - [API Changes](#api-changes)
   - [Test Plan](#test-plan)
       - [Prerequisite testing updates](#prerequisite-testing-updates)
       - [Unit tests](#unit-tests)
       - [Integration tests](#integration-tests)
       - [e2e tests](#e2e-tests)
   - [Graduation Criteria](#graduation-criteria)
+    - [Alpha](#alpha)
+    - [Beta](#beta)
   - [Upgrade / Downgrade Strategy](#upgrade--downgrade-strategy)
   - [Version Skew Strategy](#version-skew-strategy)
 - [Production Readiness Review Questionnaire](#production-readiness-review-questionnaire)
@@ -128,9 +132,9 @@ checklist items _must_ be updated for the enhancement to be released.
 
 Items marked with (R) are required *prior to targeting to a milestone / release*.
 
-- [ ] (R) Enhancement issue in release milestone, which links to KEP dir in [kubernetes/enhancements] (not the initial KEP PR)
+- [X] (R) Enhancement issue in release milestone, which links to KEP dir in [kubernetes/enhancements] (not the initial KEP PR)
 - [ ] (R) KEP approvers have approved the KEP status as `implementable`
-- [ ] (R) Design details are appropriately documented
+- [X] (R) Design details are appropriately documented
 - [ ] (R) Test plan is in place, giving consideration to SIG Architecture and SIG Testing input (including test refactors)
   - [ ] e2e Tests for all Beta API Operations (endpoints)
   - [ ] (R) Ensure GA e2e tests meet requirements for [Conformance Tests](https://github.com/kubernetes/community/blob/master/contributors/devel/sig-architecture/conformance-tests.md) 
@@ -173,6 +177,16 @@ updates.
 [documentation style guide]: https://github.com/kubernetes/community/blob/master/contributors/guide/style-guide.md
 -->
 
+The ImageVolume feature, implemented in [KEP-4639](https://github.com/kubernetes/enhancements/issues/4639),
+introduces a new way to define volumes for pods using a container image as the source.
+The image can be specified by its name and tag, or by its digest, alongside a pull policy,
+just like a container image is being specified in a pod spec.
+
+However, a difference between ImageVolume and a "standard" container image is that the image digest for container
+images is advertised in the pod's status as part of the `containerStatuses` field.
+As opposed to that, the digest of the image used for an ImageVolume is not advertised in the pod's status.
+This KEP aims to close this gap, by adding a new field to the pod's status that will contain the digest of the image.
+
 ## Motivation
 
 <!--
@@ -184,20 +198,41 @@ demonstrate the interest in a KEP within the wider Kubernetes community.
 [experience reports]: https://github.com/golang/go/wiki/ExperienceReports
 -->
 
+A container image digest is a unique identifier for a specific version of a container image.
+It is a hash of the image's content, which serves as a unique fingerprint for that image,
+ensuring that it is the exact version that originally used by the container.
+This is unlike a tag, which can be reused or changed over time.
+
+Just like container digests are advertised in the pod's status for containers,
+the same should be done for ImageVolumes with very similar use-cases,
+like reporting the image used through monitoring or debug tools,
+supporting git-ops workflows that rely on the image digest,
+and more (see Goals section below).
+
+A special use-case for this KEP is using live-migration in Kubevirt.
+Kubevirt runs VMs inside pods, and supports to move a VM instance between nodes.
+Behind the scenes, this is done by creating a similar pod (with the same images), then transferring the VM state.
+An image digest would ensure that the same image is used on both nodes, which is crucial for live-migrations to succeed.
+
 ### Goals
 
 <!--
 List the specific goals of the KEP. What is it trying to achieve? How will we
 know that this has succeeded?
 -->
 
+* Add a new field to the pod's status that will contain the digest of the image used for ImageVolumes.
+
 ### Non-Goals
 
 <!--
 What is out of scope for this KEP? Listing non-goals helps to focus discussion
 and make progress.
 -->
 
+* This KEP does not aim to change the way ImageVolumes are defined in the pod spec.
+  The image digest will be added to the pod's status, but the pod spec will remain unchanged.
+
 ## Proposal
 
 <!--
@@ -220,8 +255,19 @@ bogged down.
 
 #### Story 1
 
+As a user, I want to monitor which images are used by my pods, so I can ensure that the correct versions are running.
+This is important for ensuring the images are updated, not containing CVEs, etc.
+
 #### Story 2
 
+As a user, I want to use git-ops workflows that rely on the image digest,
+so I can ensure that the correct versions are running.
+
+#### Story 3
+
+As a user, I want to use live-migration in Kubevirt relying on ImageVolumes,
+so I can move my VM instances between nodes without downtime.
+
 ### Notes/Constraints/Caveats (Optional)
 
 <!--
@@ -254,6 +300,59 @@ required) or even code snippets. If there's any ambiguity about HOW your
 proposal will be implemented, this is the place to discuss them.
 -->
 
+### API Changes
+
+This KEP proposes to add an `ImageRef` to the `VolumeMountStatus` [struct](https://github.com/kubernetes/kubernetes/blob/70540c9f43e2fb7604924a120799206c27cbbd28/staging/src/k8s.io/api/core/v1/types.go#L3450-L3465)
+(which is part of the pod's status):
+```go
+// VolumeMountStatus shows status of volume mounts.
+type VolumeMountStatus struct {
+	// Name corresponds to the name of the original VolumeMount.
+	Name string `json:"name" protobuf:"bytes,1,opt,name=name"`
+	// MountPath corresponds to the original VolumeMount.
+	MountPath string `json:"mountPath" protobuf:"bytes,2,opt,name=mountPath"`
+	// ReadOnly corresponds to the original VolumeMount.
+	// +optional
+	ReadOnly bool `json:"readOnly,omitempty" protobuf:"varint,3,opt,name=readOnly"`
+	// RecursiveReadOnly must be set to Disabled, Enabled, or unspecified (for non-readonly mounts).
+	// An IfPossible value in the original VolumeMount must be translated to Disabled or Enabled,
+	// depending on the mount result.
+	// +featureGate=RecursiveReadOnlyMounts
+	// +optional
+	RecursiveReadOnly *RecursiveReadOnlyMode `json:"recursiveReadOnly,omitempty" protobuf:"bytes,4,opt,name=recursiveReadOnly,casttype=RecursiveReadOnlyMode"`
+	// ImageRef is the digest of the image used for this volume.
+	// It should have a value that's similar to the pod's status.containerStatuses[i].imageID.
+	// If the volume source is not an ImageVolume, this field will be empty.
+	ImageRef *string `json:"imageRef,omitempty" protobuf:"bytes,5,opt,name=imageRef"`
+}
+```
+
+Note that the `ImageRef` field is a pointer and should be omitted whenever the volume source is not an ImageVolume.
+In addition, during admission, it should be disallowed to populate the `ImageRef` field for non-ImageVolume sources.
+
+In addition, the KEP proposes to add an `ImageRef` field to the `ImageSpec` cri-api [struct](https://github.com/kubernetes/kubernetes/blob/cc466aa355f9e47709a108dd6774ad4aa716a984/staging/src/k8s.io/cri-api/pkg/apis/runtime/v1/api.proto#L833-L848):
+```protobuf
+// ImageSpec is an internal representation of an image.
+message ImageSpec {
+    // Container's Image field (e.g. imageID or imageDigest). Might not contain the image's digest.
+    string image = 1;
+    // Unstructured key-value map holding arbitrary metadata.
+    // ImageSpec Annotations can be used to help the runtime target specific
+    // images in multi-arch images.
+    map<string, string> annotations = 2;
+    // The container image reference specified by the user (e.g. image[:tag] or digest).
+    // Only set if available within the RPC context.
+    string user_specified_image = 18;
+    // Runtime handler to use for pulling the image.
+    // If the runtime handler is unknown, the request should be rejected.
+    // An empty string would select the default runtime handler.
+    string runtime_handler = 19;
+    // The digest of the image used for this volume.
+    string image_ref = 20;
+}
+```
+
+
 ### Test Plan
 
 <!--
@@ -267,7 +366,9 @@ when drafting this test plan.
 [testing-guidelines]: https://git.k8s.io/community/contributors/devel/sig-testing/testing.md
 -->
 
-[ ] I/we understand the owners of the involved components may require updates to
+Since we're proposing a CRI-API change, we'll need to test this feature with both containerd and CRI-O runtimes.
+
+[X] I/we understand the owners of the involved components may require updates to
 existing tests to make this code solid enough prior to committing the changes necessary
 to implement this enhancement.
 
@@ -420,8 +521,20 @@ in back-to-back releases.
 - Deprecate the flag
 -->
 
+#### Alpha
+
+- Feature implemented behind a feature flag
+- Initial e2e tests completed and enabled
+
+#### Beta
+- Feature is supported by both containerd and CRI-O runtimes
+
 ### Upgrade / Downgrade Strategy
 
+Upgrade: Feature gate is off by default in Alpha.
+
+Downgrade: The image digest field will be omitted from the pod's status.
+
 <!--
 If applicable, how will the component be upgraded and downgraded? Make sure
 this is in the test plan.
@@ -436,6 +549,12 @@ enhancement:
 
 ### Version Skew Strategy
 
+The CRI would have to support the new `ImageRef` field in the `VolumeMountStatus` struct that had been added to the CRI-API.
+If the CRI does not support this field, the kubelet would not be able to populate it in the pod's status,
+which means that the image digest would not be available in the pod's status.
+
+This feature affects only kubelet and the CRI, no other components are affected.
+
 <!--
 If applicable, how will the component handle version skew with other
 components? What are the guarantees? Make sure this is in the test plan.
@@ -491,15 +610,15 @@ well as the [existing list] of feature gates.
 [existing list]: https://kubernetes.io/docs/reference/command-line-tools-reference/feature-gates/
 -->
 
-- [ ] Feature gate (also fill in values in `kep.yaml`)
-  - Feature gate name:
-  - Components depending on the feature gate:
-- [ ] Other
-  - Describe the mechanism:
+- [X] Feature gate (also fill in values in `kep.yaml`)
+  - Feature gate name: ImageVolumeWithDigest.
+  - Components depending on the feature gate: kubelet. 
+- [X] Other
+  - Describe the mechanism: Kubelet needs to be restarted when enabling/disabling this feature.
   - Will enabling / disabling the feature require downtime of the control
-    plane?
+    plane? No.
   - Will enabling / disabling the feature require downtime or reprovisioning
-    of a node?
+    of a node? Yes.
 
 ###### Does enabling the feature change any default behavior?
 
@@ -508,6 +627,9 @@ Any change of default behavior may be surprising to users or break existing
 automations, so be extremely careful here.
 -->
 
+By default, the image ID should be added to the pod's status when the feature is enabled and an ImageVolume is used.
+This should not break any existing flows / behaviors.
+
 ###### Can the feature be disabled once it has been enabled (i.e. can we roll back the enablement)?
 
 <!--
@@ -521,8 +643,14 @@ feature.
 NOTE: Also set `disable-supported` to `true` or `false` in `kep.yaml`.
 -->
 
+The feature gate can be turned off.
+Kubelet would be needed to be restarted for the image digest to be removed from the pod's status.
+
 ###### What happens if we reenable the feature if it was previously rolled back?
 
+After re-enabling the feature, any new pod that would be created would include the image digest in its status.
+Vice-versa for disabling the feature.
+
 ###### Are there any tests for feature enablement/disablement?
 
 <!--
@@ -556,6 +684,9 @@ rollout. Similarly, consider large clusters and how enablement/disablement
 will rollout across nodes.
 -->
 
+The worst case scenario is that some pods would have the image digest in their status, while others would not.
+This is not expected to cause any issues or break existing behavior.
+
 ###### What specific metrics should inform a rollback?
 
 <!--
@@ -607,9 +738,9 @@ Recall that end users cannot usually observe component logs or access metrics.
 
 - [ ] Events
   - Event Reason: 
-- [ ] API .status
+- [X] API .status
   - Condition name: 
-  - Other field: 
+  - Other field: pod.status.volumeMounts[i].ImageRef
 - [ ] Other (treat as last resort)
   - Details:
 
@@ -656,6 +787,8 @@ implementation difficulties, etc.).
 This section must be completed when targeting beta to a release.
 -->
 
+This feature depends on the ImageVolume feature, which is implemented in KEP-4639.
+
 ###### Does this feature depend on any specific services running in the cluster?
 
 <!--
@@ -685,6 +818,8 @@ For GA, this section is required: approvers should be able to confirm the
 previous answers based on experience in the field.
 -->
 
+Scalability issues are not expected.
+
 ###### Will enabling / using this feature result in any new API calls?
 
 <!--
@@ -700,6 +835,8 @@ Focusing mostly on:
     heartbeats, leader election, etc.)
 -->
 
+No, just for another field in a cri-api call.
+
 ###### Will enabling / using this feature result in introducing new API types?
 
 <!--
@@ -709,6 +846,8 @@ Describe them, providing:
   - Supported number of objects per namespace (for namespace-scoped objects)
 -->
 
+Yes, a new image digest field will be added to the `VolumeMountStatus` struct in the pod's status.
+
 ###### Will enabling / using this feature result in any new calls to the cloud provider?
 
 <!--
@@ -717,6 +856,8 @@ Describe them, providing:
   - Estimated increase:
 -->
 
+No.
+
 ###### Will enabling / using this feature result in increasing size or count of the existing API objects?
 
 <!--
@@ -726,6 +867,8 @@ Describe them, providing:
   - Estimated amount of new objects: (e.g., new Object X for every existing Pod)
 -->
 
+Additional field.
+
 ###### Will enabling / using this feature result in increasing time taken by any operations covered by existing SLIs/SLOs?
 
 <!--
@@ -737,6 +880,8 @@ Think about adding additional work or introducing new steps in between
 [existing SLIs/SLOs]: https://git.k8s.io/community/sig-scalability/slos/slos.md#kubernetes-slisslos
 -->
 
+No.
+
 ###### Will enabling / using this feature result in non-negligible increase of resource usage (CPU, RAM, disk, IO, ...) in any components?
 
 <!--
@@ -749,6 +894,8 @@ This through this both in small and large cases, again with respect to the
 [supported limits]: https://git.k8s.io/community//sig-scalability/configs-and-limits/thresholds.md
 -->
 
+No.
+
 ###### Can enabling / using this feature result in resource exhaustion of some node resources (PIDs, sockets, inodes, etc.)?
 
 <!--
@@ -761,6 +908,8 @@ Are there any tests that were run/should be run to understand performance charac
 and validate the declared limits?
 -->
 
+No.
+
 ### Troubleshooting
 
 <!--