Skip to content

Commit

Permalink
fix (crc/machine) : KubeContext left in invalid state after crc stop
Browse files Browse the repository at this point in the history
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]>
  • Loading branch information
rohanKanojia committed Oct 21, 2024
1 parent f4ccc58 commit 2f2def2
Show file tree
Hide file tree
Showing 7 changed files with 158 additions and 2 deletions.
4 changes: 2 additions & 2 deletions pkg/crc/machine/kubeconfig.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down Expand Up @@ -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)
}
Expand Down
26 changes: 26 additions & 0 deletions pkg/crc/machine/kubeconfig_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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, "")
Expand Down
8 changes: 8 additions & 0 deletions pkg/crc/machine/stop.go
Original file line number Diff line number Diff line change
@@ -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"
Expand All @@ -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")
}
Expand Down
76 changes: 76 additions & 0 deletions pkg/crc/machine/stop_test.go
Original file line number Diff line number Diff line change
@@ -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)
})
}
}
Original file line number Diff line number Diff line change
@@ -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
2 changes: 2 additions & 0 deletions test/e2e/features/basic.feature
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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
26 changes: 26 additions & 0 deletions test/e2e/testsuite/testsuite.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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) {

Expand Down Expand Up @@ -1025,6 +1028,29 @@ func EnsureUserNetworkmode() error {
return nil
}

func EnsureKubeConfigIsCleanedUp() error {
kubeConfig, cfg, err := machine.GetGlobalKubeConfig()
if err != nil {
return err
}
if len(kubeConfig) == 0 {
fmt.Println("Unable to load kubeconfig file")
os.Exit(1)
}
if cfg.CurrentContext != "" {
fmt.Printf("KubeConfig's Current Context not Cleaned up. [Expected : \"\", Actual : %s]\n", cfg.CurrentContext)
os.Exit(1)
}
crcClusterDomain := fmt.Sprintf("https://api%s:6443", constants.ClusterDomain)
for name, cluster := range cfg.Clusters {
if cluster.Server == crcClusterDomain {
fmt.Printf("KubeConfig's Cluster %s is not Cleaned up, it still contains a cluster with %s domain [Expected : \"\", Actual : %s]\n", name, crcClusterDomain, crcClusterDomain)
os.Exit(1)
}
}
return nil
}

// This function will wait until the microshift cluster got operational
func EnsureMicroshiftClusterIsOperational() error {
// First wait until crc report the cluster as running
Expand Down

0 comments on commit 2f2def2

Please sign in to comment.