remove HashWithoutResources field

This commit is contained in:
HirazawaUi 2024-04-10 22:09:14 +08:00
parent f6b650430a
commit 3ec13c5e37
8 changed files with 31 additions and 56 deletions

View File

@ -261,15 +261,14 @@ func ConvertPodStatusToRunningPod(runtimeName string, podStatus *PodStatus) Pod
continue continue
} }
container := &Container{ container := &Container{
ID: containerStatus.ID, ID: containerStatus.ID,
Name: containerStatus.Name, Name: containerStatus.Name,
Image: containerStatus.Image, Image: containerStatus.Image,
ImageID: containerStatus.ImageID, ImageID: containerStatus.ImageID,
ImageRef: containerStatus.ImageRef, ImageRef: containerStatus.ImageRef,
ImageRuntimeHandler: containerStatus.ImageRuntimeHandler, ImageRuntimeHandler: containerStatus.ImageRuntimeHandler,
Hash: containerStatus.Hash, Hash: containerStatus.Hash,
HashWithoutResources: containerStatus.HashWithoutResources, State: containerStatus.State,
State: containerStatus.State,
} }
runningPod.Containers = append(runningPod.Containers, container) runningPod.Containers = append(runningPod.Containers, container)
} }

View File

@ -295,11 +295,6 @@ type Container struct {
// Hash of the container, used for comparison. Optional for containers // Hash of the container, used for comparison. Optional for containers
// not managed by kubelet. // not managed by kubelet.
Hash uint64 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 is the state of the container.
State State State State
} }
@ -365,8 +360,6 @@ type Status struct {
ImageRuntimeHandler string ImageRuntimeHandler string
// Hash of the container, used for comparison. // Hash of the container, used for comparison.
Hash uint64 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. // Number of times that the container has been restarted.
RestartCount int RestartCount int
// A string explains why container is in such a status. // A string explains why container is in such a status.

View File

@ -102,15 +102,14 @@ func (m *kubeGenericRuntimeManager) toKubeContainer(c *runtimeapi.Container) (*k
annotatedInfo := getContainerInfoFromAnnotations(c.Annotations) annotatedInfo := getContainerInfoFromAnnotations(c.Annotations)
return &kubecontainer.Container{ return &kubecontainer.Container{
ID: kubecontainer.ContainerID{Type: m.runtimeName, ID: c.Id}, ID: kubecontainer.ContainerID{Type: m.runtimeName, ID: c.Id},
Name: c.GetMetadata().GetName(), Name: c.GetMetadata().GetName(),
ImageID: imageID, ImageID: imageID,
ImageRef: c.ImageRef, ImageRef: c.ImageRef,
ImageRuntimeHandler: c.Image.RuntimeHandler, ImageRuntimeHandler: c.Image.RuntimeHandler,
Image: c.Image.Image, Image: c.Image.Image,
Hash: annotatedInfo.Hash, Hash: annotatedInfo.Hash,
HashWithoutResources: annotatedInfo.HashWithoutResources, State: toKubeContainerState(c.State),
State: toKubeContainerState(c.State),
}, nil }, nil
} }

View File

@ -622,17 +622,16 @@ func toKubeContainerStatus(status *runtimeapi.ContainerStatus, runtimeName strin
Type: runtimeName, Type: runtimeName,
ID: status.Id, ID: status.Id,
}, },
Name: labeledInfo.ContainerName, Name: labeledInfo.ContainerName,
Image: status.Image.Image, Image: status.Image.Image,
ImageID: imageID, ImageID: imageID,
ImageRef: status.ImageRef, ImageRef: status.ImageRef,
ImageRuntimeHandler: status.Image.RuntimeHandler, ImageRuntimeHandler: status.Image.RuntimeHandler,
Hash: annotatedInfo.Hash, Hash: annotatedInfo.Hash,
HashWithoutResources: annotatedInfo.HashWithoutResources, RestartCount: annotatedInfo.RestartCount,
RestartCount: annotatedInfo.RestartCount, State: toKubeContainerState(status.State),
State: toKubeContainerState(status.State), CreatedAt: time.Unix(0, status.CreatedAt),
CreatedAt: time.Unix(0, status.CreatedAt), Resources: cStatusResources,
Resources: cStatusResources,
} }
if status.State != runtimeapi.ContainerState_CONTAINER_CREATED { if status.State != runtimeapi.ContainerState_CONTAINER_CREATED {

View File

@ -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) 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) { func containerChanged(container *v1.Container, containerStatus *kubecontainer.Status) (uint64, uint64, bool) {
expectedHash := kubecontainer.HashContainer(container) expectedHash := kubecontainer.HashContainer(container)
return expectedHash, containerStatus.Hash, containerStatus.Hash != expectedHash return expectedHash, containerStatus.Hash, containerStatus.Hash != expectedHash
@ -981,10 +985,7 @@ func (m *kubeGenericRuntimeManager) computePodActions(ctx context.Context, pod *
var message string var message string
var reason containerKillReason var reason containerKillReason
restart := shouldRestartOnFailure(pod) restart := shouldRestartOnFailure(pod)
// Do not restart if only the Resources field has changed with InPlacePodVerticalScaling enabled if _, _, changed := containerChanged(&container, containerStatus); changed {
if _, _, changed := containerChanged(&container, containerStatus); changed &&
(!isInPlacePodVerticalScalingAllowed(pod) ||
kubecontainer.HashContainer(&container) != containerStatus.HashWithoutResources) {
message = fmt.Sprintf("Container %s definition changed", container.Name) message = fmt.Sprintf("Container %s definition changed", container.Name)
// Restart regardless of the restart policy because the container // Restart regardless of the restart policy because the container
// spec changed. // spec changed.

View File

@ -2436,7 +2436,6 @@ func TestComputePodActionsForPodResize(t *testing.T) {
// compute hash // compute hash
if kcs := kps.FindContainerStatusByName(pod.Spec.Containers[idx].Name); kcs != nil { if kcs := kps.FindContainerStatusByName(pod.Spec.Containers[idx].Name); kcs != nil {
kcs.Hash = kubecontainer.HashContainer(&pod.Spec.Containers[idx]) kcs.Hash = kubecontainer.HashContainer(&pod.Spec.Containers[idx])
kcs.HashWithoutResources = kubecontainer.HashContainer(&pod.Spec.Containers[idx])
} }
} }
makeAndSetFakePod(t, m, fakeRuntime, pod) makeAndSetFakePod(t, m, fakeRuntime, pod)
@ -2452,7 +2451,6 @@ func TestComputePodActionsForPodResize(t *testing.T) {
for idx := range pod.Spec.Containers { for idx := range pod.Spec.Containers {
if kcs := kps.FindContainerStatusByName(pod.Spec.Containers[idx].Name); kcs != nil { if kcs := kps.FindContainerStatusByName(pod.Spec.Containers[idx].Name); kcs != nil {
kcs.Hash = kubecontainer.HashContainer(&pod.Spec.Containers[idx]) kcs.Hash = kubecontainer.HashContainer(&pod.Spec.Containers[idx])
kcs.HashWithoutResources = kubecontainer.HashContainer(&pod.Spec.Containers[idx])
} }
} }
if test.mutatePodFn != nil { if test.mutatePodFn != nil {

View File

@ -22,10 +22,8 @@ import (
v1 "k8s.io/api/core/v1" v1 "k8s.io/api/core/v1"
kubetypes "k8s.io/apimachinery/pkg/types" kubetypes "k8s.io/apimachinery/pkg/types"
utilfeature "k8s.io/apiserver/pkg/util/feature"
"k8s.io/klog/v2" "k8s.io/klog/v2"
"k8s.io/kubelet/pkg/types" "k8s.io/kubelet/pkg/types"
"k8s.io/kubernetes/pkg/features"
kubecontainer "k8s.io/kubernetes/pkg/kubelet/container" kubecontainer "k8s.io/kubernetes/pkg/kubelet/container"
) )
@ -35,7 +33,6 @@ const (
podTerminationGracePeriodLabel = "io.kubernetes.pod.terminationGracePeriod" podTerminationGracePeriodLabel = "io.kubernetes.pod.terminationGracePeriod"
containerHashLabel = "io.kubernetes.container.hash" containerHashLabel = "io.kubernetes.container.hash"
containerHashWithoutResourcesLabel = "io.kubernetes.container.hashWithoutResources"
containerRestartCountLabel = "io.kubernetes.container.restartCount" containerRestartCountLabel = "io.kubernetes.container.restartCount"
containerTerminationMessagePathLabel = "io.kubernetes.container.terminationMessagePath" containerTerminationMessagePathLabel = "io.kubernetes.container.terminationMessagePath"
containerTerminationMessagePolicyLabel = "io.kubernetes.container.terminationMessagePolicy" containerTerminationMessagePolicyLabel = "io.kubernetes.container.terminationMessagePolicy"
@ -65,7 +62,6 @@ type labeledContainerInfo struct {
type annotatedContainerInfo struct { type annotatedContainerInfo struct {
Hash uint64 Hash uint64
HashWithoutResources uint64
RestartCount int RestartCount int
PodDeletionGracePeriod *int64 PodDeletionGracePeriod *int64
PodTerminationGracePeriod *int64 PodTerminationGracePeriod *int64
@ -117,9 +113,6 @@ func newContainerAnnotations(container *v1.Container, pod *v1.Pod, restartCount
} }
annotations[containerHashLabel] = strconv.FormatUint(kubecontainer.HashContainer(container), 16) 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[containerRestartCountLabel] = strconv.Itoa(restartCount)
annotations[containerTerminationMessagePathLabel] = container.TerminationMessagePath annotations[containerTerminationMessagePathLabel] = container.TerminationMessagePath
annotations[containerTerminationMessagePolicyLabel] = string(container.TerminationMessagePolicy) 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 { if containerInfo.Hash, err = getUint64ValueFromLabel(annotations, containerHashLabel); err != nil {
klog.ErrorS(err, "Unable to get label value from annotations", "label", containerHashLabel, "annotations", annotations) 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 { if containerInfo.RestartCount, err = getIntValueFromLabel(annotations, containerRestartCountLabel); err != nil {
klog.ErrorS(err, "Unable to get label value from annotations", "label", containerRestartCountLabel, "annotations", annotations) klog.ErrorS(err, "Unable to get label value from annotations", "label", containerRestartCountLabel, "annotations", annotations)
} }

View File

@ -155,7 +155,6 @@ func TestContainerAnnotations(t *testing.T) {
PodDeletionGracePeriod: pod.DeletionGracePeriodSeconds, PodDeletionGracePeriod: pod.DeletionGracePeriodSeconds,
PodTerminationGracePeriod: pod.Spec.TerminationGracePeriodSeconds, PodTerminationGracePeriod: pod.Spec.TerminationGracePeriodSeconds,
Hash: kubecontainer.HashContainer(container), Hash: kubecontainer.HashContainer(container),
HashWithoutResources: kubecontainer.HashContainer(container),
RestartCount: restartCount, RestartCount: restartCount,
TerminationMessagePath: container.TerminationMessagePath, TerminationMessagePath: container.TerminationMessagePath,
PreStopHandler: container.Lifecycle.PreStop, PreStopHandler: container.Lifecycle.PreStop,
@ -182,7 +181,6 @@ func TestContainerAnnotations(t *testing.T) {
expected.PreStopHandler = nil expected.PreStopHandler = nil
// Because container is changed, the Hash should be updated // Because container is changed, the Hash should be updated
expected.Hash = kubecontainer.HashContainer(container) expected.Hash = kubecontainer.HashContainer(container)
expected.HashWithoutResources = kubecontainer.HashContainer(container)
annotations = newContainerAnnotations(container, pod, restartCount, opts) annotations = newContainerAnnotations(container, pod, restartCount, opts)
containerInfo = getContainerInfoFromAnnotations(annotations) containerInfo = getContainerInfoFromAnnotations(annotations)
if !reflect.DeepEqual(containerInfo, expected) { if !reflect.DeepEqual(containerInfo, expected) {