Merge pull request #5433 from twz123/no-cache-nodeconfig

Don't cache the node config in the CfgVars struct

pkg/config/cfgvars.go

@@ -61,22 +61,7 @@ type CfgVars struct {
 	HelmRepositoryCache  string
 	HelmRepositoryConfig string
 
-	stdin      io.Reader
-	nodeConfig *v1beta1.ClusterConfig
-}
-
-func (c *CfgVars) DeepCopy() *CfgVars {
-	if c == nil {
-		return nil
-	}
-	// Make a copy of the original struct, this works because all the fields are
-	// primitive types
-	copy := *c
-
-	copy.nodeConfig = c.nodeConfig.DeepCopy()
-
-	// Return the copied struct
-	return &copy
+	stdin io.Reader
 }
 
 type CfgVarOption func(*CfgVars)
@@ -123,10 +108,6 @@ func WithCommand(cmd command) CfgVarOption {
 	}
 }
 
-func (c *CfgVars) SetNodeConfig(cfg *v1beta1.ClusterConfig) {
-	c.nodeConfig = cfg
-}
-
 func DefaultCfgVars() *CfgVars {
 	vars, _ := NewCfgVars(nil)
 	return vars
@@ -233,10 +214,6 @@ func (c *CfgVars) defaultStorageSpec() *v1beta1.StorageSpec {
 var defaultConfigPath = constant.K0sConfigPathDefault
 
 func (c *CfgVars) NodeConfig() (*v1beta1.ClusterConfig, error) {
-	if c.nodeConfig != nil {
-		return c.nodeConfig, nil
-	}
-
 	if c.StartupConfigPath == "" {
 		return nil, errors.New("config path is not set")
 	}
@@ -274,7 +251,5 @@ func (c *CfgVars) NodeConfig() (*v1beta1.ClusterConfig, error) {
 		nodeConfig.Spec.Storage.Kine = v1beta1.DefaultKineConfig(c.DataDir)
 	}
 
-	c.nodeConfig = nodeConfig
-
 	return nodeConfig, nil
 }

pkg/config/cfgvars_test.go

@@ -222,10 +222,10 @@ func TestNodeConfig_Stdin(t *testing.T) {
 	require.NoError(t, err)
 
 	nodeConfig, err := underTest.NodeConfig()
-	require.NoError(t, err)
+	assert.NoError(t, err)
 	assert.Equal(t, "calico", nodeConfig.Spec.Network.Provider)
 
-	nodeConfig2, err := underTest.NodeConfig()
-	require.NoError(t, err)
-	assert.Same(t, nodeConfig, nodeConfig2)
+	nodeConfig, err = underTest.NodeConfig()
+	assert.ErrorContains(t, err, "stdin already grabbed")
+	assert.Nil(t, nodeConfig)
 }

pkg/config/cli.go

@@ -302,7 +302,7 @@ func GetCmdOpts(cobraCmd command) (*CLIOptions, error) {
 	}
 
 	// if a runtime config can be loaded, use it to override the k0sVars
-	if rtc, err := LoadRuntimeConfig(k0sVars); err == nil {
+	if rtc, err := LoadRuntimeConfig(k0sVars.RuntimeConfigPath); err == nil {
 		k0sVars = rtc.K0sVars
 	}
 

pkg/config/runtime.go

@@ -59,8 +59,8 @@ type RuntimeConfigSpec struct {
 	Pid        int                    `json:"pid"`
 }
 
-func LoadRuntimeConfig(k0sVars *CfgVars) (*RuntimeConfigSpec, error) {
-	content, err := os.ReadFile(k0sVars.RuntimeConfigPath)
+func LoadRuntimeConfig(path string) (*RuntimeConfigSpec, error) {
+	content, err := os.ReadFile(path)
 	if err != nil {
 		return nil, err
 	}
@@ -98,7 +98,7 @@ func LoadRuntimeConfig(k0sVars *CfgVars) (*RuntimeConfigSpec, error) {
 }
 
 func NewRuntimeConfig(k0sVars *CfgVars) (*RuntimeConfigSpec, error) {
-	if _, err := LoadRuntimeConfig(k0sVars); err == nil {
+	if _, err := LoadRuntimeConfig(k0sVars.RuntimeConfigPath); err == nil {
 		return nil, ErrK0sAlreadyRunning
 	}
 
@@ -107,11 +107,6 @@ func NewRuntimeConfig(k0sVars *CfgVars) (*RuntimeConfigSpec, error) {
 		return nil, fmt.Errorf("load node config: %w", err)
 	}
 
-	vars := k0sVars.DeepCopy()
-
-	// don't persist the startup config path in the runtime config
-	vars.StartupConfigPath = ""
-
 	cfg := &RuntimeConfig{
 		ObjectMeta: metav1.ObjectMeta{
 			CreationTimestamp: metav1.Now(),

pkg/config/runtime_test.go

@@ -18,75 +18,68 @@ package config
 
 import (
 	"os"
+	"path/filepath"
 	"testing"
 
 	"github.com/k0sproject/k0s/pkg/apis/k0s/v1beta1"
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+	"sigs.k8s.io/yaml"
 )
 
 func TestLoadRuntimeConfig_K0sNotRunning(t *testing.T) {
-	// create a temporary file for runtime config
-	tmpfile, err := os.CreateTemp("", "runtime-config")
-	assert.NoError(t, err)
-	defer os.Remove(tmpfile.Name())
-
-	// prepare k0sVars
-	k0sVars := &CfgVars{
-		RuntimeConfigPath: tmpfile.Name(),
-	}
-
 	// write some content to the runtime config file
+	rtConfigPath := filepath.Join(t.TempDir(), "runtime-config")
 	content := []byte(`---
 apiVersion: k0s.k0sproject.io/v1beta1
 kind: RuntimeConfig
 spec:
   nodeConfig:
     metadata:
       name: k0s
-  pid: 9999999
+  pid: -1
 `)
-	err = os.WriteFile(k0sVars.RuntimeConfigPath, content, 0644)
-	assert.NoError(t, err)
+	require.NoError(t, os.WriteFile(rtConfigPath, content, 0644))
 
 	// try to load runtime config and check if it returns an error
-	spec, err := LoadRuntimeConfig(k0sVars)
+	spec, err := LoadRuntimeConfig(rtConfigPath)
 	assert.Nil(t, spec)
 	assert.ErrorIs(t, err, ErrK0sNotRunning)
 }
 
 func TestNewRuntimeConfig(t *testing.T) {
-	// create a temporary directory for k0s files
-	tempDir, err := os.MkdirTemp("", "k0s")
-	assert.NoError(t, err)
-	defer os.RemoveAll(tempDir)
+	// Create regular configuration file
+	tempDir := t.TempDir()
+	startupConfigPath := filepath.Join(tempDir, "startup-config")
+	startupConfig, err := yaml.Marshal(&v1beta1.ClusterConfig{
+		Spec: &v1beta1.ClusterSpec{
+			API: &v1beta1.APISpec{Address: "10.0.0.1"},
+		},
+	})
+	require.NoError(t, err)
+	require.NoError(t, os.WriteFile(startupConfigPath, startupConfig, 0644))
 
-	// create a temporary file for the runtime config
-	tmpfile, err := os.CreateTemp("", "runtime-config")
-	assert.NoError(t, err)
-	tmpfile.Close()
-	defer os.Remove(tmpfile.Name())
+	// Define runtime configuration file in a not yet existing directory
+	rtConfigPath := filepath.Join(tempDir, "runtime", "config")
 
 	// prepare k0sVars
 	k0sVars := &CfgVars{
-		RuntimeConfigPath: tmpfile.Name(),
+		StartupConfigPath: startupConfigPath,
+		RuntimeConfigPath: rtConfigPath,
 		DataDir:           tempDir,
-		nodeConfig: &v1beta1.ClusterConfig{
-			Spec: &v1beta1.ClusterSpec{
-				API: &v1beta1.APISpec{Address: "10.0.0.1"},
-			},
-		},
 	}
 
 	// create a new runtime config and check if it's valid
 	spec, err := NewRuntimeConfig(k0sVars)
 	assert.NoError(t, err)
 	assert.NotNil(t, spec)
-	assert.Equal(t, tempDir, spec.K0sVars.DataDir)
+	assert.Same(t, k0sVars, spec.K0sVars)
 	assert.Equal(t, os.Getpid(), spec.Pid)
 	assert.NotNil(t, spec.NodeConfig)
 	cfg, err := spec.K0sVars.NodeConfig()
 	assert.NoError(t, err)
 	assert.Equal(t, "10.0.0.1", cfg.Spec.API.Address)
+	assert.FileExists(t, rtConfigPath)
 
 	// try to create a new runtime config when one is already active and check if it returns an error
 	_, err = NewRuntimeConfig(k0sVars)