Skip to content

Commit

Permalink
Merge pull request #1214 from sebsoto/nodeLogsRetries
Browse files Browse the repository at this point in the history
Make log test more resilient
  • Loading branch information
openshift-merge-robot committed Aug 26, 2022
2 parents 4e6484f + 85a10ae commit 7d47218
Show file tree
Hide file tree
Showing 2 changed files with 30 additions and 4 deletions.
8 changes: 7 additions & 1 deletion hack/run-ci-e2e-test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,13 @@ WMCO_PATH_OPTION=""
export CGO_ENABLED=0

get_WMCO_logs() {
oc logs -l name=windows-machine-config-operator -n $WMCO_DEPLOY_NAMESPACE --tail=-1 >> "$ARTIFACT_DIR"/wmco.log
retries=0
until [ $retries -gt 4 ] || oc logs -l name=windows-machine-config-operator -n $WMCO_DEPLOY_NAMESPACE --tail=-1 >> "$ARTIFACT_DIR"/wmco.log
do
let retries+=1
echo "failed to get WMCO logs"
sleep 5
done
}

TEST="all"
Expand Down
26 changes: 23 additions & 3 deletions test/e2e/logs_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package e2e
import (
"fmt"
"io/ioutil"
"log"
"os"
"os/exec"
"path/filepath"
Expand All @@ -11,6 +12,9 @@ import (

"github.com/pkg/errors"
"github.com/stretchr/testify/assert"
"k8s.io/apimachinery/pkg/util/wait"

"github.com/openshift/windows-machine-config-operator/pkg/retry"
)

// testNodeLogs ensures that all required log files were created, and copies them to the test's artifact directory
Expand All @@ -35,12 +39,28 @@ func testNodeLogs(t *testing.T) {
for _, file := range mandatoryLogs {
// A subtest is useful here to attempt to get all the logs and not bail on the first error
t.Run(node.Name+"/"+file, func(t *testing.T) {
assert.NoError(t, retrieveLog(node.Name, file, nodeDir))
err := wait.PollImmediate(retry.Interval, retry.ResourceChangeTimeout, func() (bool, error) {
err := retrieveLog(node.GetName(), file, nodeDir)
if err != nil {
log.Printf("unable to retrieve log %s from node %s: %s", file, node.GetName(), err)
return false, nil
}
return true, nil
})
assert.NoError(t, err)
})
}
// Grab the optional logs for debugging purposes
for _, file := range optionalLogs {
_ = retrieveLog(node.Name, file, nodeDir)
// These logs aren't guaranteed to exist, so its better to ignore any error
_ = wait.PollImmediate(retry.Interval, retry.ResourceChangeTimeout, func() (bool, error) {
err := retrieveLog(node.GetName(), file, nodeDir)
if err != nil {
log.Printf("unable to retrieve log %s from node %s: %s", file, node.GetName(), err)
return false, nil
}
return true, nil
})
}
}
}
Expand All @@ -51,7 +71,7 @@ func retrieveLog(nodeName, srcPath, destDir string) error {
cmd := exec.Command("oc", "adm", "node-logs", "--path="+srcPath, nodeName)
out, err := cmd.Output()
if err != nil {
return errors.Wrap(err, "unable to use oc adm to get log")
return errors.Wrapf(err, "oc adm node-logs failed with output: %s", string(out))
}
// Save log files to the artifact directory
splitPath := strings.Split(srcPath, "/")
Expand Down

0 comments on commit 7d47218

Please sign in to comment.