Skip to content

Commit

Permalink
Allow restartable init containers to have lifecycle
Browse files Browse the repository at this point in the history
  • Loading branch information
@gjkim42
gjkim42 committed Jul 17, 2023
1 parent 7ef2d67 commit 3bf2826
Show file tree
Hide file tree
Showing 4 changed files with 301 additions and 43 deletions.
11 changes: 6 additions & 5 deletions pkg/apis/core/validation/validation.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3241,9 +3241,8 @@ func validateInitContainers(containers []core.Container, regularContainers []cor

switch {
case restartAlways:
// TODO: Allow restartable init containers to have a lifecycle hook.
if ctr.Lifecycle != nil {
allErrs = append(allErrs, field.Forbidden(idxPath.Child("lifecycle"), "may not be set for init containers"))
allErrs = append(allErrs, validateLifecycle(ctr.Lifecycle, idxPath.Child("lifecycle"))...)
}
allErrs = append(allErrs, validateLivenessProbe(ctr.LivenessProbe, idxPath.Child("livenessProbe"))...)
allErrs = append(allErrs, validateReadinessProbe(ctr.ReadinessProbe, idxPath.Child("readinessProbe"))...)
Expand All @@ -3252,7 +3251,7 @@ func validateInitContainers(containers []core.Container, regularContainers []cor
default:
// These fields are disallowed for init containers.
if ctr.Lifecycle != nil {
allErrs = append(allErrs, field.Forbidden(idxPath.Child("lifecycle"), "may not be set for init containers"))
allErrs = append(allErrs, field.Forbidden(idxPath.Child("lifecycle"), "may not be set for init containers without restartPolicy=Always"))
}
if ctr.LivenessProbe != nil {
allErrs = append(allErrs, field.Forbidden(idxPath.Child("livenessProbe"), "may not be set for init containers without restartPolicy=Always"))
Expand Down Expand Up @@ -3370,8 +3369,10 @@ func validateContainers(containers []core.Container, volumes map[string]core.Vol
allNames.Insert(ctr.Name)
}

// These fields are only allowed for regular containers, so only check supported values here.
// Init and ephemeral container validation will return field.Forbidden() for these paths.
// These fields are allowed for regular containers and restartable init
// containers.
// Regular init container and ephemeral container validation will return
// field.Forbidden() for these paths.
if ctr.Lifecycle != nil {
allErrs = append(allErrs, validateLifecycle(ctr.Lifecycle, path.Child("lifecycle"))...)
}
Expand Down
135 changes: 134 additions & 1 deletion pkg/apis/core/validation/validation_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8164,11 +8164,23 @@ func TestValidateInitContainers(t *testing.T) {
ImagePullPolicy: "IfNotPresent",
TerminationMessagePolicy: "File",
}, {
Name: "container-3-restart-always-with-probes",
Name: "container-3-restart-always-with-lifecycle-hook-and-probes",
Image: "image",
ImagePullPolicy: "IfNotPresent",
TerminationMessagePolicy: "File",
RestartPolicy: &containerRestartPolicyAlways,
Lifecycle: &core.Lifecycle{
PostStart: &core.LifecycleHandler{
Exec: &core.ExecAction{
Command: []string{"echo", "post start"},
},
},
PreStop: &core.LifecycleHandler{
Exec: &core.ExecAction{
Command: []string{"echo", "pre stop"},
},
},
},
LivenessProbe: &core.Probe{
ProbeHandler: core.ProbeHandler{
TCPSocket: &core.TCPSocketAction{
Expand Down Expand Up @@ -8447,6 +8459,127 @@ func TestValidateInitContainers(t *testing.T) {
},
}},
field.ErrorList{{Type: field.ErrorTypeInvalid, Field: "initContainers[0].livenessProbe.successThreshold", BadValue: int32(2)}},
}, {
"invalid lifecycle, no exec command.",
line(),
[]core.Container{{
Name: "life-123",
Image: "image",
ImagePullPolicy: "IfNotPresent",
TerminationMessagePolicy: "File",
RestartPolicy: &containerRestartPolicyAlways,
Lifecycle: &core.Lifecycle{
PreStop: &core.LifecycleHandler{
Exec: &core.ExecAction{},
},
},
}},
field.ErrorList{{Type: field.ErrorTypeRequired, Field: "initContainers[0].lifecycle.preStop.exec.command", BadValue: ""}},
}, {
"invalid lifecycle, no http path.",
line(),
[]core.Container{{
Name: "life-123",
Image: "image",
ImagePullPolicy: "IfNotPresent",
TerminationMessagePolicy: "File",
RestartPolicy: &containerRestartPolicyAlways,
Lifecycle: &core.Lifecycle{
PreStop: &core.LifecycleHandler{
HTTPGet: &core.HTTPGetAction{
Port: intstr.FromInt32(80),
Scheme: "HTTP",
},
},
},
}},
field.ErrorList{{Type: field.ErrorTypeRequired, Field: "initContainers[0].lifecycle.preStop.httpGet.path", BadValue: ""}},
}, {
"invalid lifecycle, no http port.",
line(),
[]core.Container{{
Name: "life-123",
Image: "image",
ImagePullPolicy: "IfNotPresent",
TerminationMessagePolicy: "File",
RestartPolicy: &containerRestartPolicyAlways,
Lifecycle: &core.Lifecycle{
PreStop: &core.LifecycleHandler{
HTTPGet: &core.HTTPGetAction{
Path: "/",
Scheme: "HTTP",
},
},
},
}},
field.ErrorList{{Type: field.ErrorTypeInvalid, Field: "initContainers[0].lifecycle.preStop.httpGet.port", BadValue: 0}},
}, {
"invalid lifecycle, no http scheme.",
line(),
[]core.Container{{
Name: "life-123",
Image: "image",
ImagePullPolicy: "IfNotPresent",
TerminationMessagePolicy: "File",
RestartPolicy: &containerRestartPolicyAlways,
Lifecycle: &core.Lifecycle{
PreStop: &core.LifecycleHandler{
HTTPGet: &core.HTTPGetAction{
Path: "/",
Port: intstr.FromInt32(80),
},
},
},
}},
field.ErrorList{{Type: field.ErrorTypeNotSupported, Field: "initContainers[0].lifecycle.preStop.httpGet.scheme", BadValue: core.URIScheme("")}},
}, {
"invalid lifecycle, no tcp socket port.",
line(),
[]core.Container{{
Name: "life-123",
Image: "image",
ImagePullPolicy: "IfNotPresent",
TerminationMessagePolicy: "File",
RestartPolicy: &containerRestartPolicyAlways,
Lifecycle: &core.Lifecycle{
PreStop: &core.LifecycleHandler{
TCPSocket: &core.TCPSocketAction{},
},
},
}},
field.ErrorList{{Type: field.ErrorTypeInvalid, Field: "initContainers[0].lifecycle.preStop.tcpSocket.port", BadValue: 0}},
}, {
"invalid lifecycle, zero tcp socket port.",
line(),
[]core.Container{{
Name: "life-123",
Image: "image",
ImagePullPolicy: "IfNotPresent",
TerminationMessagePolicy: "File",
RestartPolicy: &containerRestartPolicyAlways,
Lifecycle: &core.Lifecycle{
PreStop: &core.LifecycleHandler{
TCPSocket: &core.TCPSocketAction{
Port: intstr.FromInt32(0),
},
},
},
}},
field.ErrorList{{Type: field.ErrorTypeInvalid, Field: "initContainers[0].lifecycle.preStop.tcpSocket.port", BadValue: 0}},
}, {
"invalid lifecycle, no action.",
line(),
[]core.Container{{
Name: "life-123",
Image: "image",
ImagePullPolicy: "IfNotPresent",
TerminationMessagePolicy: "File",
RestartPolicy: &containerRestartPolicyAlways,
Lifecycle: &core.Lifecycle{
PreStop: &core.LifecycleHandler{},
},
}},
field.ErrorList{{Type: field.ErrorTypeRequired, Field: "initContainers[0].lifecycle.preStop", BadValue: ""}},
},
}
for _, tc := range errorCases {
Expand Down
73 changes: 50 additions & 23 deletions pkg/kubelet/kuberuntime/kuberuntime_container_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -341,7 +341,7 @@ func TestToKubeContainerStatusWithResources(t *testing.T) {
}
}

func TestLifeCycleHook(t *testing.T) {
func testLifeCycleHook(t *testing.T, testPod *v1.Pod, testContainer *v1.Container) {

// Setup
fakeRuntime, _, m, _ := createTestRuntimeManager()
Expand All @@ -352,23 +352,6 @@ func TestLifeCycleHook(t *testing.T) {
ID: "foo",
}

testPod := &v1.Pod{
ObjectMeta: metav1.ObjectMeta{
Name: "bar",
Namespace: "default",
},
Spec: v1.PodSpec{
Containers: []v1.Container{
{
Name: "foo",
Image: "busybox",
ImagePullPolicy: v1.PullIfNotPresent,
Command: []string{"testCommand"},
WorkingDir: "testWorkingDir",
},
},
},
}
cmdPostStart := &v1.Lifecycle{
PostStart: &v1.LifecycleHandler{
Exec: &v1.ExecAction{
Expand Down Expand Up @@ -418,7 +401,7 @@ func TestLifeCycleHook(t *testing.T) {
// Configured and works as expected
t.Run("PreStop-CMDExec", func(t *testing.T) {
ctx := context.Background()
testPod.Spec.Containers[0].Lifecycle = cmdLifeCycle
testContainer.Lifecycle = cmdLifeCycle
m.killContainer(ctx, testPod, cID, "foo", "testKill", "", &gracePeriod)
if fakeRunner.Cmd[0] != cmdLifeCycle.PreStop.Exec.Command[0] {
t.Errorf("CMD Prestop hook was not invoked")
Expand All @@ -432,7 +415,7 @@ func TestLifeCycleHook(t *testing.T) {
defer func() { fakeHTTP.req = nil }()
defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.ConsistentHTTPGetHandlers, false)()
httpLifeCycle.PreStop.HTTPGet.Port = intstr.IntOrString{}
testPod.Spec.Containers[0].Lifecycle = httpLifeCycle
testContainer.Lifecycle = httpLifeCycle
m.killContainer(ctx, testPod, cID, "foo", "testKill", "", &gracePeriod)

if fakeHTTP.req == nil || !strings.Contains(fakeHTTP.req.URL.String(), httpLifeCycle.PreStop.HTTPGet.Host) {
Expand All @@ -443,7 +426,7 @@ func TestLifeCycleHook(t *testing.T) {
ctx := context.Background()
defer func() { fakeHTTP.req = nil }()
httpLifeCycle.PreStop.HTTPGet.Port = intstr.FromInt32(80)
testPod.Spec.Containers[0].Lifecycle = httpLifeCycle
testContainer.Lifecycle = httpLifeCycle
m.killContainer(ctx, testPod, cID, "foo", "testKill", "", &gracePeriod)

if fakeHTTP.req == nil || !strings.Contains(fakeHTTP.req.URL.String(), httpLifeCycle.PreStop.HTTPGet.Host) {
Expand Down Expand Up @@ -473,8 +456,7 @@ func TestLifeCycleHook(t *testing.T) {
// Fake all the things you need before trying to create a container
fakeSandBox, _ := makeAndSetFakePod(t, m, fakeRuntime, testPod)
fakeSandBoxConfig, _ := m.generatePodSandboxConfig(testPod, 0)
testPod.Spec.Containers[0].Lifecycle = cmdPostStart
testContainer := &testPod.Spec.Containers[0]
testContainer.Lifecycle = cmdPostStart
fakePodStatus := &kubecontainer.PodStatus{
ContainerStatuses: []*kubecontainer.Status{
{
Expand All @@ -500,6 +482,51 @@ func TestLifeCycleHook(t *testing.T) {
})
}

func TestLifeCycleHook(t *testing.T) {
testPod := &v1.Pod{
ObjectMeta: metav1.ObjectMeta{
Name: "bar",
Namespace: "default",
},
Spec: v1.PodSpec{
Containers: []v1.Container{
{
Name: "foo",
Image: "busybox",
ImagePullPolicy: v1.PullIfNotPresent,
Command: []string{"testCommand"},
WorkingDir: "testWorkingDir",
},
},
},
}

testLifeCycleHook(t, testPod, &testPod.Spec.Containers[0])
}

func TestLifeCycleHookForRestartableInitContainer(t *testing.T) {
testPod := &v1.Pod{
ObjectMeta: metav1.ObjectMeta{
Name: "bar",
Namespace: "default",
},
Spec: v1.PodSpec{
InitContainers: []v1.Container{
{
Name: "foo",
Image: "busybox",
ImagePullPolicy: v1.PullIfNotPresent,
Command: []string{"testCommand"},
WorkingDir: "testWorkingDir",
RestartPolicy: &containerRestartPolicyAlways,
},
},
},
}

testLifeCycleHook(t, testPod, &testPod.Spec.InitContainers[0])
}

func TestStartSpec(t *testing.T) {
podStatus := &kubecontainer.PodStatus{
ContainerStatuses: []*kubecontainer.Status{
Expand Down

0 comments on commit 3bf2826

Please sign in to comment.