From 6f06cd6e05704a9a7b18e74a048a297e5bdb5498 Mon Sep 17 00:00:00 2001 From: Ondrej Kokes Date: Wed, 27 Nov 2024 15:06:58 +0100 Subject: [PATCH] Do not reload kubeconfig from disk When `kubeadm init phase bootstrap-token` gets invoked, it reads the kubeconfig from disk repeatedly. This is wasteful, but, more importantly, it blocks the use of `/dev/stdin` and other sources of data that cannot be read repeatedly. This change introduces a new field that caches a parsed kubeconfig and when a new clientset is requested, it is converted from this pre-parsed kubeconfig, the code no longer reaches out to disk. --- cmd/kubeadm/app/cmd/init.go | 24 ++++++++++++- .../app/cmd/phases/init/bootstraptoken.go | 6 +++- cmd/kubeadm/app/cmd/phases/init/data.go | 2 ++ cmd/kubeadm/app/cmd/phases/init/data_test.go | 2 ++ .../bootstraptoken/clusterinfo/clusterinfo.go | 17 +++++----- .../clusterinfo/clusterinfo_test.go | 34 +++++-------------- 6 files changed, 50 insertions(+), 35 deletions(-) diff --git a/cmd/kubeadm/app/cmd/init.go b/cmd/kubeadm/app/cmd/init.go index ac8f14235ea..156a1e1fc34 100644 --- a/cmd/kubeadm/app/cmd/init.go +++ b/cmd/kubeadm/app/cmd/init.go @@ -28,6 +28,8 @@ import ( "k8s.io/apimachinery/pkg/util/sets" clientset "k8s.io/client-go/kubernetes" + "k8s.io/client-go/tools/clientcmd" + clientcmdapi "k8s.io/client-go/tools/clientcmd/api" "k8s.io/klog/v2" kubeadmapi "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm" @@ -87,6 +89,7 @@ type initData struct { cfg *kubeadmapi.InitConfiguration skipTokenPrint bool dryRun bool + kubeconfig *clientcmdapi.Config kubeconfigDir string kubeconfigPath string ignorePreflightErrors sets.Set[string] @@ -456,6 +459,21 @@ func (d *initData) CertificateDir() string { return d.certificatesDir } +// KubeConfig returns a kubeconfig after loading it from KubeConfigPath(). +func (d *initData) KubeConfig() (*clientcmdapi.Config, error) { + if d.kubeconfig != nil { + return d.kubeconfig, nil + } + + var err error + d.kubeconfig, err = clientcmd.LoadFromFile(d.KubeConfigPath()) + if err != nil { + return nil, err + } + + return d.kubeconfig, nil +} + // KubeConfigDir returns the path of the Kubernetes configuration folder or the temporary folder path in case of DryRun. func (d *initData) KubeConfigDir() string { if d.dryRun { @@ -536,7 +554,11 @@ func (d *initData) Client() (clientset.Interface, error) { d.adminKubeConfigBootstrapped = true } else { // Alternatively, just load the config pointed at the --kubeconfig path - d.client, err = kubeconfigutil.ClientSetFromFile(d.KubeConfigPath()) + cfg, err := d.KubeConfig() + if err != nil { + return nil, err + } + d.client, err = kubeconfigutil.ToClientSet(cfg) if err != nil { return nil, err } diff --git a/cmd/kubeadm/app/cmd/phases/init/bootstraptoken.go b/cmd/kubeadm/app/cmd/phases/init/bootstraptoken.go index 85c3a2fc39c..21f89e081aa 100644 --- a/cmd/kubeadm/app/cmd/phases/init/bootstraptoken.go +++ b/cmd/kubeadm/app/cmd/phases/init/bootstraptoken.go @@ -72,6 +72,10 @@ func runBootstrapToken(c workflow.RunData) error { if err != nil { return err } + kubeconfig, err := data.KubeConfig() + if err != nil { + return err + } if !data.SkipTokenPrint() { tokens := data.Tokens() @@ -106,7 +110,7 @@ func runBootstrapToken(c workflow.RunData) error { } // Create the cluster-info ConfigMap with the associated RBAC rules - if err := clusterinfophase.CreateBootstrapConfigMapIfNotExists(client, data.KubeConfigPath()); err != nil { + if err := clusterinfophase.CreateBootstrapConfigMapIfNotExists(client, kubeconfig); err != nil { return errors.Wrap(err, "error creating bootstrap ConfigMap") } if err := clusterinfophase.CreateClusterInfoRBACRules(client); err != nil { diff --git a/cmd/kubeadm/app/cmd/phases/init/data.go b/cmd/kubeadm/app/cmd/phases/init/data.go index 4560e03cbc8..c8a385280b0 100644 --- a/cmd/kubeadm/app/cmd/phases/init/data.go +++ b/cmd/kubeadm/app/cmd/phases/init/data.go @@ -22,6 +22,7 @@ import ( "k8s.io/apimachinery/pkg/util/sets" clientset "k8s.io/client-go/kubernetes" + clientcmdapi "k8s.io/client-go/tools/clientcmd/api" kubeadmapi "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm" ) @@ -38,6 +39,7 @@ type InitData interface { IgnorePreflightErrors() sets.Set[string] CertificateWriteDir() string CertificateDir() string + KubeConfig() (*clientcmdapi.Config, error) KubeConfigDir() string KubeConfigPath() string ManifestDir() string diff --git a/cmd/kubeadm/app/cmd/phases/init/data_test.go b/cmd/kubeadm/app/cmd/phases/init/data_test.go index 11c6ea9b536..4e609d2b61e 100644 --- a/cmd/kubeadm/app/cmd/phases/init/data_test.go +++ b/cmd/kubeadm/app/cmd/phases/init/data_test.go @@ -22,6 +22,7 @@ import ( "k8s.io/apimachinery/pkg/util/sets" clientset "k8s.io/client-go/kubernetes" + clientcmdapi "k8s.io/client-go/tools/clientcmd/api" kubeadmapi "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm" ) @@ -41,6 +42,7 @@ func (t *testInitData) SkipTokenPrint() bool { r func (t *testInitData) IgnorePreflightErrors() sets.Set[string] { return nil } func (t *testInitData) CertificateWriteDir() string { return "" } func (t *testInitData) CertificateDir() string { return "" } +func (t *testInitData) KubeConfig() (*clientcmdapi.Config, error) { return nil, nil } func (t *testInitData) KubeConfigDir() string { return "" } func (t *testInitData) KubeConfigPath() string { return "" } func (t *testInitData) ManifestDir() string { return "" } diff --git a/cmd/kubeadm/app/phases/bootstraptoken/clusterinfo/clusterinfo.go b/cmd/kubeadm/app/phases/bootstraptoken/clusterinfo/clusterinfo.go index 20478f6f0b3..fd1097dae7f 100644 --- a/cmd/kubeadm/app/phases/bootstraptoken/clusterinfo/clusterinfo.go +++ b/cmd/kubeadm/app/phases/bootstraptoken/clusterinfo/clusterinfo.go @@ -21,7 +21,7 @@ import ( "github.com/pkg/errors" - "k8s.io/api/core/v1" + v1 "k8s.io/api/core/v1" rbac "k8s.io/api/rbac/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apiserver/pkg/authentication/user" @@ -40,19 +40,20 @@ const ( ) // CreateBootstrapConfigMapIfNotExists creates the kube-public ConfigMap if it doesn't exist already -func CreateBootstrapConfigMapIfNotExists(client clientset.Interface, file string) error { +func CreateBootstrapConfigMapIfNotExists(client clientset.Interface, kubeconfig *clientcmdapi.Config) error { fmt.Printf("[bootstrap-token] Creating the %q ConfigMap in the %q namespace\n", bootstrapapi.ConfigMapClusterInfo, metav1.NamespacePublic) - klog.V(1).Infoln("[bootstrap-token] loading admin kubeconfig") - adminConfig, err := clientcmd.LoadFromFile(file) - if err != nil { - return errors.Wrap(err, "failed to load admin kubeconfig") - } - if err = clientcmdapi.FlattenConfig(adminConfig); err != nil { + // Clone the kubeconfig so that it's not mutated. + adminConfig := kubeconfig.DeepCopy() + if err := clientcmdapi.FlattenConfig(adminConfig); err != nil { return err } + if adminConfig.Contexts[adminConfig.CurrentContext] == nil { + return errors.New("invalid kubeconfig") + } + adminCluster := adminConfig.Contexts[adminConfig.CurrentContext].Cluster // Copy the cluster from admin.conf to the bootstrap kubeconfig, contains the CA cert and the server URL klog.V(1).Infoln("[bootstrap-token] copying the cluster from admin.conf to the bootstrap kubeconfig") diff --git a/cmd/kubeadm/app/phases/bootstraptoken/clusterinfo/clusterinfo_test.go b/cmd/kubeadm/app/phases/bootstraptoken/clusterinfo/clusterinfo_test.go index b352407d8af..74b34d758e3 100644 --- a/cmd/kubeadm/app/phases/bootstraptoken/clusterinfo/clusterinfo_test.go +++ b/cmd/kubeadm/app/phases/bootstraptoken/clusterinfo/clusterinfo_test.go @@ -17,8 +17,8 @@ limitations under the License. package clusterinfo import ( + "bytes" "context" - "os" "testing" "text/template" "time" @@ -31,6 +31,7 @@ import ( "k8s.io/apiserver/pkg/authentication/user" clientsetfake "k8s.io/client-go/kubernetes/fake" core "k8s.io/client-go/testing" + "k8s.io/client-go/tools/clientcmd" bootstrapapi "k8s.io/cluster-bootstrap/token/api" kubeadmapi "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm" @@ -55,34 +56,24 @@ users: func TestCreateBootstrapConfigMapIfNotExists(t *testing.T) { tests := []struct { name string - fileExist bool createErr error expectErr bool }{ { "successful case should have no error", - true, nil, false, }, { "if configmap already exists, return error", - true, apierrors.NewAlreadyExists(schema.GroupResource{Resource: "configmaps"}, "test"), true, }, { "unexpected error should be returned", - true, apierrors.NewUnauthorized("go away!"), true, }, - { - "if the file does not exist, return error", - false, - nil, - true, - }, } servers := []struct { @@ -93,20 +84,12 @@ func TestCreateBootstrapConfigMapIfNotExists(t *testing.T) { } for _, server := range servers { - file, err := os.CreateTemp("", "") - if err != nil { - t.Fatalf("could not create tempfile: %v", err) - } - defer os.Remove(file.Name()) + var buf bytes.Buffer - if err := testConfigTempl.Execute(file, server); err != nil { + if err := testConfigTempl.Execute(&buf, server); err != nil { t.Fatalf("could not write to tempfile: %v", err) } - if err := file.Close(); err != nil { - t.Fatalf("could not close tempfile: %v", err) - } - // Override the default timeouts to be shorter defaultTimeouts := kubeadmapi.GetActiveTimeouts() defaultAPICallTimeout := defaultTimeouts.KubernetesAPICall @@ -124,11 +107,12 @@ func TestCreateBootstrapConfigMapIfNotExists(t *testing.T) { }) } - fileName := file.Name() - if !tc.fileExist { - fileName = "notexistfile" + kubeconfig, err := clientcmd.Load(buf.Bytes()) + if err != nil { + t.Fatal(err) } - err := CreateBootstrapConfigMapIfNotExists(client, fileName) + + err = CreateBootstrapConfigMapIfNotExists(client, kubeconfig) if tc.expectErr && err == nil { t.Errorf("CreateBootstrapConfigMapIfNotExists(%s) wanted error, got nil", tc.name) } else if !tc.expectErr && err != nil {