[release-0.3] Fix login race

pkg/internal/checkup/checkup.go

@@ -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,
 	}
 }
 
@@ -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)
 	}
 
@@ -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
 	}
@@ -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))
 	}
 
@@ -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 {
@@ -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) {
@@ -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
 			}
 		}
@@ -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
 }
@@ -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{
@@ -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,
@@ -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
+}

pkg/internal/checkup/checkup_test.go

@@ -200,7 +200,7 @@ func TestTeardownShouldFailWhen(t *testing.T) {
 	})
 }
 
-func TestTrafficGenCMTeardownFailure(t *testing.T) {
+func TestVMConfigMapTeardownFailure(t *testing.T) {
 	testClient := newClientStub()
 	testConfig := newTestConfig()
 
@@ -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
 }

pkg/internal/checkup/executor/console/console.go

@@ -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
-}

pkg/internal/checkup/executor/console/login.go

@@ -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
@@ -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},
@@ -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,
 			},
@@ -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)
@@ -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
+}

pkg/internal/checkup/executor/executor.go

@@ -43,7 +43,6 @@ type vmiSerialConsoleClient interface {
 type Executor struct {
 	vmiSerialClient                  vmiSerialConsoleClient
 	namespace                        string
-	vmiUsername                      string
 	vmiPassword                      string
 	vmiUnderTestEastNICPCIAddress    string
 	trafficGenEastMACAddress         string
@@ -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(),
@@ -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)
 	}