fix (crc/machine) : KubeContext left in invalid state after crc stop

At the moment, we are only cleaning up crc context from kubeconfig
during `crc delete`. This can be problematic if user tries to run any
cluster related command after running `crc stop` as kubeconfig still
points to CRC cluster that is not active.

I checked minikube's behavior and noticed it's cleaning up kube config
in case of both stop and delete commands. Make crc behavior consistent
with minikube and perform kubeconfig cleanup in both sub commands.

Signed-off-by: Rohan Kumar <[email protected]>

Author: rohanKanojia

pkg/crc/machine/kubeconfig.go

@@ -50,7 +50,7 @@ func updateClientCrtAndKeyToKubeconfig(clientKey, clientCrt []byte, srcKubeconfi
 }
 
 func writeKubeconfig(ip string, clusterConfig *types.ClusterConfig, ingressHTTPSPort uint) error {
-	kubeconfig, cfg, err := getGlobalKubeConfig()
+	kubeconfig, cfg, err := GetGlobalKubeConfig()
 	if err != nil {
 		return err
 	}
@@ -91,7 +91,7 @@ func writeKubeconfig(ip string, clusterConfig *types.ClusterConfig, ingressHTTPS
 	return clientcmd.WriteToFile(*cfg, kubeconfig)
 }
 
-func getGlobalKubeConfig() (string, *api.Config, error) {
+func GetGlobalKubeConfig() (string, *api.Config, error) {
 	kubeconfig := getGlobalKubeConfigPath()
 	return getKubeConfigFromFile(kubeconfig)
 }

pkg/crc/machine/kubeconfig_test.go

@@ -52,6 +52,32 @@ func TestCleanKubeconfig(t *testing.T) {
 	assert.YAMLEq(t, string(expected), string(actual))
 }
 
+func TestCleanKubeConfigIdempotency(t *testing.T) {
+	// Given
+	dir := t.TempDir()
+	// When
+	assert.NoError(t, cleanKubeconfig(filepath.Join("testdata", "kubeconfig.out"), filepath.Join(dir, "kubeconfig")))
+	actual, err := os.ReadFile(filepath.Join(dir, "kubeconfig"))
+	// Then
+	assert.NoError(t, err)
+	expected, err := os.ReadFile(filepath.Join("testdata", "kubeconfig.out"))
+	assert.NoError(t, err)
+	assert.YAMLEq(t, string(expected), string(actual))
+}
+
+func TestCleanKubeConfigShouldDoNothingWhenClusterDomainIsNotEqualToCrcTesting(t *testing.T) {
+	// Given
+	dir := t.TempDir()
+	// When
+	assert.NoError(t, cleanKubeconfig(filepath.Join("testdata", "kubeconfig-without-api-crc-testing-cluster-domain"), filepath.Join(dir, "kubeconfig")))
+	actual, err := os.ReadFile(filepath.Join(dir, "kubeconfig"))
+	// Then
+	assert.NoError(t, err)
+	expected, err := os.ReadFile(filepath.Join("testdata", "kubeconfig-without-api-crc-testing-cluster-domain"))
+	assert.NoError(t, err)
+	assert.YAMLEq(t, string(expected), string(actual))
+}
+
 func TestUpdateUserCaAndKeyToKubeconfig(t *testing.T) {
 	f, err := os.CreateTemp("", "kubeconfig")
 	assert.NoError(t, err, "")

pkg/crc/machine/stop.go

@@ -1,6 +1,8 @@
 package machine
 
 import (
+	"os"
+
 	"github.com/crc-org/crc/v2/pkg/crc/logging"
 	"github.com/crc-org/crc/v2/pkg/crc/machine/state"
 	crcPreset "github.com/crc-org/crc/v2/pkg/crc/preset"
@@ -9,6 +11,12 @@ import (
 )
 
 func (client *client) Stop() (state.State, error) {
+	defer func(input, output string) {
+		err := cleanKubeconfig(input, output)
+		if !errors.Is(err, os.ErrNotExist) {
+			logging.Warnf("Failed to remove crc contexts from kubeconfig: %v", err)
+		}
+	}(getGlobalKubeConfigPath(), getGlobalKubeConfigPath())
 	if running, _ := client.IsRunning(); !running {
 		return state.Error, errors.New("Instance is already stopped")
 	}

pkg/crc/machine/stop_test.go

@@ -0,0 +1,76 @@
+package machine
+
+import (
+	"os"
+	"path/filepath"
+	"testing"
+
+	crcConfig "github.com/crc-org/crc/v2/pkg/crc/config"
+	"github.com/crc-org/crc/v2/pkg/crc/machine/state"
+	crcOs "github.com/crc-org/crc/v2/pkg/os"
+	"github.com/stretchr/testify/assert"
+)
+
+func TestClient_WhenStopInvokedWithNonExistentVM_ThenThrowError(t *testing.T) {
+	// Given
+	dir := t.TempDir()
+	oldKubeConfigEnvVarValue := os.Getenv("KUBECONFIG")
+	kubeConfigPath := filepath.Join(dir, "kubeconfig")
+	err := crcOs.CopyFile(filepath.Join("testdata", "kubeconfig.in"), kubeConfigPath)
+	assert.NoError(t, err)
+	err = os.Setenv("KUBECONFIG", kubeConfigPath)
+	assert.NoError(t, err)
+	crcConfigStorage := crcConfig.New(crcConfig.NewEmptyInMemoryStorage(), crcConfig.NewEmptyInMemorySecretStorage())
+	client := NewClient("i-dont-exist", false, crcConfigStorage)
+
+	// When
+	clusterState, stopErr := client.Stop()
+
+	// Then
+	assert.EqualError(t, stopErr, "Instance is already stopped")
+	assert.Equal(t, clusterState, state.Error)
+	err = os.Setenv("KUBECONFIG", oldKubeConfigEnvVarValue)
+	assert.NoError(t, err)
+}
+
+var testArguments = map[string]struct {
+	inputKubeConfigPath    string
+	expectedKubeConfigPath string
+}{
+	"When KubeConfig contains crc context, then cleanup KubeConfig": {
+		"kubeconfig.in", "kubeconfig.out",
+	},
+	"When KubeConfig does not contain crc context, then KubeConfig remains unchanged": {
+		"kubeconfig.out", "kubeconfig.out",
+	},
+}
+
+func TestClient_WhenStopInvoked_ThenKubeConfigUpdatedIfRequired(t *testing.T) {
+	for name, test := range testArguments {
+		t.Run(name, func(t *testing.T) {
+			// Given
+			dir := t.TempDir()
+			oldKubeConfigEnvVarValue := os.Getenv("KUBECONFIG")
+			kubeConfigPath := filepath.Join(dir, "kubeconfig")
+			err := crcOs.CopyFile(filepath.Join("testdata", test.inputKubeConfigPath), kubeConfigPath)
+			assert.NoError(t, err)
+			err = os.Setenv("KUBECONFIG", kubeConfigPath)
+			assert.NoError(t, err)
+			crcConfigStorage := crcConfig.New(crcConfig.NewEmptyInMemoryStorage(), crcConfig.NewEmptyInMemorySecretStorage())
+			client := NewClient("test-client", false, crcConfigStorage)
+
+			// When
+			clusterState, _ := client.Stop()
+
+			// Then
+			actualKubeConfigFile, err := os.ReadFile(kubeConfigPath)
+			assert.NoError(t, err)
+			expectedKubeConfigPath, err := os.ReadFile(filepath.Join("testdata", test.expectedKubeConfigPath))
+			assert.NoError(t, err)
+			assert.YAMLEq(t, string(expectedKubeConfigPath), string(actualKubeConfigFile))
+			assert.Equal(t, clusterState, state.Error)
+			err = os.Setenv("KUBECONFIG", oldKubeConfigEnvVarValue)
+			assert.NoError(t, err)
+		})
+	}
+}

pkg/crc/machine/testdata/kubeconfig-without-api-crc-testing-cluster-domain

@@ -0,0 +1,18 @@
+apiVersion: v1
+clusters:
+- cluster:
+    server: https://api.unknown.testing:6443
+  name: api-crc-testing:6443
+contexts:
+- context:
+    cluster: api-crc-testing:6443
+    namespace: default
+    user: kubeadmin/api-crc-testing:6443
+  name: default/api-crc-testing:6443/kubeadmin
+current-context: default/api-crc-testing:6443/kubeadmin
+kind: Config
+preferences: {}
+users:
+- name: kubeadmin/api-crc-testing:6443
+  user:
+    token: sha256~secret

test/e2e/features/basic.feature

@@ -60,6 +60,7 @@ Feature: Basic test
         When executing "crc stop"
         Then stdout should match "(.*)[Ss]topped the instance"
         And executing "oc whoami" fails
+        And kubeconfig is cleaned up
         # status check
         When checking that CRC is stopped
         And stdout should not contain "Running"
@@ -69,5 +70,6 @@ Feature: Basic test
         # delete
         When executing "crc delete -f" succeeds
         Then stdout should contain "Deleted the instance"
+        And kubeconfig is cleaned up
         # cleanup
         When executing crc cleanup command succeeds

test/e2e/testsuite/testsuite.go

@@ -13,6 +13,7 @@ import (
 	"time"
 
 	"github.com/crc-org/crc/v2/pkg/crc/constants"
+	"github.com/crc-org/crc/v2/pkg/crc/machine"
 	"github.com/crc-org/crc/v2/pkg/crc/preset"
 	"github.com/crc-org/crc/v2/pkg/crc/version"
 	"github.com/crc-org/crc/v2/test/extended/crc/cmd"
@@ -522,6 +523,8 @@ func InitializeScenario(s *godog.ScenarioContext) {
 		EnsureUserNetworkmode)
 	s.Step(`^ensuring microshift cluster is fully operational$`,
 		EnsureMicroshiftClusterIsOperational)
+	s.Step(`^kubeconfig is cleaned up$`,
+		EnsureKubeConfigIsCleanedUp)
 
 	s.After(func(ctx context.Context, _ *godog.Scenario, err error) (context.Context, error) {
 
@@ -1025,6 +1028,26 @@ func EnsureUserNetworkmode() error {
 	return nil
 }
 
+func EnsureKubeConfigIsCleanedUp() error {
+	kubeConfig, cfg, err := machine.GetGlobalKubeConfig()
+	if err != nil {
+		return err
+	}
+	if len(kubeConfig) == 0 {
+		return fmt.Errorf("unable to load kube config file while verifying kube config cleanup")
+	}
+	if cfg.CurrentContext != "" {
+		return fmt.Errorf("kube config's current context not cleaned up. [expected : \"\", actual : %s]", cfg.CurrentContext)
+	}
+	crcClusterDomain := fmt.Sprintf("https://api%s:6443", constants.ClusterDomain)
+	for name, cluster := range cfg.Clusters {
+		if cluster.Server == crcClusterDomain {
+			return fmt.Errorf("kube config's cluster %s is not cleaned up, it still contains a cluster with %s domain [expected : \"\", actual : %s]", name, crcClusterDomain, crcClusterDomain)
+		}
+	}
+	return nil
+}
+
 // This function will wait until the microshift cluster got operational
 func EnsureMicroshiftClusterIsOperational() error {
 	// First wait until crc report the cluster as running