diff --git a/cmd/config.go b/cmd/config.go index 08dffe5..4097792 100644 --- a/cmd/config.go +++ b/cmd/config.go @@ -14,6 +14,7 @@ import ( log "github.com/sirupsen/logrus" "github.com/spf13/viper" + "github.com/LINBIT/virter/internal/virter" "github.com/LINBIT/virter/pkg/sshkeys" ) @@ -209,3 +210,12 @@ func initSSHFromConfig() { log.Fatal(err) } } + +func getReadyConfig() virter.VmReadyConfig { + // The config keys are kept for compatibility. + // Note that viper.RegisterAlias sounds like it could be used, but I couldn't make it work. + return virter.VmReadyConfig{ + Retries: viper.GetInt("time.ssh_ping_count"), + CheckTimeout: viper.GetDuration("time.ssh_ping_period"), + } +} diff --git a/cmd/image_build.go b/cmd/image_build.go index fd8a0b5..5f11940 100644 --- a/cmd/image_build.go +++ b/cmd/image_build.go @@ -184,11 +184,6 @@ func imageBuildCommand() *cobra.Command { Mounts: mounts, } - sshPingConfig := virter.SSHPingConfig{ - SSHPingCount: viper.GetInt("time.ssh_ping_count"), - SSHPingPeriod: viper.GetDuration("time.ssh_ping_period"), - } - containerName := "virter-build-" + newImageName if provisionConfig.NeedsContainers() { @@ -210,7 +205,7 @@ func imageBuildCommand() *cobra.Command { p = mpb.NewWithContext(ctx, DefaultContainerOpt()) - err = v.ImageBuild(ctx, tools, vmConfig, sshPingConfig, buildConfig, virter.WithProgress(DefaultProgressFormat(p))) + err = v.ImageBuild(ctx, tools, vmConfig, getReadyConfig(), buildConfig, virter.WithProgress(DefaultProgressFormat(p))) if err != nil { log.Fatalf("Failed to build image: %v", err) } diff --git a/cmd/vm_run.go b/cmd/vm_run.go index 608726f..0ebab1f 100644 --- a/cmd/vm_run.go +++ b/cmd/vm_run.go @@ -226,12 +226,7 @@ func vmRunCommand() *cobra.Command { } if waitSSH { - sshPingConfig := virter.SSHPingConfig{ - SSHPingCount: viper.GetInt("time.ssh_ping_count"), - SSHPingPeriod: viper.GetDuration("time.ssh_ping_period"), - } - - err = v.PingSSH(ctx, SSHClientBuilder{}, thisVMName, sshPingConfig) + err = v.WaitVmReady(ctx, SSHClientBuilder{}, thisVMName, getReadyConfig()) if err != nil { return fmt.Errorf("Failed to connect to VM %d over SSH: %w", id, err) } diff --git a/internal/virter/image.go b/internal/virter/image.go index 63e39b2..28a7fff 100644 --- a/internal/virter/image.go +++ b/internal/virter/image.go @@ -537,11 +537,11 @@ type ImageBuildConfig struct { ResetMachineID bool } -func (v *Virter) imageBuildProvisionCommit(ctx context.Context, tools ImageBuildTools, vmConfig VMConfig, pingConfig SSHPingConfig, buildConfig ImageBuildConfig, opts ...LayerOperationOption) error { +func (v *Virter) imageBuildProvisionCommit(ctx context.Context, tools ImageBuildTools, vmConfig VMConfig, readyConfig VmReadyConfig, buildConfig ImageBuildConfig, opts ...LayerOperationOption) error { vmNames := []string{vmConfig.Name} var err error - err = v.PingSSH(ctx, tools.ShellClientBuilder, vmConfig.Name, pingConfig) + err = v.WaitVmReady(ctx, tools.ShellClientBuilder, vmConfig.Name, readyConfig) if err != nil { return err } @@ -588,7 +588,7 @@ func (v *Virter) imageBuildProvisionCommit(ctx context.Context, tools ImageBuild } // ImageBuild builds an image by running a VM and provisioning it. -func (v *Virter) ImageBuild(ctx context.Context, tools ImageBuildTools, vmConfig VMConfig, pingConfig SSHPingConfig, buildConfig ImageBuildConfig, opts ...LayerOperationOption) error { +func (v *Virter) ImageBuild(ctx context.Context, tools ImageBuildTools, vmConfig VMConfig, readyConfig VmReadyConfig, buildConfig ImageBuildConfig, opts ...LayerOperationOption) error { // VMRun is responsible to call CheckVMConfig here! // TODO(): currently we can not know why VM run failed, so we don't clean up in this stage, // it could have been an existing VM, we don't want to delete it. @@ -598,7 +598,7 @@ func (v *Virter) ImageBuild(ctx context.Context, tools ImageBuildTools, vmConfig } // from here on it is safe to rm the VM if something fails - err = v.imageBuildProvisionCommit(ctx, tools, vmConfig, pingConfig, buildConfig, opts...) + err = v.imageBuildProvisionCommit(ctx, tools, vmConfig, readyConfig, buildConfig, opts...) if err != nil { log.Warn("could not build image, deleting VM") if rmErr := v.VMRm(vmConfig.Name, !vmConfig.StaticDHCP, true); rmErr != nil { diff --git a/internal/virter/virter.go b/internal/virter/virter.go index 9799b9f..d7a0fbc 100644 --- a/internal/virter/virter.go +++ b/internal/virter/virter.go @@ -152,10 +152,10 @@ type VMMeta struct { HostKey string `xml:"hostkey"` } -// SSHPingConfig contains the configuration for pinging a VM via SSH -type SSHPingConfig struct { - SSHPingCount int - SSHPingPeriod time.Duration +// VmReadyConfig contains the configuration for waiting for a VM to be ready. +type VmReadyConfig struct { + Retries int + CheckTimeout time.Duration } func checkDisks(vmConfig VMConfig) error { diff --git a/internal/virter/vm.go b/internal/virter/vm.go index c84b8b5..6131455 100644 --- a/internal/virter/vm.go +++ b/internal/virter/vm.go @@ -169,8 +169,8 @@ func diskVolumeName(vmName, diskName string) string { return vmName + "-" + diskName } -// PingSSH repeatedly tries to connect to a VM via SSH until it succeeds -func (v *Virter) PingSSH(ctx context.Context, shellClientBuilder ShellClientBuilder, vmName string, pingConfig SSHPingConfig) error { +// WaitVmReady repeatedly tries to connect to a VM and checks if it's ready to be used. +func (v *Virter) WaitVmReady(ctx context.Context, shellClientBuilder ShellClientBuilder, vmName string, readyConfig VmReadyConfig) error { ips, err := v.getIPs([]string{vmName}) if err != nil { return err @@ -190,41 +190,37 @@ func (v *Virter) PingSSH(ctx context.Context, shellClientBuilder ShellClientBuil sshConfig := ssh.ClientConfig{ Auth: v.sshkeys.Auth(), - Timeout: pingConfig.SSHPingPeriod, + Timeout: readyConfig.CheckTimeout, User: "root", HostKeyCallback: hostkeyCheck, HostKeyAlgorithms: supportedAlgos, } - sshTry := func() error { - return tryDialSSH(ctx, shellClientBuilder, hostPort, sshConfig) + readyFunc := func() error { + sshClient := shellClientBuilder.NewShellClient(hostPort, sshConfig) + if err := sshClient.DialContext(ctx); err != nil { + log.Debugf("SSH dial attempt failed: %v", err) + return err + } + defer sshClient.Close() + if err := sshClient.ExecScript("test -f /run/cloud-init/result.json"); err != nil { + log.Debugf("cloud-init not done: %v", err) + return err + } + + return nil } - log.Print("Wait for SSH port to open") + log.Print("Wait for VM to get ready") // Using ActualTime breaks the expectation of the unit tests // that this code does not sleep, but we work around that by // always making the first ping successful in tests - if err := (actualtime.ActualTime{}.Ping(ctx, pingConfig.SSHPingCount, pingConfig.SSHPingPeriod, sshTry)); err != nil { - return fmt.Errorf("unable to connect to SSH port: %w", err) - } - - log.Print("Successfully connected to SSH port") - return nil -} - -func tryDialSSH(ctx context.Context, shellClientBuilder ShellClientBuilder, hostPort string, sshConfig ssh.ClientConfig) error { - sshClient := shellClientBuilder.NewShellClient(hostPort, sshConfig) - if err := sshClient.DialContext(ctx); err != nil { - log.Debugf("SSH dial attempt failed: %v", err) - return err - } - defer sshClient.Close() - if err := sshClient.ExecScript("test -f /run/cloud-init/result.json"); err != nil { - log.Debugf("cloud-init not done: %v", err) - return err + if err := (actualtime.ActualTime{}.Ping(ctx, readyConfig.Retries, readyConfig.CheckTimeout, readyFunc)); err != nil { + return fmt.Errorf("VM not ready: %w", err) } + log.Print("Successfully connected to ready VM") return nil } diff --git a/internal/virter/vm_test.go b/internal/virter/vm_test.go index e55e7e8..58c0698 100644 --- a/internal/virter/vm_test.go +++ b/internal/virter/vm_test.go @@ -76,15 +76,15 @@ func TestVMRun(t *testing.T) { assert.True(t, domain.active) } -func TestSSHPing(t *testing.T) { +func TestWaitVmReady(t *testing.T) { shell := new(mocks.ShellClient) shell.On("DialContext", mock.Anything).Return(nil) shell.On("Close").Return(nil) shell.On("ExecScript", mock.Anything).Return(nil) - sshPingConfig := virter.SSHPingConfig{ - SSHPingCount: 1, - SSHPingPeriod: time.Second, // ignored + readyConfig := virter.VmReadyConfig{ + Retries: 1, + CheckTimeout: time.Second, // ignored } l := newFakeLibvirtConnection() @@ -96,7 +96,7 @@ func TestSSHPing(t *testing.T) { v := virter.New(l, poolName, networkName, newMockKeystore()) - err := v.PingSSH(context.Background(), MockShellClientBuilder{shell}, vmName, sshPingConfig) + err := v.WaitVmReady(context.Background(), MockShellClientBuilder{shell}, vmName, readyConfig) assert.NoError(t, err) shell.AssertExpectations(t)