From cfb0a8ac5f95205bdb7ffb2929da5b49a5119f73 Mon Sep 17 00:00:00 2001 From: Kevin Hannon Date: Mon, 30 Jan 2023 14:39:15 -0500 Subject: [PATCH] add template --- .../README.md | 1048 +++++++++++++++++ .../kep.yaml | 43 + 2 files changed, 1091 insertions(+) create mode 100644 keps/sig-node/3815-pod-condition-for-configuration-errors/README.md create mode 100644 keps/sig-node/3815-pod-condition-for-configuration-errors/kep.yaml diff --git a/keps/sig-node/3815-pod-condition-for-configuration-errors/README.md b/keps/sig-node/3815-pod-condition-for-configuration-errors/README.md new file mode 100644 index 000000000000..8cc8b212d5ef --- /dev/null +++ b/keps/sig-node/3815-pod-condition-for-configuration-errors/README.md @@ -0,0 +1,1048 @@ + +# KEP-3815: Pod Condition for Configuration Errors + + + + + + +- [Release Signoff Checklist](#release-signoff-checklist) +- [Summary](#summary) +- [Motivation](#motivation) + - [Goals](#goals) + - [Non-Goals](#non-goals) +- [Proposal](#proposal) + - [User Stories (Optional)](#user-stories-optional) + - [Story 1](#story-1) + - [Story 2](#story-2) + - [Story 3](#story-3) + - [Notes/Constraints/Caveats (Optional)](#notesconstraintscaveats-optional) + - [Pending Pod State Review](#pending-pod-state-review) + - [Invalid Image Name](#invalid-image-name) + - [ErrImageNeverPull](#errimageneverpull) + - [CreateContainerConfigError - Missing Key Reference](#createcontainerconfigerror---missing-key-reference) + - [Risks and Mitigations](#risks-and-mitigations) +- [Design Details](#design-details) + - [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) + - [Upgrade / Downgrade Strategy](#upgrade--downgrade-strategy) + - [Version Skew Strategy](#version-skew-strategy) +- [Production Readiness Review Questionnaire](#production-readiness-review-questionnaire) + - [Feature Enablement and Rollback](#feature-enablement-and-rollback) + - [Rollout, Upgrade and Rollback Planning](#rollout-upgrade-and-rollback-planning) + - [Monitoring Requirements](#monitoring-requirements) + - [Dependencies](#dependencies) + - [Scalability](#scalability) + - [Troubleshooting](#troubleshooting) +- [Implementation History](#implementation-history) +- [Drawbacks](#drawbacks) +- [Alternatives](#alternatives) + - [Missing Secret When Mounting](#missing-secret-when-mounting) + - [Missing ConfigMap When Mounting](#missing-configmap-when-mounting) +- [Infrastructure Needed (Optional)](#infrastructure-needed-optional) + + +## Release Signoff Checklist + + + +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) +- [ ] (R) KEP approvers have approved the KEP status as `implementable` +- [ ] (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) + - [ ] (R) Minimum Two Week Window for GA e2e tests to prove flake free +- [ ] (R) Graduation criteria is in place + - [ ] (R) [all GA Endpoints](https://github.com/kubernetes/community/pull/1806) must be hit by [Conformance Tests](https://github.com/kubernetes/community/blob/master/contributors/devel/sig-architecture/conformance-tests.md) +- [ ] (R) Production readiness review completed +- [ ] (R) Production readiness review approved +- [ ] "Implementation History" section is up-to-date for milestone +- [ ] User-facing documentation has been created in [kubernetes/website], for publication to [kubernetes.io] +- [ ] Supporting documentation—e.g., additional design documents, links to mailing list discussions/SIG meetings, relevant PRs/issues, release notes + + + +[kubernetes.io]: https://kubernetes.io/ +[kubernetes/enhancements]: https://git.k8s.io/enhancements +[kubernetes/website]: https://git.k8s.io/website + +## Summary + + + +As a batch user, it is very likely that users experience configuration errors when submitting pods to Kubernetes. We would like to propose a new condition so that users can determine if a pod has an error in startup. A new condition will provide a quick way to verify if a pod detected a unrecoverable configuration error. Many of these configuration errors are fixable via user interactions (ie creating a secret, changing the image name) but without any modifications the pods will stay stuck in the pending state. + +## Motivation + +Batch users may not have deep knowledge of Kubernetes. It is common occurrence to find users submitting Pods with configuration errors (invalid images, wrong image names, missing volumes, missing secrets, wrong volume/secret for config map). These cases cause the Pod to be stuck in pending and there is code required to remove these pods or user input to remove. + +As a batch system developer, we find that higher level controllers have to write code to handle Pending Pod cases. Since the pods are not considered failed, the higher level controller needs to transition this state to failed in order to enforce retries. For example, the job controller will run these pods and the state of the Pod will be in Pending. The job controller does not transition this state to failed even if the pod will never start running. + +For CNCF projects, in the Armada project, we implement logic that determines retry ability based on events and statuses. Events are not standard so we expect issues when updating and we discovered various states that don't have a representative status. + +It would be ideal to have a standard condition for these common configuration errors. These errors can be fixed via human interaction but they can sometimes be difficult to programatically fix. A goal of having a standard condition for these configuration errors would be for third-party controllers and/or Kubernetes controllers to start using a single way of detecting fixable configuration errors. + + + +### Goals + +- Add a new condition FailingToStart to detect failures in configuration. + +For future cases that we want to add to to this condition, the suggestion would be to consider cases where pods are forever stuck in the pending state. We want to avoid transient pods that will eventually resolve themselves so we decided to avoid targeting events that are related to ImagePullBackOff because these can eventually be fixed without any interaction. The general rule could be that pods that require manually intervention to fix could be added as cases for this condition. + + + +### Non-Goals + +- Will not modify behavior for ImagePullBackOff + - This status can be business as usual or image pull errors + - Unclear if it is possible to detect this as a failure + - Invalid ImagePullSecrets are a common fail case but it is difficult to distinguish this case from others. +- Configuration errors that happen during ContainerCreation (Mounting volumes) will not be included as part of this condition + - PodReadyToStartContainers condition actually covers these cases so there is no need to cover them in this KEP. + + +## Proposal + +- Add a FailingToStart Condition for the following cases: + + - Invalid Image Name + - Image not found locally when ImagePullPolicy is set to never. + - Existing config map but invalid key reference + + + +### User Stories + + + +#### Story 1 + +As an end-user, if I create a pod with an invalid image, I would expect to see a condition that reflects that the pod failed to start due to a configuration error. + +#### Story 2 + +As an end-user, if I create a pod with an ImagePullPolicy of Never but image doesn't exist on the node, I would expect to see a condition that states that the pod failed to start due to a configuration error. + +#### Story 3 + +As an end-user, if I create a pod with a missing config map, I would expect to see a condition that states that the pod failed to start due to a configuration error. + +### Notes/Constraints/Caveats (Optional) + +#### Pending Pod State Review + +##### Invalid Image Name + +- Reproduction: Run image with invalid image name (all capitals) +- Pod status: + - status: Pending +- ContainerStatus: + - state=Waiting + - message='Failed to apply default image tag "BUSYBOX": couldn''t parse image + reference "BUSYBOX": invalid reference format: repository name must be + lowercase' + - reason=InvalidImageName +- Conditions: + - Type=Status + - Initialized=True + - Ready=False + - ContainersReady=False + - PodScheduled=True + - PodReadyToStartContainers=True + +##### ErrImageNeverPull + +- Reproduction: container with ImagePullPolicy: never but container doesn't exist locally +- Pod status: + - status: Pending +- ContainerStatus: + - state=Waiting + - reason=ErrImageNeverPull +- Conditions: + - Type=Status + - Initialized=True + - Ready=False + - ContainersReady=False + - PodScheduled=True + - PodReadyToStartContainers=True +- Container with missing config map + - Reproduction: Container with missing config map + - Pod status: + - status: Pending + - ContainerStatus: + - state=Waiting + - message=configmap "%s" not found + - reason=CreateContainerConfigError + - Conditions: + - Type=Status + - Initialized=True + - Ready=False + - ContainersReady=False + - PodScheduled=True + - PodReadyToStartContainers=True + +##### CreateContainerConfigError - Missing Key Reference + +- Reproduction: Container with existing config map but invalid key reference +- Pod status: + - status: Pending +- ContainerStatus: + - state=Waiting + - message=couldn't find key special.how in ConfigMap default/special-config + - reason=CreateContainerConfigError +- Conditions: + - Type=Status + - Initialized=True + - Ready=False + - ContainersReady=False + - PodScheduled=True + - PodReadyToStartContainers=True + + + +### Risks and Mitigations + +The impact will be a new condition that gets set to True if a user has a error during startup of their pod. + + +## Design Details + +Add a FailingToStart condition in core/v1: + +```go +const ( + ... + // PodFailingToStart Condition if Pod is stuck due + // to configuration error. + PodFailingToStart PodConditionType = "FailingToStart" +) +``` + +For errors that have a reason, we can create a condition for PodFailingToStart as true based on the container status. For any case where reason is set, we can just match reason and generate a PodFailingToStart condition of true. + +There are two areas where we set ContainerStatus reason: + +[Image-Errors](https://github.com/kubernetes/kubernetes/blob/master/pkg/kubelet/images/types.go#L27) + +```golang + // ErrImageInspect - Unable to inspect image + ErrImageInspect = errors.New("ImageInspectError") + + // ErrImagePull - General image pull error + ErrImagePull = errors.New("ErrImagePull") + + // ErrImageNeverPull - Required Image is absent on host and PullPolicy is NeverPullImage + ErrImageNeverPull = errors.New("ErrImageNeverPull") + + // ErrRegistryUnavailable - Get http error when pulling image from registry + ErrRegistryUnavailable = errors.New("RegistryUnavailable") + + // ErrInvalidImageName - Unable to parse the image name. + ErrInvalidImageName = errors.New("InvalidImageName") +``` + +- Should we include all these errors as potential failure cases for pending pods? I can replicate ErrImageNeverPull and InvalidImageName but not sure how to reproduce the other errors. + +[kuberuntime_container errors](https://github.com/kubernetes/kubernetes/blob/master/pkg/kubelet/kuberuntime/kuberuntime_container.go#L59) + +```golang +var ( + // ErrCreateContainerConfig - failed to create container config + ErrCreateContainerConfig = errors.New("CreateContainerConfigError") + // ErrPreCreateHook - failed to execute PreCreateHook + ErrPreCreateHook = errors.New("PreCreateHookError") + // ErrCreateContainer - failed to create container + ErrCreateContainer = errors.New("CreateContainerError") + // ErrPreStartHook - failed to execute PreStartHook + ErrPreStartHook = errors.New("PreStartHookError") + // ErrPostStartHook - failed to execute PostStartHook + ErrPostStartHook = errors.New("PostStartHookError") +) +``` + +- If these are set, can we assume that container failed? Not sure how to recreate failures of states other than ErrCreateContainerConfig +- These seem to be a lot more internal errors so I am not sure how often we get hook errors but should we include these in detection of errors. + +Potential function for detecting error and setting condition + +```golang +func GeneratePodFailingToStart(pod *v1.Pod, containerStatus []v1.ContainerStatus) v1.PodCondition { + successfulCondition := v1.PodCondition{ + Type: v1.PodFailingToStart, + Status: v1.ConditionFalse, + } + failedReasons := map[string]bool{"InvalidImageName": true, "CreateContainerConfigError": true, "ErrImageNeverPull": true} + for _, status := range containerStatus { + if status.State.Waiting == nil { + continue + } + _, ok := failedReasons[status.State.Waiting.Reason] + if ok { + return v1.PodCondition{ + Type: v1.PodFailingToStart, + Status: v1.ConditionTrue, + } + + } + } + return successfulCondition +} +``` + +One question is if we should include all the above errors in our scan of potential failed reasons? + + + +### Test Plan + + + +[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. + +#### Prerequisite testing updates + + + +##### Unit tests + + + + + +- `kubectl/kubelet_pods.go`: `Jan 31st 2023` - `71.1` +- `kubectl/status/generate.go`: `Jan 31st 2023` - `95.8` +- `kubectl/types/pod_status.go`: `Jan 31st 2023` - `100` + +##### Integration tests + +Integration tests should include every case that we are targeting for alpha. In this case, there will be a goal to write integration tests for each user story that we intend to support. + + + +- : + +##### e2e tests + +Not sure on e2e tests for this. Are these different from integration tests in this case? + + +- : + +### Graduation Criteria + + + +### Upgrade / Downgrade Strategy + + + +### Version Skew Strategy + + + +## Production Readiness Review Questionnaire + + + +### Feature Enablement and Rollback + + + +#### How can this feature be enabled / disabled in a live cluster? + + + +- [x] Feature gate (also fill in values in `kep.yaml`) + - Feature gate name: PodFailingToStartCondition + +- Turning on this feature gate means that we add FailingToStart as a condition in pods. +- If a pod has a configuration error then we would set FailingToStart to be true. + +##### Does enabling the feature change any default behavior? + +Enabling the feature gate will just add a new condition to pods. + + +###### Can the feature be disabled once it has been enabled (i.e. can we roll back the enablement)? + +Yes. The feature gate would stop setting conditions. + + + +###### What happens if we reenable the feature if it was previously rolled back? + +###### Are there any tests for feature enablement/disablement? + + + +### Rollout, Upgrade and Rollback Planning + + + +#### How can a rollout or rollback fail? Can it impact already running workloads? + + + +#### What specific metrics should inform a rollback? + + + +#### Were upgrade and rollback tested? Was the upgrade->downgrade->upgrade path tested? + + + +#### Is the rollout accompanied by any deprecations and/or removals of features, APIs, fields of API types, flags, etc.? + + + +### Monitoring Requirements + + + +#### How can an operator determine if the feature is in use by workloads? + + + +#### How can someone using this feature know that it is working for their instance? + + + +- [ ] Condition + - Details: PodFailingToStart will be set to either True or False. If feature is enabled and matches one of the user stories above then you will set a PodFailingToStart=True + +#### What are the reasonable SLOs (Service Level Objectives) for the enhancement? + + + +##### What are the SLIs (Service Level Indicators) an operator can use to determine the health of the service? + + + +- [ ] Metrics + - Metric name: + - [Optional] Aggregation method: + - Components exposing the metric: +- [ ] Other (treat as last resort) + - Details: + +###### Are there any missing metrics that would be useful to have to improve observability of this feature? + + + +### Dependencies + + + +#### Does this feature depend on any specific services running in the cluster? + +N/A + + +### Scalability + + + +#### Will enabling / using this feature result in any new API calls? + +No. + + +##### Will enabling / using this feature result in introducing new API types? + +Yes, we will be adding new condition in the pod status. + + +##### Will enabling / using this feature result in any new calls to the cloud provider? + +No + + +##### Will enabling / using this feature result in increasing size or count of the existing API objects? + +- New Condition + + +##### Will enabling / using this feature result in increasing time taken by any operations covered by existing SLIs/SLOs? + +No + + +##### Will enabling / using this feature result in non-negligible increase of resource usage (CPU, RAM, disk, IO, ...) in any components? + +No + + +##### Can enabling / using this feature result in resource exhaustion of some node resources (PIDs, sockets, inodes, etc.)? + +No + + +### Troubleshooting + + + +#### How does this feature react if the API server and/or etcd is unavailable? + +#### What are other known failure modes? + + + +#### What steps should be taken if SLOs are not being met to determine the problem? + +## Implementation History + + + +## Drawbacks + +Another condition could confuse users. + + +## Alternatives + +We decided to not tackle configuration errors that cause a sandbox creation to fail. The PodReadyToStartContainers covers the following conditions. This is actually a large pain point for our users but does not need to be covered in this KEP due to an already existing condition. + +### Missing Secret When Mounting + +- Reproduction: Container with missing secret for volume +- Pod status: + - status: Pending +- ContainerStatus: + - state=Waiting + - Reason=ContainerCreating +- Conditions: + - Type=Status + - Initialized=True + - Ready=False + - ContainersReady=False + - PodScheduled=True + - PodReadyToStartContainers=False +- Get Table + - Ready=0/1 + - Status=ContainerCreating +- Events show a message for MountVolume.SetUp +- "MountVolume.SetUp failed for volume "secret-volume" : secret "test-secret" not found" + +### Missing ConfigMap When Mounting + +- Reproduction: Container with missing secret for volume +- Pod status: + - status: Pending +- ContainerStatus: + - state=Waiting + - Reason=ContainerCreating +- Conditions: + - Type=Status + - Initialized=True + - Ready=False + - ContainersReady=False + - PodScheduled=True + - PodReadyToStartContainers=False +- Get Table + - Ready=0/1 + - Status=ContainerCreating +- Events show a message for MountVolume.SetUp +- MountVolume.SetUp failed for volume "clusters-config-volume" : configmap "clusters-config-file" not found +- This does not show up as reason or error. + + + +## Infrastructure Needed (Optional) + +N/A + diff --git a/keps/sig-node/3815-pod-condition-for-configuration-errors/kep.yaml b/keps/sig-node/3815-pod-condition-for-configuration-errors/kep.yaml new file mode 100644 index 000000000000..5e8fb934ccba --- /dev/null +++ b/keps/sig-node/3815-pod-condition-for-configuration-errors/kep.yaml @@ -0,0 +1,43 @@ +title: Pod Condition for Configuration Errors +kep-number: 3815 +authors: + - "@kannon92" +owning-sig: sig-node +participating-sigs: + - sig-node +status: provisional +creation-date: 2023-02-01 +reviewers: + - TBD +approvers: + - TBD + +see-also: + - "/keps/sig-node/3085-pod-conditions-for-starting-completition-of-sandbox-creation" + +# The target maturity stage in the current dev cycle for this KEP. +stage: alpha + +# The most recent milestone for which work toward delivery of this KEP has been +# done. This can be the current (upcoming) milestone, if it is being actively +# worked on. +latest-milestone: "v1.28" + +# The milestone at which this feature was, or is targeted to be, at each stage. +milestone: + alpha: "v1.28" + beta: "TBD" + stable: "TBD" + +# The following PRR answers are required at alpha release +# List the feature gate name and the components for which it must be enabled +feature-gates: + - name: PodFailedToStartCondition + components: + - kube-apiserver + - kubelet +disable-supported: true + +# The following PRR answers are required at beta release +metrics: + - my_feature_metric