Merge pull request #128763 from srivastav-abhishek/fix-err-string

Fixed failing UT TestWriteKubeletConfigFiles by removing privilege check and adding proper error handling
This commit is contained in:
Kubernetes Prow Robot 2024-11-13 18:54:47 +00:00 committed by GitHub
commit deecaf73eb
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
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."
)