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

NO-JIRA: network, virt: Retry virtctl console cmd on err #29520

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
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
133 changes: 75 additions & 58 deletions test/extended/networking/kubevirt/console.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,12 +24,15 @@ package kubevirt
import (
"bufio"
"fmt"
"log"
"regexp"
"strings"
"time"

. "github.com/onsi/ginkgo/v2"

"k8s.io/client-go/util/retry"

"google.golang.org/grpc/codes"

expect "github.com/google/goexpect"
Expand Down Expand Up @@ -189,69 +192,83 @@ func LoginToFedora(virtctlPath, vmiNamespace, vmiName, user, password string) er

// LoginToFedora performs a console login to a Fedora base VM
func LoginToFedoraWithHostname(virtctlPath, vmiNamespace, vmiName, user, password, hostname string) error {
expecter, _, err := newExpecter(virtctlPath, vmiNamespace, vmiName, consoleConnectionTimeout, expect.Verbose(true), expect.VerboseWriter(GinkgoWriter))
if err != nil {
return err
}
defer expecter.Close()
err := retry.OnError(retry.DefaultBackoff,
func(err error) bool {
return strings.Contains(err.Error(), "expect: Process not running")
},
func() error {
Comment on lines +195 to +199
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit:

Suggested change
err := retry.OnError(retry.DefaultBackoff,
func(err error) bool {
return strings.Contains(err.Error(), "expect: Process not running")
},
func() error {
err := retry.OnError(
retry.DefaultBackoff,
func(err error) bool {
return strings.Contains(err.Error(), "expect: Process not running")
},
func() error {

expecter, _, err := newExpecter(virtctlPath, vmiNamespace, vmiName, consoleConnectionTimeout,
expect.Verbose(true),
expect.VerboseWriter(GinkgoWriter),
expect.DebugCheck(log.New(GinkgoWriter, "expect: virtctl console check", 0 /* no flags */)),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what does this do @qinqon ? Just log that we have to re-try establishing the connection ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It runs the check that of "virtctl command" running and prints it and return back the result, so if it fails we will se the stdout of the virtcl command itself.

)
if err != nil {
return err
}
defer expecter.Close()

err = expecter.Send("\n")
if err != nil {
return err
}
err = expecter.Send("\n")
if err != nil {
return err
}

// Do not login, if we already logged in
loggedInPromptRegex := fmt.Sprintf(
`(\[%s@%s ~\]\$ |\[root@%s %s\]\# )`, user, hostname, hostname, user,
)
b := []expect.Batcher{
&expect.BSnd{S: "\n"},
&expect.BExp{R: loggedInPromptRegex},
}
_, err = expecter.ExpectBatch(b, promptTimeout)
if err == nil {
return nil
}
// Do not login, if we already logged in
loggedInPromptRegex := fmt.Sprintf(
`(\[%s@%s ~\]\$ |\[root@%s %s\]\# )`, user, hostname, hostname, user,
)
b := []expect.Batcher{
&expect.BSnd{S: "\n"},
&expect.BExp{R: loggedInPromptRegex},
}
_, err = expecter.ExpectBatch(b, promptTimeout)
if err == nil {
return nil
}

b = []expect.Batcher{
&expect.BSnd{S: "\n"},
&expect.BSnd{S: "\n"},
&expect.BCas{C: []expect.Caser{
&expect.Case{
// 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(`%s login: `, hostname)),
S: user + "\n",
T: expect.Next(),
Rt: 10,
},
&expect.Case{
R: regexp.MustCompile(`Password:`),
S: password + "\n",
T: expect.Next(),
Rt: 10,
},
&expect.Case{
R: regexp.MustCompile(`Login incorrect`),
T: expect.LogContinue("Failed to log in", expect.NewStatus(codes.PermissionDenied, "login failed")),
Rt: 10,
},
&expect.Case{
R: regexp.MustCompile(loggedInPromptRegex),
T: expect.OK(),
},
}},
&expect.BSnd{S: "sudo su\n"},
&expect.BExp{R: PromptExpression},
}
res, err := expecter.ExpectBatch(b, loginTimeout)
if err != nil {
return fmt.Errorf("Login failed: %+v: %v", res, err)
}
b = []expect.Batcher{
&expect.BSnd{S: "\n"},
&expect.BSnd{S: "\n"},
&expect.BCas{C: []expect.Caser{
&expect.Case{
// 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(`%s login: `, hostname)),
S: user + "\n",
T: expect.Next(),
Rt: 10,
},
&expect.Case{
R: regexp.MustCompile(`Password:`),
S: password + "\n",
T: expect.Next(),
Rt: 10,
},
&expect.Case{
R: regexp.MustCompile(`Login incorrect`),
T: expect.LogContinue("Failed to log in", expect.NewStatus(codes.PermissionDenied, "login failed")),
Rt: 10,
},
&expect.Case{
R: regexp.MustCompile(loggedInPromptRegex),
T: expect.OK(),
},
}},
&expect.BSnd{S: "sudo su\n"},
&expect.BExp{R: PromptExpression},
}
_, err = expecter.ExpectBatch(b, loginTimeout)
if err != nil {
return err
}

err = configureConsole(expecter, false)
err = configureConsole(expecter, false)
if err != nil {
return err
}
return nil
})
if err != nil {
return err
return fmt.Errorf("Login failed: %v", err)
}
return nil
}
Expand Down