diff --git a/pkg/kubelet/container/helpers.go b/pkg/kubelet/container/helpers.go index bcd20643271..7f8963ab6a1 100644 --- a/pkg/kubelet/container/helpers.go +++ b/pkg/kubelet/container/helpers.go @@ -261,15 +261,14 @@ func ConvertPodStatusToRunningPod(runtimeName string, podStatus *PodStatus) Pod continue } container := &Container{ - ID: containerStatus.ID, - Name: containerStatus.Name, - Image: containerStatus.Image, - ImageID: containerStatus.ImageID, - ImageRef: containerStatus.ImageRef, - ImageRuntimeHandler: containerStatus.ImageRuntimeHandler, - Hash: containerStatus.Hash, - HashWithoutResources: containerStatus.HashWithoutResources, - State: containerStatus.State, + ID: containerStatus.ID, + Name: containerStatus.Name, + Image: containerStatus.Image, + ImageID: containerStatus.ImageID, + ImageRef: containerStatus.ImageRef, + ImageRuntimeHandler: containerStatus.ImageRuntimeHandler, + Hash: containerStatus.Hash, + State: containerStatus.State, } runningPod.Containers = append(runningPod.Containers, container) } diff --git a/pkg/kubelet/container/runtime.go b/pkg/kubelet/container/runtime.go index b53214ca017..539c97bb0e4 100644 --- a/pkg/kubelet/container/runtime.go +++ b/pkg/kubelet/container/runtime.go @@ -295,11 +295,6 @@ type Container struct { // Hash of the container, used for comparison. Optional for containers // not managed by kubelet. Hash uint64 - // Hash of the container over fields with Resources field zero'd out. - // NOTE: This is needed during alpha and beta so that containers using Resources are - // not unexpectedly restarted when InPlacePodVerticalScaling feature-gate is toggled. - //TODO(vinaykul,InPlacePodVerticalScaling): Remove this in GA+1 and make HashWithoutResources to become Hash. - HashWithoutResources uint64 // State is the state of the container. State State } @@ -365,8 +360,6 @@ type Status struct { ImageRuntimeHandler string // Hash of the container, used for comparison. Hash uint64 - // Hash of the container over fields with Resources field zero'd out. - HashWithoutResources uint64 // Number of times that the container has been restarted. RestartCount int // A string explains why container is in such a status. diff --git a/pkg/kubelet/kuberuntime/helpers.go b/pkg/kubelet/kuberuntime/helpers.go index a257e52d75f..85961cb0280 100644 --- a/pkg/kubelet/kuberuntime/helpers.go +++ b/pkg/kubelet/kuberuntime/helpers.go @@ -102,15 +102,14 @@ func (m *kubeGenericRuntimeManager) toKubeContainer(c *runtimeapi.Container) (*k annotatedInfo := getContainerInfoFromAnnotations(c.Annotations) return &kubecontainer.Container{ - ID: kubecontainer.ContainerID{Type: m.runtimeName, ID: c.Id}, - Name: c.GetMetadata().GetName(), - ImageID: imageID, - ImageRef: c.ImageRef, - ImageRuntimeHandler: c.Image.RuntimeHandler, - Image: c.Image.Image, - Hash: annotatedInfo.Hash, - HashWithoutResources: annotatedInfo.HashWithoutResources, - State: toKubeContainerState(c.State), + ID: kubecontainer.ContainerID{Type: m.runtimeName, ID: c.Id}, + Name: c.GetMetadata().GetName(), + ImageID: imageID, + ImageRef: c.ImageRef, + ImageRuntimeHandler: c.Image.RuntimeHandler, + Image: c.Image.Image, + Hash: annotatedInfo.Hash, + State: toKubeContainerState(c.State), }, nil } diff --git a/pkg/kubelet/kuberuntime/kuberuntime_container.go b/pkg/kubelet/kuberuntime/kuberuntime_container.go index 3b534f1536f..4d84643dfae 100644 --- a/pkg/kubelet/kuberuntime/kuberuntime_container.go +++ b/pkg/kubelet/kuberuntime/kuberuntime_container.go @@ -622,17 +622,16 @@ func toKubeContainerStatus(status *runtimeapi.ContainerStatus, runtimeName strin Type: runtimeName, ID: status.Id, }, - Name: labeledInfo.ContainerName, - Image: status.Image.Image, - ImageID: imageID, - ImageRef: status.ImageRef, - ImageRuntimeHandler: status.Image.RuntimeHandler, - Hash: annotatedInfo.Hash, - HashWithoutResources: annotatedInfo.HashWithoutResources, - RestartCount: annotatedInfo.RestartCount, - State: toKubeContainerState(status.State), - CreatedAt: time.Unix(0, status.CreatedAt), - Resources: cStatusResources, + Name: labeledInfo.ContainerName, + Image: status.Image.Image, + ImageID: imageID, + ImageRef: status.ImageRef, + ImageRuntimeHandler: status.Image.RuntimeHandler, + Hash: annotatedInfo.Hash, + RestartCount: annotatedInfo.RestartCount, + State: toKubeContainerState(status.State), + CreatedAt: time.Unix(0, status.CreatedAt), + Resources: cStatusResources, } if status.State != runtimeapi.ContainerState_CONTAINER_CREATED { diff --git a/pkg/kubelet/kuberuntime/kuberuntime_manager.go b/pkg/kubelet/kuberuntime/kuberuntime_manager.go index b81cffee064..716ff486385 100644 --- a/pkg/kubelet/kuberuntime/kuberuntime_manager.go +++ b/pkg/kubelet/kuberuntime/kuberuntime_manager.go @@ -518,6 +518,10 @@ func (p podActions) String() string { p.KillPod, p.CreateSandbox, p.UpdatePodResources, p.Attempt, p.InitContainersToStart, p.ContainersToStart, p.EphemeralContainersToStart, p.ContainersToUpdate, p.ContainersToKill) } +// containerChanged will determine whether the container has changed based on the fields that will affect the running of the container. +// Currently, there are only `image` and `name` fields. +// we don't need to consider the pod UID here, because we find the containerStatus through the pod UID. +// If the pod UID changes, we will not be able to find the containerStatus to compare against. func containerChanged(container *v1.Container, containerStatus *kubecontainer.Status) (uint64, uint64, bool) { expectedHash := kubecontainer.HashContainer(container) return expectedHash, containerStatus.Hash, containerStatus.Hash != expectedHash @@ -981,10 +985,7 @@ func (m *kubeGenericRuntimeManager) computePodActions(ctx context.Context, pod * var message string var reason containerKillReason restart := shouldRestartOnFailure(pod) - // Do not restart if only the Resources field has changed with InPlacePodVerticalScaling enabled - if _, _, changed := containerChanged(&container, containerStatus); changed && - (!isInPlacePodVerticalScalingAllowed(pod) || - kubecontainer.HashContainer(&container) != containerStatus.HashWithoutResources) { + if _, _, changed := containerChanged(&container, containerStatus); changed { message = fmt.Sprintf("Container %s definition changed", container.Name) // Restart regardless of the restart policy because the container // spec changed. diff --git a/pkg/kubelet/kuberuntime/kuberuntime_manager_test.go b/pkg/kubelet/kuberuntime/kuberuntime_manager_test.go index 3fc2519f6aa..7b3519635ad 100644 --- a/pkg/kubelet/kuberuntime/kuberuntime_manager_test.go +++ b/pkg/kubelet/kuberuntime/kuberuntime_manager_test.go @@ -2436,7 +2436,6 @@ func TestComputePodActionsForPodResize(t *testing.T) { // compute hash if kcs := kps.FindContainerStatusByName(pod.Spec.Containers[idx].Name); kcs != nil { kcs.Hash = kubecontainer.HashContainer(&pod.Spec.Containers[idx]) - kcs.HashWithoutResources = kubecontainer.HashContainer(&pod.Spec.Containers[idx]) } } makeAndSetFakePod(t, m, fakeRuntime, pod) @@ -2452,7 +2451,6 @@ func TestComputePodActionsForPodResize(t *testing.T) { for idx := range pod.Spec.Containers { if kcs := kps.FindContainerStatusByName(pod.Spec.Containers[idx].Name); kcs != nil { kcs.Hash = kubecontainer.HashContainer(&pod.Spec.Containers[idx]) - kcs.HashWithoutResources = kubecontainer.HashContainer(&pod.Spec.Containers[idx]) } } if test.mutatePodFn != nil { diff --git a/pkg/kubelet/kuberuntime/labels.go b/pkg/kubelet/kuberuntime/labels.go index edec7785a28..3611bebff0f 100644 --- a/pkg/kubelet/kuberuntime/labels.go +++ b/pkg/kubelet/kuberuntime/labels.go @@ -22,10 +22,8 @@ import ( v1 "k8s.io/api/core/v1" kubetypes "k8s.io/apimachinery/pkg/types" - utilfeature "k8s.io/apiserver/pkg/util/feature" "k8s.io/klog/v2" "k8s.io/kubelet/pkg/types" - "k8s.io/kubernetes/pkg/features" kubecontainer "k8s.io/kubernetes/pkg/kubelet/container" ) @@ -35,7 +33,6 @@ const ( podTerminationGracePeriodLabel = "io.kubernetes.pod.terminationGracePeriod" containerHashLabel = "io.kubernetes.container.hash" - containerHashWithoutResourcesLabel = "io.kubernetes.container.hashWithoutResources" containerRestartCountLabel = "io.kubernetes.container.restartCount" containerTerminationMessagePathLabel = "io.kubernetes.container.terminationMessagePath" containerTerminationMessagePolicyLabel = "io.kubernetes.container.terminationMessagePolicy" @@ -65,7 +62,6 @@ type labeledContainerInfo struct { type annotatedContainerInfo struct { Hash uint64 - HashWithoutResources uint64 RestartCount int PodDeletionGracePeriod *int64 PodTerminationGracePeriod *int64 @@ -117,9 +113,6 @@ func newContainerAnnotations(container *v1.Container, pod *v1.Pod, restartCount } annotations[containerHashLabel] = strconv.FormatUint(kubecontainer.HashContainer(container), 16) - if utilfeature.DefaultFeatureGate.Enabled(features.InPlacePodVerticalScaling) { - annotations[containerHashWithoutResourcesLabel] = strconv.FormatUint(kubecontainer.HashContainer(container), 16) - } annotations[containerRestartCountLabel] = strconv.Itoa(restartCount) annotations[containerTerminationMessagePathLabel] = container.TerminationMessagePath annotations[containerTerminationMessagePolicyLabel] = string(container.TerminationMessagePolicy) @@ -200,11 +193,6 @@ func getContainerInfoFromAnnotations(annotations map[string]string) *annotatedCo if containerInfo.Hash, err = getUint64ValueFromLabel(annotations, containerHashLabel); err != nil { klog.ErrorS(err, "Unable to get label value from annotations", "label", containerHashLabel, "annotations", annotations) } - if utilfeature.DefaultFeatureGate.Enabled(features.InPlacePodVerticalScaling) { - if containerInfo.HashWithoutResources, err = getUint64ValueFromLabel(annotations, containerHashWithoutResourcesLabel); err != nil { - klog.ErrorS(err, "Unable to get label value from annotations", "label", containerHashWithoutResourcesLabel, "annotations", annotations) - } - } if containerInfo.RestartCount, err = getIntValueFromLabel(annotations, containerRestartCountLabel); err != nil { klog.ErrorS(err, "Unable to get label value from annotations", "label", containerRestartCountLabel, "annotations", annotations) } diff --git a/pkg/kubelet/kuberuntime/labels_test.go b/pkg/kubelet/kuberuntime/labels_test.go index 7a7a0fef706..439a52a7ef4 100644 --- a/pkg/kubelet/kuberuntime/labels_test.go +++ b/pkg/kubelet/kuberuntime/labels_test.go @@ -155,7 +155,6 @@ func TestContainerAnnotations(t *testing.T) { PodDeletionGracePeriod: pod.DeletionGracePeriodSeconds, PodTerminationGracePeriod: pod.Spec.TerminationGracePeriodSeconds, Hash: kubecontainer.HashContainer(container), - HashWithoutResources: kubecontainer.HashContainer(container), RestartCount: restartCount, TerminationMessagePath: container.TerminationMessagePath, PreStopHandler: container.Lifecycle.PreStop, @@ -182,7 +181,6 @@ func TestContainerAnnotations(t *testing.T) { expected.PreStopHandler = nil // Because container is changed, the Hash should be updated expected.Hash = kubecontainer.HashContainer(container) - expected.HashWithoutResources = kubecontainer.HashContainer(container) annotations = newContainerAnnotations(container, pod, restartCount, opts) containerInfo = getContainerInfoFromAnnotations(annotations) if !reflect.DeepEqual(containerInfo, expected) {