Skip to content

Commit

Permalink
Merge pull request #119659 from kannon92/beta-pod-ready-to-start
Browse files Browse the repository at this point in the history
[KEP-3085] Promote PodReadyToStartContainers to beta in 1.29
  • Loading branch information
k8s-ci-robot committed Oct 12, 2023
2 parents 32ea66d + c94240e commit 8923c3c
Show file tree
Hide file tree
Showing 10 changed files with 32 additions and 54 deletions.
5 changes: 3 additions & 2 deletions pkg/features/kube_features.go
Original file line number Diff line number Diff line change
Expand Up @@ -674,8 +674,9 @@ const (
// Set pod completion index as a pod label for Indexed Jobs.
PodIndexLabel featuregate.Feature = "PodIndexLabel"

// owner: @ddebroy
// owner: @ddebroy, @kannon92
// alpha: v1.25
// beta: v1.29
//
// Enables reporting of PodReadyToStartContainersCondition condition in pod status after pod
// sandbox creation and network configuration completes successfully
Expand Down Expand Up @@ -1132,7 +1133,7 @@ var defaultKubernetesFeatureGates = map[featuregate.Feature]featuregate.FeatureS

PodDisruptionConditions: {Default: true, PreRelease: featuregate.Beta},

PodReadyToStartContainersCondition: {Default: false, PreRelease: featuregate.Alpha},
PodReadyToStartContainersCondition: {Default: true, PreRelease: featuregate.Beta},

PodHostIPs: {Default: false, PreRelease: featuregate.Alpha},

Expand Down
18 changes: 9 additions & 9 deletions pkg/kubelet/kubelet_pods_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3367,7 +3367,7 @@ func Test_generateAPIPodStatus(t *testing.T) {
LastTransitionTime: normalized_now,
},
expectedPodReadyToStartContainersCondition: v1.PodCondition{
Type: kubetypes.PodReadyToStartContainers,
Type: v1.PodReadyToStartContainers,
Status: v1.ConditionTrue,
},
},
Expand Down Expand Up @@ -3408,7 +3408,7 @@ func Test_generateAPIPodStatus(t *testing.T) {
},
},
expectedPodReadyToStartContainersCondition: v1.PodCondition{
Type: kubetypes.PodReadyToStartContainers,
Type: v1.PodReadyToStartContainers,
Status: v1.ConditionTrue,
},
},
Expand Down Expand Up @@ -3448,7 +3448,7 @@ func Test_generateAPIPodStatus(t *testing.T) {
},
},
expectedPodReadyToStartContainersCondition: v1.PodCondition{
Type: kubetypes.PodReadyToStartContainers,
Type: v1.PodReadyToStartContainers,
Status: v1.ConditionTrue,
},
},
Expand Down Expand Up @@ -3489,7 +3489,7 @@ func Test_generateAPIPodStatus(t *testing.T) {
},
},
expectedPodReadyToStartContainersCondition: v1.PodCondition{
Type: kubetypes.PodReadyToStartContainers,
Type: v1.PodReadyToStartContainers,
Status: v1.ConditionFalse,
},
},
Expand Down Expand Up @@ -3536,7 +3536,7 @@ func Test_generateAPIPodStatus(t *testing.T) {
Message: "test",
},
expectedPodReadyToStartContainersCondition: v1.PodCondition{
Type: kubetypes.PodReadyToStartContainers,
Type: v1.PodReadyToStartContainers,
Status: v1.ConditionFalse,
},
},
Expand Down Expand Up @@ -3590,7 +3590,7 @@ func Test_generateAPIPodStatus(t *testing.T) {
Message: "test",
},
expectedPodReadyToStartContainersCondition: v1.PodCondition{
Type: kubetypes.PodReadyToStartContainers,
Type: v1.PodReadyToStartContainers,
Status: v1.ConditionFalse,
},
},
Expand Down Expand Up @@ -3631,7 +3631,7 @@ func Test_generateAPIPodStatus(t *testing.T) {
},
},
expectedPodReadyToStartContainersCondition: v1.PodCondition{
Type: kubetypes.PodReadyToStartContainers,
Type: v1.PodReadyToStartContainers,
Status: v1.ConditionTrue,
},
},
Expand Down Expand Up @@ -3687,7 +3687,7 @@ func Test_generateAPIPodStatus(t *testing.T) {
},
},
expectedPodReadyToStartContainersCondition: v1.PodCondition{
Type: kubetypes.PodReadyToStartContainers,
Type: v1.PodReadyToStartContainers,
Status: v1.ConditionTrue,
},
},
Expand Down Expand Up @@ -3747,7 +3747,7 @@ func Test_generateAPIPodStatus(t *testing.T) {
},
},
expectedPodReadyToStartContainersCondition: v1.PodCondition{
Type: kubetypes.PodReadyToStartContainers,
Type: v1.PodReadyToStartContainers,
Status: v1.ConditionTrue,
},
},
Expand Down
4 changes: 2 additions & 2 deletions pkg/kubelet/status/generate.go
Original file line number Diff line number Diff line change
Expand Up @@ -247,12 +247,12 @@ func GeneratePodReadyToStartContainersCondition(pod *v1.Pod, podStatus *kubecont
// fresh sandbox and configure networking for the sandbox.
if !newSandboxNeeded {
return v1.PodCondition{
Type: kubetypes.PodReadyToStartContainers,
Type: v1.PodReadyToStartContainers,
Status: v1.ConditionTrue,
}
}
return v1.PodCondition{
Type: kubetypes.PodReadyToStartContainers,
Type: v1.PodReadyToStartContainers,
Status: v1.ConditionFalse,
}
}
Expand Down
3 changes: 1 addition & 2 deletions pkg/kubelet/status/generate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ import (
v1 "k8s.io/api/core/v1"
runtimeapi "k8s.io/cri-api/pkg/apis/runtime/v1"
kubecontainer "k8s.io/kubernetes/pkg/kubelet/container"
kubetypes "k8s.io/kubernetes/pkg/kubelet/types"
"k8s.io/utils/pointer"
)

Expand Down Expand Up @@ -615,7 +614,7 @@ func TestGeneratePodReadyToStartContainersCondition(t *testing.T) {
},
} {
t.Run(desc, func(t *testing.T) {
test.expected.Type = kubetypes.PodReadyToStartContainers
test.expected.Type = v1.PodReadyToStartContainers
condition := GeneratePodReadyToStartContainersCondition(test.pod, test.status)
require.Equal(t, test.expected.Type, condition.Type)
require.Equal(t, test.expected.Status, condition.Status)
Expand Down
2 changes: 1 addition & 1 deletion pkg/kubelet/status/status_manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -634,7 +634,7 @@ func (m *manager) updateStatusInternal(pod *v1.Pod, status v1.PodStatus, forceUp
updateLastTransitionTime(&status, &oldStatus, v1.PodInitialized)

// Set PodReadyToStartContainersCondition.LastTransitionTime.
updateLastTransitionTime(&status, &oldStatus, kubetypes.PodReadyToStartContainers)
updateLastTransitionTime(&status, &oldStatus, v1.PodReadyToStartContainers)

// Set PodScheduledCondition.LastTransitionTime.
updateLastTransitionTime(&status, &oldStatus, v1.PodScheduled)
Expand Down
9 changes: 0 additions & 9 deletions pkg/kubelet/types/constants.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,12 +38,3 @@ const (
LimitedSwap = "LimitedSwap"
UnlimitedSwap = "UnlimitedSwap"
)

// Alpha conditions managed by Kubelet that are not yet part of the API. The
// entries here should be moved to staging/src/k8s.io.api/core/v1/types.go
// once the feature managing the condition graduates to Beta.
const (
// PodReadyToStartContainers pod sandbox is successfully configured and
// the pod is ready to launch containers.
PodReadyToStartContainers = "PodReadyToStartContainers"
)
2 changes: 1 addition & 1 deletion pkg/kubelet/types/pod_status.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ func PodConditionByKubelet(conditionType v1.PodConditionType) bool {
}
}
if utilfeature.DefaultFeatureGate.Enabled(features.PodReadyToStartContainersCondition) {
if conditionType == PodReadyToStartContainers {
if conditionType == v1.PodReadyToStartContainers {
return true
}
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/kubelet/types/pod_status_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ func TestPodConditionByKubelet(t *testing.T) {
v1.PodReady,
v1.PodInitialized,
v1.ContainersReady,
PodReadyToStartContainers,
v1.PodReadyToStartContainers,
}

for _, tc := range trueCases {
Expand Down
3 changes: 3 additions & 0 deletions staging/src/k8s.io/api/core/v1/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -2866,6 +2866,9 @@ const (
// DisruptionTarget indicates the pod is about to be terminated due to a
// disruption (such as preemption, eviction API or garbage-collection).
DisruptionTarget PodConditionType = "DisruptionTarget"
// PodReadyToStartContainers pod sandbox is successfully configured and
// the pod is ready to launch containers.
PodReadyToStartContainers PodConditionType = "PodReadyToStartContainers"
)

// These are reasons for a pod's transition to a condition.
Expand Down
38 changes: 11 additions & 27 deletions test/e2e_node/pod_conditions_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ import (
"k8s.io/apimachinery/pkg/fields"
"k8s.io/apimachinery/pkg/util/uuid"
"k8s.io/kubernetes/pkg/kubelet/events"
kubetypes "k8s.io/kubernetes/pkg/kubelet/types"
"k8s.io/kubernetes/test/e2e/framework"
e2eevents "k8s.io/kubernetes/test/e2e/framework/events"
e2epod "k8s.io/kubernetes/test/e2e/framework/pod"
Expand All @@ -41,6 +40,7 @@ import (
kubeletconfig "k8s.io/kubernetes/pkg/kubelet/apis/config"

"github.com/onsi/ginkgo/v2"
"github.com/onsi/gomega"
)

var _ = SIGDescribe("Pod conditions managed by Kubelet", func() {
Expand Down Expand Up @@ -113,7 +113,7 @@ func runPodFailingConditionsTest(f *framework.Framework, hasInitContainers, chec

// Verify PodReadyToStartContainers is not set (since sandboxcreation is blocked)
if checkPodReadyToStart {
_, err := getTransitionTimeForPodConditionWithStatus(p, kubetypes.PodReadyToStartContainers, false)
_, err := getTransitionTimeForPodConditionWithStatus(p, v1.PodReadyToStartContainers, false)
framework.ExpectNoError(err)
}

Expand All @@ -125,9 +125,7 @@ func runPodFailingConditionsTest(f *framework.Framework, hasInitContainers, chec
// Verify PodInitialized is set if init containers are not present (since without init containers, it gets set very early)
initializedTime, err := getTransitionTimeForPodConditionWithStatus(p, v1.PodInitialized, true)
framework.ExpectNoError(err)
if initializedTime.Before(scheduledTime) {
framework.Failf("pod without init containers is initialized at: %v which is before pod scheduled at: %v", initializedTime, scheduledTime)
}
gomega.Expect(initializedTime.Before(scheduledTime)).NotTo(gomega.BeTrue(), fmt.Sprintf("pod without init containers is initialized at: %v which is before pod scheduled at: %v", initializedTime, scheduledTime))
}

// Verify ContainersReady is not set (since sandboxcreation is blocked)
Expand Down Expand Up @@ -164,47 +162,33 @@ func runPodReadyConditionsTest(f *framework.Framework, hasInitContainers, checkP
condBeforeContainersReadyTransitionTime := initializedTime
errSubstrIfContainersReadyTooEarly := "is initialized"
if checkPodReadyToStart {
readyToStartContainersTime, err := getTransitionTimeForPodConditionWithStatus(p, kubetypes.PodReadyToStartContainers, true)
readyToStartContainersTime, err := getTransitionTimeForPodConditionWithStatus(p, v1.PodReadyToStartContainers, true)
framework.ExpectNoError(err)

if hasInitContainers {
// With init containers, verify the sequence of conditions is: Scheduled => PodReadyToStartContainers => Initialized
if readyToStartContainersTime.Before(scheduledTime) {
framework.Failf("pod with init containers is initialized at: %v which is before pod has ready to start at: %v", initializedTime, readyToStartContainersTime)
}
if initializedTime.Before(readyToStartContainersTime) {
framework.Failf("pod with init containers is initialized at: %v which is before pod has ready to start at: %v", initializedTime, readyToStartContainersTime)
}
gomega.Expect(readyToStartContainersTime.Before(scheduledTime)).ToNot(gomega.BeTrue(), fmt.Sprintf("pod with init containers is initialized at: %v which is before pod has ready to start at: %v", initializedTime, readyToStartContainersTime))
gomega.Expect(initializedTime.Before(readyToStartContainersTime)).ToNot(gomega.BeTrue(), fmt.Sprintf("pod with init containers is initialized at: %v which is before pod has ready to start at: %v", initializedTime, readyToStartContainersTime))
} else {
// Without init containers, verify the sequence of conditions is: Scheduled => Initialized => PodReadyToStartContainers
condBeforeContainersReadyTransitionTime = readyToStartContainersTime
errSubstrIfContainersReadyTooEarly = "ready to start"
if initializedTime.Before(scheduledTime) {
framework.Failf("pod without init containers initialized at: %v which is before pod scheduled at: %v", initializedTime, scheduledTime)
}
if readyToStartContainersTime.Before(initializedTime) {
framework.Failf("pod without init containers has ready to start at: %v which is before pod is initialized at: %v", readyToStartContainersTime, initializedTime)
}
gomega.Expect(initializedTime.Before(scheduledTime)).NotTo(gomega.BeTrue(), fmt.Sprintf("pod without init containers initialized at: %v which is before pod scheduled at: %v", initializedTime, scheduledTime))
gomega.Expect(readyToStartContainersTime.Before(initializedTime)).NotTo(gomega.BeTrue(), fmt.Sprintf("pod without init containers has ready to start at: %v which is before pod is initialized at: %v", readyToStartContainersTime, initializedTime))
}
} else {
// In the absence of PodHasReadyToStartContainers feature disabled, verify the sequence is: Scheduled => Initialized
if initializedTime.Before(scheduledTime) {
framework.Failf("pod initialized at: %v which is before pod scheduled at: %v", initializedTime, scheduledTime)
}
gomega.Expect(initializedTime.Before(scheduledTime)).NotTo(gomega.BeTrue(), fmt.Sprintf("pod initialized at: %v which is before pod scheduled at: %v", initializedTime, scheduledTime))
}
// Verify the next condition to get set is ContainersReady
containersReadyTime, err := getTransitionTimeForPodConditionWithStatus(p, v1.ContainersReady, true)
framework.ExpectNoError(err)
if containersReadyTime.Before(condBeforeContainersReadyTransitionTime) {
framework.Failf("containers ready at: %v which is before pod %s: %v", containersReadyTime, errSubstrIfContainersReadyTooEarly, initializedTime)
}
gomega.Expect(containersReadyTime.Before(condBeforeContainersReadyTransitionTime)).NotTo(gomega.BeTrue(), fmt.Sprintf("containers ready at: %v which is before pod %s: %v", containersReadyTime, errSubstrIfContainersReadyTooEarly, initializedTime))

// Verify ContainersReady => PodReady
podReadyTime, err := getTransitionTimeForPodConditionWithStatus(p, v1.PodReady, true)
framework.ExpectNoError(err)
if podReadyTime.Before(containersReadyTime) {
framework.Failf("pod ready at: %v which is before pod containers ready at: %v", podReadyTime, containersReadyTime)
}
gomega.Expect(podReadyTime.Before(containersReadyTime)).NotTo(gomega.BeTrue(), fmt.Sprintf("pod ready at: %v which is before pod containers ready at: %v", podReadyTime, containersReadyTime))
}
}

Expand Down

0 comments on commit 8923c3c

Please sign in to comment.