Fixed failing test by removing privilege check and some refactor

Addressed review comments
This commit is contained in:
Abhishek Kr Srivastav 2024-11-12 17:57:06 +05:30
parent 6b031e50b2
commit 56e3c787a5
5 changed files with 31 additions and 98 deletions

View File

@ -28,6 +28,10 @@ import (
"k8s.io/kubernetes/cmd/kubeadm/app/phases/upgrade"
)
const (
kubeletConfigDir = ""
)
var (
kubeletConfigLongDesc = cmdutil.LongDesc(`
Upgrade the kubelet configuration for this node by downloading it from the kubelet-config ConfigMap stored in the cluster
@ -60,7 +64,7 @@ func runKubeletConfigPhase(c workflow.RunData) error {
// Write the configuration for the kubelet down to disk and print the generated manifests instead of dry-running.
// If not dry-running, the kubelet config file will be backed up to the /etc/kubernetes/tmp/ dir, so that it could be
// recovered if anything goes wrong.
err := upgrade.WriteKubeletConfigFiles(data.InitCfg(), data.PatchesDir(), data.DryRun(), data.OutputWriter())
err := upgrade.WriteKubeletConfigFiles(data.InitCfg(), kubeletConfigDir, data.PatchesDir(), data.DryRun(), data.OutputWriter())
if err != nil {
return err
}

View File

@ -96,16 +96,21 @@ func UnupgradedControlPlaneInstances(client clientset.Interface, nodeName string
}
// WriteKubeletConfigFiles writes the kubelet config file to disk, but first creates a backup of any existing one.
func WriteKubeletConfigFiles(cfg *kubeadmapi.InitConfiguration, patchesDir string, dryRun bool, out io.Writer) error {
// Set up the kubelet directory to use. If dry-running, this will return a fake directory
kubeletDir, err := GetKubeletDir(dryRun)
func WriteKubeletConfigFiles(cfg *kubeadmapi.InitConfiguration, kubeletConfigDir string, patchesDir string, dryRun bool, out io.Writer) error {
var (
err error
kubeletDir = kubeadmconstants.KubeletRunDirectory
)
// If dry-running, this will return a directory under /etc/kubernetes/tmp or kubeletConfigDir.
if dryRun {
kubeletDir, err = kubeadmconstants.CreateTempDir(kubeletConfigDir, "kubeadm-upgrade-dryrun")
}
if err != nil {
// The error here should never occur in reality, would only be thrown if /tmp doesn't exist on the machine.
return err
}
// Create a copy of the kubelet config file in the /etc/kubernetes/tmp/ folder.
backupDir, err := kubeadmconstants.CreateTempDir("", "kubeadm-kubelet-config")
// Create a copy of the kubelet config file in the /etc/kubernetes/tmp or kubeletConfigDir.
backupDir, err := kubeadmconstants.CreateTempDir(kubeletConfigDir, "kubeadm-kubelet-config")
if err != nil {
return err
}
@ -178,14 +183,6 @@ func WriteKubeletConfigFiles(cfg *kubeadmapi.InitConfiguration, patchesDir strin
return nil
}
// GetKubeletDir gets the kubelet directory based on whether the user is dry-running this command or not.
func GetKubeletDir(dryRun bool) (string, error) {
if dryRun {
return kubeadmconstants.CreateTempDir("", "kubeadm-upgrade-dryrun")
}
return kubeadmconstants.KubeletRunDirectory, nil
}
// UpdateKubeletLocalMode changes the Server URL in the kubelets kubeconfig to the local API endpoint if it is currently
// set to the ControlPlaneEndpoint.
// TODO: remove this function once kubeletKubeConfigFilePath goes GA and is hardcoded to enabled by default:

View File

@ -1,24 +0,0 @@
//go:build !windows
// +build !windows
/*
Copyright 2023 The Kubernetes Authors.
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/
package upgrade
const (
missingKubeletConfig = "no kubelet component config found.*no such file or directory"
)

View File

@ -20,7 +20,6 @@ import (
"os"
"path/filepath"
"reflect"
"regexp"
"strings"
"testing"
@ -35,7 +34,6 @@ import (
kubeadmapi "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm"
"k8s.io/kubernetes/cmd/kubeadm/app/componentconfigs"
"k8s.io/kubernetes/cmd/kubeadm/app/constants"
"k8s.io/kubernetes/cmd/kubeadm/app/preflight"
testutil "k8s.io/kubernetes/cmd/kubeadm/test"
)
@ -114,26 +112,15 @@ func TestRollbackFiles(t *testing.T) {
}
func TestWriteKubeletConfigFiles(t *testing.T) {
// exit early if the user doesn't have root permission as the test needs to create /etc/kubernetes directory
// while the permission should be granted to the user.
isPrivileged := preflight.IsPrivilegedUserCheck{}
if _, err := isPrivileged.Check(); err != nil {
return
}
tempDir := t.TempDir()
testCases := []struct {
name string
dryrun bool
patchesDir string
errPattern string
cfg *kubeadmapi.InitConfiguration
name string
patchesDir string
expectedError bool
cfg *kubeadmapi.InitConfiguration
}{
// Be careful that if the dryrun is set to false and the test is run on a live cluster, the kubelet config file might be overwritten.
// However, you should be able to find the original config file in /etc/kubernetes/tmp/kubeadm-kubelet-configxxx folder.
// The test haven't clean up the temporary file created under /etc/kubernetes/tmp/ as that could be accidentally delete other files in
// that folder as well which might be unexpected.
{
name: "write kubelet config file successfully",
dryrun: true,
name: "write kubelet config file successfully",
cfg: &kubeadmapi.InitConfiguration{
ClusterConfiguration: kubeadmapi.ClusterConfiguration{
ComponentConfigs: kubeadmapi.ComponentConfigMap{
@ -143,16 +130,14 @@ func TestWriteKubeletConfigFiles(t *testing.T) {
},
},
{
name: "aggregate errs: no kubelet config file and cannot read config file",
dryrun: true,
errPattern: missingKubeletConfig,
cfg: &kubeadmapi.InitConfiguration{},
name: "aggregate errs: no kubelet config file and cannot read config file",
expectedError: true,
cfg: &kubeadmapi.InitConfiguration{},
},
{
name: "only one err: patch dir does not exist",
dryrun: true,
patchesDir: "Bogus",
errPattern: "could not list patch files for path \"Bogus\"",
name: "only one err: patch dir does not exist",
patchesDir: "Bogus",
expectedError: true,
cfg: &kubeadmapi.InitConfiguration{
ClusterConfiguration: kubeadmapi.ClusterConfiguration{
ComponentConfigs: kubeadmapi.ComponentConfigMap{
@ -163,14 +148,9 @@ func TestWriteKubeletConfigFiles(t *testing.T) {
},
}
for _, tc := range testCases {
err := WriteKubeletConfigFiles(tc.cfg, tc.patchesDir, tc.dryrun, os.Stdout)
if err != nil && tc.errPattern != "" {
if match, _ := regexp.MatchString(tc.errPattern, err.Error()); !match {
t.Fatalf("Expected error contains %q, got %v", tc.errPattern, err.Error())
}
}
if err == nil && len(tc.errPattern) != 0 {
t.Fatalf("WriteKubeletConfigFiles didn't return error expected %s", tc.errPattern)
err := WriteKubeletConfigFiles(tc.cfg, tempDir, tc.patchesDir, true, os.Stdout)
if (err != nil) != tc.expectedError {
t.Fatalf("expected error: %v, got: %v, error: %v", tc.expectedError, err != nil, err)
}
}
}

View File

@ -1,24 +0,0 @@
//go:build windows
// +build windows
/*
Copyright 2023 The Kubernetes Authors.
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/
package upgrade
const (
missingKubeletConfig = "no kubelet component config found.*The system cannot find the file specified."
)