Skip to content

NO-JIRA: cleanup : remove kubelet version check logic #1951

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

NO-JIRA: cleanup : remove kubelet version check logic #1951

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
53 changes: 2 additions & 51 deletions pkg/operator/staticpod/installerpod/cmd.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,17 +3,17 @@ package installerpod
import (
"context"
"fmt"
"k8s.io/utils/clock"
"os"
"path"
"sort"
"strconv"
"strings"
"time"

"k8s.io/utils/clock"

"k8s.io/apimachinery/pkg/util/wait"

"github.com/blang/semver/v4"
"github.com/davecgh/go-spew/spew"
"github.com/spf13/cobra"
"github.com/spf13/pflag"
Expand All @@ -22,7 +22,6 @@ import (
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/util/sets"
"k8s.io/apimachinery/pkg/util/uuid"
"k8s.io/apiserver/pkg/server"
"k8s.io/client-go/kubernetes"
"k8s.io/client-go/rest"
Expand Down Expand Up @@ -67,8 +66,6 @@ type InstallOptions struct {
StaticPodManifestsLockFile string

PodMutationFns []PodMutationFunc

KubeletVersion string
}

// PodMutationFunc is a function that has a chance at changing the pod before it is created
Expand Down Expand Up @@ -208,14 +205,6 @@ func (o *InstallOptions) prefixFor(name string) string {
return name[0 : len(name)-len(fmt.Sprintf("-%s", o.Revision))]
}

func (o *InstallOptions) kubeletVersion(ctx context.Context) (string, error) {
node, err := o.KubeClient.CoreV1().Nodes().Get(ctx, o.NodeName, metav1.GetOptions{})
if err != nil {
return "", err
}
return node.Status.NodeInfo.KubeletVersion, nil
}

func (o *InstallOptions) copySecretsAndConfigMaps(ctx context.Context, resourceDir string,
secretNames, optionalSecretNames, configNames, optionalConfigNames sets.Set[string], prefixed bool) error {
klog.Infof("Creating target resource directory %q ...", resourceDir)
Expand Down Expand Up @@ -428,22 +417,6 @@ func (o *InstallOptions) Run(ctx context.Context) error {
return err
}

klog.Infof("Querying kubelet version for node %s", o.NodeName)
err = retry.RetryOnConnectionErrors(ctx, func(context.Context) (bool, error) {
version, err := o.kubeletVersion(ctx)
if err != nil {
return false, err
}
// trim 'v' from 'v1.22.1' because semver parsing don't like it
o.KubeletVersion = strings.TrimPrefix(version, "v")
return true, nil
})
if err != nil || len(o.KubeletVersion) == 0 {
klog.Warningf("unable to get kubelet version for node %q: %v", o.NodeName, err)
} else {
klog.Infof("Got kubelet version %s on target node %s", o.KubeletVersion, o.NodeName)
}

recorder := events.NewRecorder(o.KubeClient.CoreV1().Events(o.Namespace), "static-pod-installer", eventTarget, o.Clock)
if err := o.copyContent(ctx); err != nil {
recorder.Warningf("StaticPodInstallerFailed", "Installing revision %s: %v", o.Revision, err)
Expand Down Expand Up @@ -565,21 +538,6 @@ func (o *InstallOptions) getInstallerPodsOnThisNode(ctx context.Context) ([]*cor
return installerPodsOnThisNode, nil
}

func (o *InstallOptions) installerPodNeedUUID() bool {
kubeletVersion, err := semver.Make(o.KubeletVersion)
if err != nil {
klog.Warningf("Failed to parse kubelet version %q to semver: %v (we will not generate pod UID)", o.KubeletVersion, err)
return false
}
// if we run kubelet older than 4.9, installer pods require UID generation
// Switching this to 1.23 because there was a fix backported late to 1.22: https://github.com/kubernetes/kubernetes/pull/106394
// Once we prove this out in 1.23 after the kube bump, we can consider relaxing the constraint to 1.22.Z, but TRT
// suspects that this is failing installs due to
// hyperkube[1371]: I1202 08:50:47.935427 1371 status_manager.go:611] "Pod was deleted and then recreated, skipping status update" pod="openshift-kube-scheduler/openshift-kube-scheduler-ip-10-0-129-204.us-east-2.compute.internal" oldPodUID=f11e314508a00e4ea9bf37d76b82b162 podUID=cce59ead72804895d31dba4208b395aa
// in kubelet logs
return kubeletVersion.LT(semver.MustParse("1.23.0"))
}

func (o *InstallOptions) writePod(rawPodBytes []byte, manifestFileName, resourceDir string) error {
// the kubelet has a bug that prevents graceful termination from working on static pods with the same name, filename
// and uuid. By setting the pod UID we can work around the kubelet bug and get our graceful termination honored.
Expand All @@ -589,13 +547,6 @@ func (o *InstallOptions) writePod(rawPodBytes []byte, manifestFileName, resource
return err
}

// If the kubelet version is >=4.9 (1.22), the installer pod UID does not need to be set in the file.
if o.installerPodNeedUUID() {
pod.UID = uuid.NewUUID()
} else {
pod.UID = ""
}

for _, fn := range o.PodMutationFns {
klog.V(2).Infof("Customizing static pod ...")
pod = pod.DeepCopy()
Expand Down
13 changes: 0 additions & 13 deletions pkg/operator/staticpod/installerpod/cmd_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -245,19 +245,6 @@ func TestCopyContent(t *testing.T) {
}
}

func TestKubeletVersion(t *testing.T) {
o := &InstallOptions{}
o.KubeletVersion = "1.23.1+1b2affc"
if o.installerPodNeedUUID() {
t.Fatalf("kubelet \"v1.22.1+1b2affc\" does not need UID")
}

o.KubeletVersion = "1.20.0+b12afff"
if !o.installerPodNeedUUID() {
t.Fatalf("kubelet \"v1.20.0+1b2affc\" need UID")
}
}

func checkFileContent(t *testing.T, file, expected string) {
actual, err := os.ReadFile(file)
if err != nil {
Expand Down