Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor (crc/machine) : Provide a dummy implementation for virtualMachine object for writing unit tests (#4407) #4423

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 17 additions & 8 deletions pkg/crc/machine/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,21 +30,30 @@ type Client interface {
}

type client struct {
name string
debug bool
config crcConfig.Storage
name string
debug bool
config crcConfig.Storage
virtualMachine VirtualMachine

diskDetails *memoize.Memoizer
ramDetails *memoize.Memoizer
}

func NewClient(name string, debug bool, config crcConfig.Storage) Client {
return newClientWithVirtualMachine(name, debug, config, nil)
}

// newClientWithVirtualMachine creates a Client instance with an overridden VirtualMachine implementation.
cfergeau marked this conversation as resolved.
Show resolved Hide resolved
// It would not create a new VirtualMachine object. This method is primarily created for usage in tests so
// that we can pass a fake VirtualMachine implementation.
func newClientWithVirtualMachine(name string, debug bool, config crcConfig.Storage, vm VirtualMachine) Client {
return &client{
name: name,
debug: debug,
config: config,
diskDetails: memoize.NewMemoizer(time.Minute, 5*time.Minute),
ramDetails: memoize.NewMemoizer(30*time.Second, 2*time.Minute),
name: name,
debug: debug,
config: config,
diskDetails: memoize.NewMemoizer(time.Minute, 5*time.Minute),
ramDetails: memoize.NewMemoizer(30*time.Second, 2*time.Minute),
virtualMachine: vm,
cfergeau marked this conversation as resolved.
Show resolved Hide resolved
}
}

Expand Down
2 changes: 1 addition & 1 deletion pkg/crc/machine/console.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ func (client *client) GetConsoleURL() (*types.ConsoleResult, error) {
return nil, errors.Wrap(err, "Error getting the state for virtual machine")
}

clusterConfig, err := getClusterConfig(vm.bundle)
clusterConfig, err := getClusterConfig(vm.Bundle())
if err != nil {
return nil, errors.Wrap(err, "Error loading cluster configuration")
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/crc/machine/delete.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ func (client *client) Delete() error {

// In case usermode networking make sure all the port bind on host should be released
if client.useVSock() {
if err := unexposePorts(); err != nil {
if err := vm.UnExposePorts(); err != nil {
return err
}
}
Expand Down
126 changes: 126 additions & 0 deletions pkg/crc/machine/fakemachine/virtualmachine.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,126 @@
package fakemachine

import (
"errors"

"github.com/crc-org/crc/v2/pkg/crc/machine/bundle"
"github.com/crc-org/crc/v2/pkg/crc/machine/state"
crcPreset "github.com/crc-org/crc/v2/pkg/crc/preset"
"github.com/crc-org/crc/v2/pkg/crc/ssh"
"github.com/crc-org/crc/v2/pkg/libmachine"
libmachinehost "github.com/crc-org/crc/v2/pkg/libmachine/host"
"github.com/crc-org/machine/libmachine/drivers"
)

type FakeSSHClient struct {
LastExecutedCommand string
IsSSHClientClosed bool
}

func (client *FakeSSHClient) Run(command string) ([]byte, []byte, error) {
client.LastExecutedCommand = command
return []byte("test"), []byte("test"), nil
}

func (client *FakeSSHClient) Close() {
client.IsSSHClientClosed = true
}

func NewFakeSSHClient() *FakeSSHClient {
return &FakeSSHClient{
LastExecutedCommand: "",
IsSSHClientClosed: false,
}
}

type FakeVirtualMachine struct {
IsClosed bool
IsRemoved bool
IsStopped bool
FailingStop bool
FailingState bool
FakeSSHClient *FakeSSHClient
PortsExposed bool
}

func (vm *FakeVirtualMachine) Close() error {
vm.IsClosed = true
return nil
}

func (vm *FakeVirtualMachine) Remove() error {
vm.IsRemoved = true
return nil
}

func (vm *FakeVirtualMachine) Stop() error {
if vm.FailingStop {
return errors.New("stopping failed")
}
vm.IsStopped = true
return nil
}

func (vm *FakeVirtualMachine) State() (state.State, error) {
if vm.FailingState {
return state.Error, errors.New("unable to get virtual machine state")
}
if vm.IsClosed || vm.IsStopped {
return state.Stopped, nil
}
return state.Running, nil
}

func (vm *FakeVirtualMachine) IP() (string, error) {
panic("not implemented")
}

func (vm *FakeVirtualMachine) SSHPort() int {
panic("not implemented")
}

func (vm *FakeVirtualMachine) SSHRunner() (*ssh.Runner, error) {
runner, err := ssh.CreateRunnerWithClient(vm.FakeSSHClient)
return runner, err
}

func (vm *FakeVirtualMachine) Bundle() *bundle.CrcBundleInfo {
panic("not implemented")
}

func (vm *FakeVirtualMachine) Driver() drivers.Driver {
panic("not implemented")
}

func (vm *FakeVirtualMachine) API() libmachine.API {
panic("not implemented")
}

func (vm *FakeVirtualMachine) Host() *libmachinehost.Host {
panic("not implemented")
}

func (vm *FakeVirtualMachine) Kill() error {
panic("not implemented")
}

func (vm *FakeVirtualMachine) ExposePorts(_ crcPreset.Preset, _, _ uint) error {
vm.PortsExposed = true
return nil
}

func (vm *FakeVirtualMachine) UnExposePorts() error {
vm.PortsExposed = false
return nil
}

func NewFakeVirtualMachine(failingStop bool, failingState bool) *FakeVirtualMachine {
return &FakeVirtualMachine{
FakeSSHClient: NewFakeSSHClient(),
IsStopped: false,
IsRemoved: false,
IsClosed: false,
FailingStop: failingStop,
FailingState: failingState,
}
}
4 changes: 2 additions & 2 deletions pkg/crc/machine/generate_bundle.go
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ func loadVM(client *client) (*bundle.CrcBundleInfo, *crcssh.Runner, error) {
}
defer vm.Close()

currentState, err := vm.Driver.GetState()
currentState, err := vm.Driver().GetState()
if err != nil {
return nil, nil, errors.Wrap(err, "Cannot get machine state")
}
Expand All @@ -124,5 +124,5 @@ func loadVM(client *client) (*bundle.CrcBundleInfo, *crcssh.Runner, error) {
return nil, nil, errors.Wrap(err, "Error creating the ssh client")
}

return vm.bundle, sshRunner, nil
return vm.Bundle(), sshRunner, nil
}
2 changes: 1 addition & 1 deletion pkg/crc/machine/ip.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,6 @@ func (client *client) ConnectionDetails() (*types.ConnectionDetails, error) {
IP: ip,
SSHPort: vm.SSHPort(),
SSHUsername: constants.DefaultSSHUser,
SSHKeys: []string{constants.GetPrivateKeyPath(), constants.GetECDSAPrivateKeyPath(), vm.bundle.GetSSHKeyPath()},
SSHKeys: []string{constants.GetPrivateKeyPath(), constants.GetECDSAPrivateKeyPath(), vm.Bundle().GetSSHKeyPath()},
}, nil
}
50 changes: 25 additions & 25 deletions pkg/crc/machine/start.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,48 +60,48 @@ func getCrcBundleInfo(preset crcPreset.Preset, bundleName, bundlePath string, en
return bundle.Use(bundleName)
}

func (client *client) updateVMConfig(startConfig types.StartConfig, vm *virtualMachine) error {
func (client *client) updateVMConfig(startConfig types.StartConfig, vm VirtualMachine) error {
/* Memory */
logging.Debugf("Updating CRC VM configuration")
if err := setMemory(vm.Host, startConfig.Memory); err != nil {
if err := setMemory(vm.Host(), startConfig.Memory); err != nil {
logging.Debugf("Failed to update CRC VM configuration: %v", err)
if err == drivers.ErrNotImplemented {
logging.Warn("Memory configuration change has been ignored as the machine driver does not support it")
} else {
return err
}
}
if err := setVcpus(vm.Host, startConfig.CPUs); err != nil {
if err := setVcpus(vm.Host(), startConfig.CPUs); err != nil {
logging.Debugf("Failed to update CRC VM configuration: %v", err)
if err == drivers.ErrNotImplemented {
logging.Warn("CPU configuration change has been ignored as the machine driver does not support it")
} else {
return err
}
}
if err := vm.api.Save(vm.Host); err != nil {
if err := vm.API().Save(vm.Host()); err != nil {
return err
}

/* Disk size */
if startConfig.DiskSize != constants.DefaultDiskSize {
if err := setDiskSize(vm.Host, startConfig.DiskSize); err != nil {
if err := setDiskSize(vm.Host(), startConfig.DiskSize); err != nil {
logging.Debugf("Failed to update CRC disk configuration: %v", err)
if err == drivers.ErrNotImplemented {
logging.Warn("Disk size configuration change has been ignored as the machine driver does not support it")
} else {
return err
}
}
if err := vm.api.Save(vm.Host); err != nil {
if err := vm.API().Save(vm.Host()); err != nil {
return err
}
}

// we want to set the shared dir password on-the-fly to be used
// we do not want this value to be persisted to disk
if startConfig.SharedDirPassword != "" {
if err := setSharedDirPassword(vm.Host, startConfig.SharedDirPassword); err != nil {
if err := setSharedDirPassword(vm.Host(), startConfig.SharedDirPassword); err != nil {
return fmt.Errorf("Failed to set shared dir password: %w", err)
}
}
Expand Down Expand Up @@ -206,9 +206,9 @@ func growLVForMicroshift(sshRunner *crcssh.Runner, lvFullName string, rootPart s
return nil
}

func configureSharedDirs(vm *virtualMachine, sshRunner *crcssh.Runner) error {
func configureSharedDirs(vm VirtualMachine, sshRunner *crcssh.Runner) error {
logging.Debugf("Configuring shared directories")
sharedDirs, err := vm.Driver.GetSharedDirs()
sharedDirs, err := vm.Driver().GetSharedDirs()
if err != nil {
// the libvirt machine driver uses net/rpc, which wraps errors
// in rpc.ServerError, but without using golang 1.13 error
Expand Down Expand Up @@ -334,7 +334,7 @@ func (client *client) Start(ctx context.Context, startConfig types.StartConfig)
}
defer vm.Close()

currentBundleName := vm.bundle.GetBundleName()
currentBundleName := vm.Bundle().GetBundleName()
if currentBundleName != bundleName {
logging.Debugf("Bundle '%s' was requested, but the existing VM is using '%s'",
bundleName, currentBundleName)
Expand All @@ -347,8 +347,8 @@ func (client *client) Start(ctx context.Context, startConfig types.StartConfig)
return nil, errors.Wrap(err, "Error getting the machine state")
}
if vmState == state.Running {
logging.Infof("A CRC VM for %s %s is already running", startConfig.Preset.ForDisplay(), vm.bundle.GetVersion())
clusterConfig, err := getClusterConfig(vm.bundle)
logging.Infof("A CRC VM for %s %s is already running", startConfig.Preset.ForDisplay(), vm.Bundle().GetVersion())
clusterConfig, err := getClusterConfig(vm.Bundle())
if err != nil {
return nil, errors.Wrap(err, "Cannot create cluster configuration")
}
Expand All @@ -365,10 +365,10 @@ func (client *client) Start(ctx context.Context, startConfig types.StartConfig)
return nil, err
}

logging.Infof("Starting CRC VM for %s %s...", startConfig.Preset, vm.bundle.GetVersion())
logging.Infof("Starting CRC VM for %s %s...", startConfig.Preset, vm.Bundle().GetVersion())

if client.useVSock() {
if err := exposePorts(startConfig.Preset, startConfig.IngressHTTPPort, startConfig.IngressHTTPSPort); err != nil {
if err := vm.ExposePorts(startConfig.Preset, startConfig.IngressHTTPPort, startConfig.IngressHTTPSPort); err != nil {
return nil, err
}
}
Expand Down Expand Up @@ -457,7 +457,7 @@ func (client *client) Start(ctx context.Context, startConfig types.StartConfig)
return nil, errors.Wrap(err, "Failed to change permissions to root podman socket")
}

proxyConfig, err := getProxyConfig(vm.bundle)
proxyConfig, err := getProxyConfig(vm.Bundle())
if err != nil {
return nil, errors.Wrap(err, "Error getting proxy configuration")
}
Expand All @@ -472,7 +472,7 @@ func (client *client) Start(ctx context.Context, startConfig types.StartConfig)
SSHRunner: sshRunner,
IP: instanceIP,
// TODO: should be more finegrained
BundleMetadata: *vm.bundle,
BundleMetadata: *vm.Bundle(),
NetworkMode: client.networkMode(),
}

Expand Down Expand Up @@ -502,7 +502,7 @@ func (client *client) Start(ctx context.Context, startConfig types.StartConfig)
logging.Warn(fmt.Sprintf("Failed to query DNS from host: %v", err))
}

if vm.bundle.IsMicroshift() {
if vm.Bundle().IsMicroshift() {
// **************************
// END OF MICROSHIFT START CODE
// **************************
Expand Down Expand Up @@ -547,7 +547,7 @@ func (client *client) Start(ctx context.Context, startConfig types.StartConfig)
ocConfig := oc.UseOCWithSSH(sshRunner)

if err := cluster.ApproveCSRAndWaitForCertsRenewal(ctx, sshRunner, ocConfig, certsExpired[cluster.KubeletClientCert], certsExpired[cluster.KubeletServerCert], certsExpired[cluster.AggregatorClientCert]); err != nil {
logBundleDate(vm.bundle)
logBundleDate(vm.Bundle())
return nil, errors.Wrap(err, "Failed to renew TLS certificates: please check if a newer CRC release is available")
}

Expand Down Expand Up @@ -596,7 +596,7 @@ func (client *client) Start(ctx context.Context, startConfig types.StartConfig)
}
}

if err := updateKubeconfig(ctx, ocConfig, sshRunner, vm.bundle.GetKubeConfigPath()); err != nil {
if err := updateKubeconfig(ctx, ocConfig, sshRunner, vm.Bundle().GetKubeConfigPath()); err != nil {
return nil, errors.Wrap(err, "Failed to update kubeconfig file")
}

Expand All @@ -607,7 +607,7 @@ func (client *client) Start(ctx context.Context, startConfig types.StartConfig)

waitForProxyPropagation(ctx, ocConfig, proxyConfig)

clusterConfig, err := getClusterConfig(vm.bundle)
clusterConfig, err := getClusterConfig(vm.Bundle())
if err != nil {
return nil, errors.Wrap(err, "Cannot get cluster configuration")
}
Expand All @@ -625,7 +625,7 @@ func (client *client) Start(ctx context.Context, startConfig types.StartConfig)
}

func (client *client) IsRunning() (bool, error) {
vm, err := loadVirtualMachine(client.name, client.useVSock())
vm, err := loadVirtualMachineLazily(client.virtualMachine, client.name, client.useVSock())
if err != nil {
return false, errors.Wrap(err, "Cannot load machine")
}
Expand Down Expand Up @@ -694,17 +694,17 @@ func createHost(machineConfig config.MachineConfig, preset crcPreset.Preset) err
return nil
}

func startHost(ctx context.Context, vm *virtualMachine) error {
if err := vm.Driver.Start(); err != nil {
func startHost(ctx context.Context, vm VirtualMachine) error {
if err := vm.Driver().Start(); err != nil {
return fmt.Errorf("Error in driver during machine start: %s", err)
}

if err := vm.api.Save(vm.Host); err != nil {
if err := vm.API().Save(vm.Host()); err != nil {
return fmt.Errorf("Error saving virtual machine to store after attempting creation: %s", err)
}

logging.Debug("Waiting for machine to be running, this may take a few minutes...")
if err := crcerrors.Retry(ctx, 3*time.Minute, host.MachineInState(vm.Driver, libmachinestate.Running), 3*time.Second); err != nil {
if err := crcerrors.Retry(ctx, 3*time.Minute, host.MachineInState(vm.Driver(), libmachinestate.Running), 3*time.Second); err != nil {
return fmt.Errorf("Error waiting for machine to be running: %s", err)
}

Expand Down
Loading
Loading