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.

Sign up for GitHub

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

Jump to bottom

Incremental improvements to kubelet e2e tests #24426

Merged
merged 1 commit into from
Apr 20, 2016
Merged

Incremental improvements to kubelet e2e tests #24426

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion hack/jenkins/job-configs/kubernetes-jenkins/node-e2e.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@
- ansicolor:
colormap: xterm
- timeout:
timeout: 45
timeout: 90
fail: true
- timestamps
- inject:
Expand Down
5 changes: 3 additions & 2 deletions test/e2e_node/e2e_node_suite_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@ var _ = AfterSuite(func() {
glog.Infof("Stopping node services...")
e2es.stop()
}
glog.Infof("Tests Finished")
})

var _ Reporter = &LogReporter{}
Expand All @@ -91,7 +92,7 @@ type LogReporter struct{}
func (lr *LogReporter) SpecSuiteWillBegin(config config.GinkgoConfigType, summary *types.SuiteSummary) {
b := &bytes.Buffer{}
b.WriteString("******************************************************\n")
glog.V(0).Infof(b.String())
glog.Infof(b.String())
}

func (lr *LogReporter) BeforeSuiteDidRun(setupSummary *types.SetupSummary) {}
Expand All @@ -115,5 +116,5 @@ func (lr *LogReporter) SpecSuiteDidEnd(summary *types.SuiteSummary) {
b.WriteString(fmt.Sprintf("etcd output:\n%s\n", e2es.etcdCombinedOut.String()))
}
b.WriteString("******************************************************\n")
glog.V(0).Infof(b.String())
glog.Infof(b.String())
}
8 changes: 4 additions & 4 deletions test/e2e_node/e2e_remote.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ func init() {
glog.Fatal(err)
}
sshOptionsMap = map[string]string{
"gce": fmt.Sprintf("-i %s/.ssh/google_compute_engine -o UserKnownHostsFile=/dev/null -o IdentitiesOnly=yes -o CheckHostIP=no -o StrictHostKeyChecking=no", usr.HomeDir),
"gce": fmt.Sprintf("-i %s/.ssh/google_compute_engine -o UserKnownHostsFile=/dev/null -o IdentitiesOnly=yes -o CheckHostIP=no -o StrictHostKeyChecking=no -o ServerAliveInterval=30", usr.HomeDir),
}
}

Expand Down Expand Up @@ -118,14 +118,14 @@ func CreateTestArchive() string {
}

// RunRemote copies the archive file to a /tmp file on host, unpacks it, and runs the e2e_node.test
func RunRemote(archive string, host string, deleteFiles bool) (string, error) {
func RunRemote(archive string, host string, cleanup bool) (string, error) {
// Create the temp staging directory
tmp := fmt.Sprintf("/tmp/gcloud-e2e-%d", rand.Int31())
_, err := RunSshCommand("ssh", host, "--", "mkdir", tmp)
if err != nil {
return "", err
}
if deleteFiles {
if cleanup {
defer func() {
output, err := RunSshCommand("ssh", host, "--", "rm", "-rf", tmp)
if err != nil {
Expand Down Expand Up @@ -155,7 +155,7 @@ func RunRemote(archive string, host string, deleteFiles bool) (string, error) {
cmd = getSshCommand(" && ",
fmt.Sprintf("cd %s", tmp),
fmt.Sprintf("tar -xzvf ./%s", archiveName),
fmt.Sprintf("./e2e_node.test --logtostderr --v 2 --build-services=false --node-name=%s", host),
fmt.Sprintf("./e2e_node.test --logtostderr --v 2 --build-services=false --stop-services=%t --node-name=%s", cleanup, host),
)
output, err := RunSshCommand("ssh", host, "--", "sh", "-c", cmd)
if err != nil {
Expand Down
2 changes: 1 addition & 1 deletion test/e2e_node/e2e_service.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ func (es *e2eService) stop() {
if es.apiServerCmd != nil {
err := es.apiServerCmd.Process.Kill()
if err != nil {
glog.Errorf("Failed to stop be-apiserver.\n%v", err)
glog.Errorf("Failed to stop kube-apiserver.\n%v", err)
}
}
if es.etcdCmd != nil {
Expand Down
2 changes: 0 additions & 2 deletions test/e2e_node/privileged_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ import (
"net/url"
"time"

"github.com/golang/glog"
"k8s.io/kubernetes/pkg/api"
"k8s.io/kubernetes/pkg/api/unversioned"
"k8s.io/kubernetes/pkg/apimachinery/registered"
Expand Down Expand Up @@ -151,7 +150,6 @@ func (config *PrivilegedPodTestConfig) dialFromContainer(containerIP string, con
var output map[string]string
err = json.Unmarshal([]byte(stdout), &output)
Expect(err).NotTo(HaveOccurred(), fmt.Sprintf("Could not unmarshal curl response: %s", stdout))
glog.Infof("Deserialized output is %v", output)
return output
}
Copy link
Contributor

Choose a reason for hiding this comment

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

This output was just not useful at all?

Copy link
Member Author

Choose a reason for hiding this comment

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

cc @yujuhong

The tests that call this log the map if it does not match expectations. I don't think we want to output this when the tests pass since it clutters up the build log.

Copy link
Contributor

Choose a reason for hiding this comment

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

I copied this from the original test. The output seems pretty meaningful. E.g., "privileged_test.go:154] Deserialized output is map[output:RTNETLINK answers: Operation not permitted"

Do we have any general rules for logging in a node e2e suite? A cluster e2e test logs quite a lot at the info level, but I've just noticed that it's very different in the node e2e suite.

Copy link
Member Author

Choose a reason for hiding this comment

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

I expect that message is ignored unless the test fails, in which case it should be printed out by the assertion. That output is actually expected, but can make debugging unrelated failures hard because it looks like an error message even though it is printed at info level.

My opinion would be that anything besides the test results themselves are just debug information that we should only assume will be seen if there is a test failure. To the extent possible, debug information should only be printed if it is related to a failed tests.

Copy link
Contributor

Choose a reason for hiding this comment

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

I agree in general that we should keep the output to the minimum, but fwiw, comparing the logs of a passed test against those of a failed test is helpful at times. I guess we won't need that if every test is well-written :)


Expand Down