Skip to content

Commit

Permalink
Merge pull request #226 from orelmisan/release-0.3-login
Browse files Browse the repository at this point in the history
[release-0.3] Fix login race
  • Loading branch information
orelmisan authored Jan 28, 2024
2 parents 5dc53ee + 85898cf commit 36ad82b
Show file tree
Hide file tree
Showing 11 changed files with 197 additions and 254 deletions.
93 changes: 61 additions & 32 deletions pkg/internal/checkup/checkup.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,34 +54,38 @@ type testExecutor interface {
}

type Checkup struct {
client kubeVirtVMIClient
namespace string
params config.Config
vmiUnderTest *kvcorev1.VirtualMachineInstance
trafficGen *kvcorev1.VirtualMachineInstance
trafficGenConfigMap *k8scorev1.ConfigMap
results status.Results
executor testExecutor
client kubeVirtVMIClient
namespace string
params config.Config
vmiUnderTest *kvcorev1.VirtualMachineInstance
trafficGen *kvcorev1.VirtualMachineInstance
trafficGenConfigMap *k8scorev1.ConfigMap
vmiUnderTestConfigMap *k8scorev1.ConfigMap
results status.Results
executor testExecutor
}

const (
TrafficGenConfigMapNamePrefix = "dpdk-traffic-gen-config"
TrafficGenConfigMapNamePrefix = "dpdk-traffic-gen-config"
vmiUnderTestConfigMapNamePrefix = "vmi-under-test-config"
)

func New(client kubeVirtVMIClient, namespace string, checkupConfig config.Config, executor testExecutor) *Checkup {
const randomStringLen = 5
randomSuffix := rand.String(randomStringLen)

trafficGenCMName := trafficGenConfigMapName(randomSuffix)
vmiUnderTestCMName := vmiUnderTestConfigMapName(randomSuffix)

return &Checkup{
client: client,
namespace: namespace,
params: checkupConfig,
vmiUnderTest: newVMIUnderTest(vmiUnderTestName(randomSuffix), checkupConfig),
trafficGen: newTrafficGen(trafficGenName(randomSuffix), checkupConfig, trafficGenCMName),
trafficGenConfigMap: newTrafficGenConfigMap(trafficGenCMName, checkupConfig),
executor: executor,
client: client,
namespace: namespace,
params: checkupConfig,
vmiUnderTest: newVMIUnderTest(vmiUnderTestName(randomSuffix), checkupConfig, vmiUnderTestCMName),
vmiUnderTestConfigMap: newVMIUnderTestConfigMap(vmiUnderTestCMName, checkupConfig),
trafficGen: newTrafficGen(trafficGenName(randomSuffix), checkupConfig, trafficGenCMName),
trafficGenConfigMap: newTrafficGenConfigMap(trafficGenCMName, checkupConfig),
executor: executor,
}
}

Expand All @@ -93,7 +97,11 @@ func (c *Checkup) Setup(ctx context.Context) (setupErr error) {
const errMessagePrefix = "setup"
var err error

if err = c.createTrafficGenCM(setupCtx); err != nil {
if err = c.createConfigmap(setupCtx, c.trafficGenConfigMap); err != nil {
return fmt.Errorf("%s: %w", errMessagePrefix, err)
}

if err = c.createConfigmap(setupCtx, c.vmiUnderTestConfigMap); err != nil {
return fmt.Errorf("%s: %w", errMessagePrefix, err)
}

Expand All @@ -116,15 +124,14 @@ func (c *Checkup) Setup(ctx context.Context) (setupErr error) {
}()

var updatedVMIUnderTest *kvcorev1.VirtualMachineInstance
updatedVMIUnderTest, err = c.waitForVMIToBoot(setupCtx, c.vmiUnderTest.Name)
updatedVMIUnderTest, err = c.waitForVMIToBeReady(setupCtx, c.vmiUnderTest.Name)
if err != nil {
return err
}

c.vmiUnderTest = updatedVMIUnderTest

var updatedTrafficGen *kvcorev1.VirtualMachineInstance
updatedTrafficGen, err = c.waitForVMIToBoot(setupCtx, c.trafficGen.Name)
updatedTrafficGen, err = c.waitForVMIToBeReady(setupCtx, c.trafficGen.Name)
if err != nil {
return err
}
Expand Down Expand Up @@ -178,7 +185,11 @@ func (c *Checkup) Teardown(ctx context.Context) error {
teardownErrors = append(teardownErrors, fmt.Sprintf("%s: %v", errMessagePrefix, err))
}

if err := c.deleteTrafficGenCM(ctx); err != nil {
if err := c.deleteConfigmap(ctx, c.trafficGenConfigMap); err != nil {
teardownErrors = append(teardownErrors, fmt.Sprintf("%s: %v", errMessagePrefix, err))
}

if err := c.deleteConfigmap(ctx, c.vmiUnderTestConfigMap); err != nil {
teardownErrors = append(teardownErrors, fmt.Sprintf("%s: %v", errMessagePrefix, err))
}

Expand All @@ -201,17 +212,17 @@ func (c *Checkup) Results() status.Results {
return c.results
}

func (c *Checkup) createTrafficGenCM(ctx context.Context) error {
log.Printf("Creating ConfigMap %q...", ObjectFullName(c.namespace, c.trafficGenConfigMap.Name))
func (c *Checkup) createConfigmap(ctx context.Context, configMap *k8scorev1.ConfigMap) error {
log.Printf("Creating ConfigMap %q...", ObjectFullName(c.namespace, configMap.Name))

_, err := c.client.CreateConfigMap(ctx, c.namespace, c.trafficGenConfigMap)
_, err := c.client.CreateConfigMap(ctx, c.namespace, configMap)
return err
}

func (c *Checkup) deleteTrafficGenCM(ctx context.Context) error {
log.Printf("Deleting ConfigMap %q...", ObjectFullName(c.namespace, c.trafficGenConfigMap.Name))
func (c *Checkup) deleteConfigmap(ctx context.Context, configMap *k8scorev1.ConfigMap) error {
log.Printf("Deleting ConfigMap %q...", ObjectFullName(c.namespace, configMap.Name))

return c.client.DeleteConfigMap(ctx, c.namespace, c.trafficGenConfigMap.Name)
return c.client.DeleteConfigMap(ctx, c.namespace, configMap.Name)
}

func (c *Checkup) createVMI(ctx context.Context, vmiToCreate *kvcorev1.VirtualMachineInstance) error {
Expand All @@ -221,9 +232,9 @@ func (c *Checkup) createVMI(ctx context.Context, vmiToCreate *kvcorev1.VirtualMa
return err
}

func (c *Checkup) waitForVMIToBoot(ctx context.Context, name string) (*kvcorev1.VirtualMachineInstance, error) {
func (c *Checkup) waitForVMIToBeReady(ctx context.Context, name string) (*kvcorev1.VirtualMachineInstance, error) {
vmiFullName := ObjectFullName(c.namespace, name)
log.Printf("Waiting for VMI %q to boot...", vmiFullName)
log.Printf("Waiting for VMI %q to be ready...", vmiFullName)
var updatedVMI *kvcorev1.VirtualMachineInstance

conditionFn := func(ctx context.Context) (bool, error) {
Expand All @@ -234,7 +245,7 @@ func (c *Checkup) waitForVMIToBoot(ctx context.Context, name string) (*kvcorev1.
}

for _, condition := range updatedVMI.Status.Conditions {
if condition.Type == kvcorev1.VirtualMachineInstanceAgentConnected && condition.Status == k8scorev1.ConditionTrue {
if condition.Type == kvcorev1.VirtualMachineInstanceReady && condition.Status == k8scorev1.ConditionTrue {
return true, nil
}
}
Expand All @@ -243,10 +254,10 @@ func (c *Checkup) waitForVMIToBoot(ctx context.Context, name string) (*kvcorev1.
}
const pollInterval = 5 * time.Second
if err := wait.PollImmediateUntilWithContext(ctx, pollInterval, conditionFn); err != nil {
return nil, fmt.Errorf("failed to wait for VMI %q to boot: %v", vmiFullName, err)
return nil, fmt.Errorf("failed to wait for VMI %q to be ready: %v", vmiFullName, err)
}

log.Printf("VMI %q had successfully booted", vmiFullName)
log.Printf("VMI %q has successfully reached ready condition", vmiFullName)

return updatedVMI, nil
}
Expand Down Expand Up @@ -304,6 +315,19 @@ func ObjectFullName(namespace, name string) string {
return fmt.Sprintf("%s/%s", namespace, name)
}

func newVMIUnderTestConfigMap(name string, checkupConfig config.Config) *k8scorev1.ConfigMap {
vmiUnderTestConfigData := map[string]string{
config.BootScriptName: generateBootScript(),
}

return configmap.New(
name,
checkupConfig.PodName,
checkupConfig.PodUID,
vmiUnderTestConfigData,
)
}

func newTrafficGenConfigMap(name string, checkupConfig config.Config) *k8scorev1.ConfigMap {
trexConfig := trex.NewConfig(checkupConfig)
trafficGenConfigData := map[string]string{
Expand All @@ -312,6 +336,7 @@ func newTrafficGenConfigMap(name string, checkupConfig config.Config) *k8scorev1
trex.CfgFileName: trexConfig.GenerateCfgFile(),
trex.StreamPyFileName: trexConfig.GenerateStreamPyFile(),
trex.StreamPeerParamsPyFileName: trexConfig.GenerateStreamAddrPyFile(),
config.BootScriptName: generateBootScript(),
}
return configmap.New(
name,
Expand All @@ -332,3 +357,7 @@ func trafficGenName(suffix string) string {
func trafficGenConfigMapName(suffix string) string {
return TrafficGenConfigMapNamePrefix + "-" + suffix
}

func vmiUnderTestConfigMapName(suffix string) string {
return vmiUnderTestConfigMapNamePrefix + "-" + suffix
}
11 changes: 6 additions & 5 deletions pkg/internal/checkup/checkup_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,7 @@ func TestTeardownShouldFailWhen(t *testing.T) {
})
}

func TestTrafficGenCMTeardownFailure(t *testing.T) {
func TestVMConfigMapTeardownFailure(t *testing.T) {
testClient := newClientStub()
testConfig := newTestConfig()

Expand Down Expand Up @@ -417,10 +417,11 @@ func (cs *clientStub) GetVirtualMachineInstance(_ context.Context, namespace, na
return nil, k8serrors.NewNotFound(schema.GroupResource{Group: "kubevirt.io", Resource: "virtualmachineinstances"}, name)
}

vmi.Status.Conditions = append(vmi.Status.Conditions, kvcorev1.VirtualMachineInstanceCondition{
Type: kvcorev1.VirtualMachineInstanceAgentConnected,
Status: k8scorev1.ConditionTrue,
})
vmi.Status.Conditions = append(vmi.Status.Conditions,
kvcorev1.VirtualMachineInstanceCondition{
Type: kvcorev1.VirtualMachineInstanceReady,
Status: k8scorev1.ConditionTrue,
})

return vmi, nil
}
Expand Down
23 changes: 0 additions & 23 deletions pkg/internal/checkup/executor/console/console.go
Original file line number Diff line number Diff line change
Expand Up @@ -182,26 +182,3 @@ func expectBatchWithValidatedSend(expecter expect.Expecter, batch []expect.Batch
res, err := expecter.ExpectBatch(batch, timeout)
return res, err
}

func configureConsole(expecter expect.Expecter, shouldSudo bool) error {
sudoString := ""
if shouldSudo {
sudoString = "sudo "
}
batch := []expect.Batcher{
&expect.BSnd{S: "stty cols 500 rows 500\n"},
&expect.BExp{R: PromptExpression},
&expect.BSnd{S: "echo $?\n"},
&expect.BExp{R: RetValue("0")},
&expect.BSnd{S: fmt.Sprintf("%sdmesg -n 1\n", sudoString)},
&expect.BExp{R: PromptExpression},
&expect.BSnd{S: "echo $?\n"},
&expect.BExp{R: RetValue("0")},
}
const configureConsoleTimeout = 30 * time.Second
resp, err := expecter.ExpectBatch(batch, configureConsoleTimeout)
if err != nil {
log.Printf("%v", resp)
}
return err
}
31 changes: 23 additions & 8 deletions pkg/internal/checkup/executor/console/login.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ import (
"google.golang.org/grpc/codes"
)

func (e Expecter) LoginToCentOS(username, password string) error {
func (e Expecter) LoginToCentOSAsRoot(password string) error {
const (
connectionTimeout = 10 * time.Second
promptTimeout = 5 * time.Second
Expand All @@ -47,9 +47,7 @@ func (e Expecter) LoginToCentOS(username, password string) error {
}

// Do not login, if we already logged in
loggedInPromptRegex := fmt.Sprintf(
`(\[%s@(localhost|centos|%s) ~\]\$ |\[root@(localhost|centos|%s) centos\]\# )`, username, e.vmiName, e.vmiName,
)
loggedInPromptRegex := fmt.Sprintf(`(\[root@(localhost|centos|%s) ~\]\# )`, e.vmiName)
b := []expect.Batcher{
&expect.BSnd{S: "\n"},
&expect.BExp{R: loggedInPromptRegex},
Expand All @@ -67,7 +65,7 @@ func (e Expecter) LoginToCentOS(username, password string) error {
// Using only "login: " would match things like "Last failed login: Tue Jun 9 22:25:30 UTC 2020 on ttyS0"
// and in case the VM's did not get hostname form DHCP server try the default hostname
R: regexp.MustCompile(fmt.Sprintf(`(localhost|centos|%s) login: `, e.vmiName)),
S: fmt.Sprintf("%s\n", username),
S: "root\n",
T: expect.Next(),
Rt: 10,
},
Expand All @@ -87,8 +85,6 @@ func (e Expecter) LoginToCentOS(username, password string) error {
T: expect.OK(),
},
}},
&expect.BSnd{S: "sudo su\n"},
&expect.BExp{R: PromptExpression},
}
const loginTimeout = 2 * time.Minute
res, err := genExpect.ExpectBatch(b, loginTimeout)
Expand All @@ -101,9 +97,28 @@ func (e Expecter) LoginToCentOS(username, password string) error {
}
}

err = configureConsole(genExpect, false)
err = configureConsole(genExpect)
if err != nil {
return err
}
return nil
}

func configureConsole(expecter expect.Expecter) error {
batch := []expect.Batcher{
&expect.BSnd{S: "stty cols 500 rows 500\n"},
&expect.BExp{R: PromptExpression},
&expect.BSnd{S: "echo $?\n"},
&expect.BExp{R: RetValue("0")},
&expect.BSnd{S: "dmesg -n 1\n"},
&expect.BExp{R: PromptExpression},
&expect.BSnd{S: "echo $?\n"},
&expect.BExp{R: RetValue("0")},
}
const configureConsoleTimeout = 30 * time.Second
resp, err := expecter.ExpectBatch(batch, configureConsoleTimeout)
if err != nil {
log.Printf("%v", resp)
}
return err
}
6 changes: 2 additions & 4 deletions pkg/internal/checkup/executor/executor.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,6 @@ type vmiSerialConsoleClient interface {
type Executor struct {
vmiSerialClient vmiSerialConsoleClient
namespace string
vmiUsername string
vmiPassword string
vmiUnderTestEastNICPCIAddress string
trafficGenEastMACAddress string
Expand All @@ -58,7 +57,6 @@ func New(client vmiSerialConsoleClient, namespace string, cfg config.Config) Exe
return Executor{
vmiSerialClient: client,
namespace: namespace,
vmiUsername: config.VMIUsername,
vmiPassword: config.VMIPassword,
vmiUnderTestEastNICPCIAddress: config.VMIEastNICPCIAddress,
trafficGenEastMACAddress: cfg.TrafficGenEastMacAddress.String(),
Expand All @@ -73,13 +71,13 @@ func New(client vmiSerialConsoleClient, namespace string, cfg config.Config) Exe
func (e Executor) Execute(ctx context.Context, vmiUnderTestName, trafficGenVMIName string) (status.Results, error) {
log.Printf("Login to VMI under test...")
vmiUnderTestConsoleExpecter := console.NewExpecter(e.vmiSerialClient, e.namespace, vmiUnderTestName)
if err := vmiUnderTestConsoleExpecter.LoginToCentOS(e.vmiUsername, e.vmiPassword); err != nil {
if err := vmiUnderTestConsoleExpecter.LoginToCentOSAsRoot(e.vmiPassword); err != nil {
return status.Results{}, fmt.Errorf("failed to login to VMI \"%s/%s\": %w", e.namespace, vmiUnderTestName, err)
}

log.Printf("Login to traffic generator...")
trafficGenConsoleExpecter := console.NewExpecter(e.vmiSerialClient, e.namespace, trafficGenVMIName)
if err := trafficGenConsoleExpecter.LoginToCentOS(e.vmiUsername, e.vmiPassword); err != nil {
if err := trafficGenConsoleExpecter.LoginToCentOSAsRoot(e.vmiPassword); err != nil {
return status.Results{}, fmt.Errorf("failed to login to VMI \"%s/%s\": %w", e.namespace, trafficGenVMIName, err)
}

Expand Down
Loading

0 comments on commit 36ad82b

Please sign in to comment.