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

e2e: gpu: add a basic tensorflow test #1374

Merged
merged 1 commit into from
Aug 22, 2023
Merged

Conversation

tkatila
Copy link
Contributor

@tkatila tkatila commented Apr 6, 2023

No description provided.

@tkatila tkatila force-pushed the e2e-gpu-tf branch 3 times, most recently from b833633 to 504f09e Compare April 11, 2023 11:01
@codecov-commenter
Copy link

codecov-commenter commented Apr 11, 2023

Codecov Report

Merging #1374 (1a1534d) into main (a70651f) will not change coverage.
The diff coverage is n/a.

❗ Current head 1a1534d differs from pull request most recent head cf90a6a. Consider uploading reports for the commit cf90a6a to get more accurate results

@@           Coverage Diff           @@
##             main    #1374   +/-   ##
=======================================
  Coverage   50.04%   50.04%           
=======================================
  Files          43       43           
  Lines        4884     4884           
=======================================
  Hits         2444     2444           
  Misses       2301     2301           
  Partials      139      139           

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

demo/intel-gpu-tensorflow/kustomization.yaml Outdated Show resolved Hide resolved
test/e2e/gpu/gpu.go Outdated Show resolved Hide resolved
@tkatila
Copy link
Contributor Author

tkatila commented Apr 12, 2023

This is stuck until I can get access to the bmaas machine to debug the failure.

@tkatila tkatila force-pushed the e2e-gpu-tf branch 2 times, most recently from 883d612 to b638697 Compare April 17, 2023 07:22
@tkatila tkatila force-pushed the e2e-gpu-tf branch 2 times, most recently from 5eaf78d to 7616ff0 Compare May 11, 2023 11:27
@tkatila tkatila force-pushed the e2e-gpu-tf branch 2 times, most recently from b61409e to ed09577 Compare August 17, 2023 09:10
@tkatila tkatila marked this pull request as ready for review August 17, 2023 09:47
@tkatila tkatila force-pushed the e2e-gpu-tf branch 3 times, most recently from 1645929 to cf90a6a Compare August 17, 2023 11:56
@tkatila
Copy link
Contributor Author

tkatila commented Aug 17, 2023

Please review.

Comment on lines 137 to 145
ginkgo.By("checking log output")
log, err := e2epod.GetPodLogs(ctx, f.ClientSet, f.Namespace.Name, tfPodName, containerName)

framework.Logf("logs: %s", log)

if err != nil {
framework.Failf("unable to get log from pod: %v", err)
}

Copy link
Contributor

Choose a reason for hiding this comment

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

@hj-johannes-lee can you comment on the logging part

Copy link
Contributor

Choose a reason for hiding this comment

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

made a separate review because the previous line should be also changed IMO.

Comment on lines 135 to 143
e2epod.NewPodClient(f).WaitForSuccess(ctx, tfPodName, 300*time.Second)

ginkgo.By("checking log output")
log, err := e2epod.GetPodLogs(ctx, f.ClientSet, f.Namespace.Name, tfPodName, containerName)

framework.Logf("logs: %s", log)

if err != nil {
framework.Failf("unable to get log from pod: %v", err)
Copy link
Contributor

Choose a reason for hiding this comment

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

	err := e2epod.WaitForPodSuccessInNamespaceTimeout(ctx, f.ClientSet, tfPodName, f.Namespace.Name, 300*time.Second)
	gomega.Expect(err).To(gomega.BeNil(), utils.GetPodLogs(ctx, f, tfPodName, containerName))

Signed-off-by: Tuomas Katila <tuomas.katila@intel.com>
Copy link
Contributor

@mythi mythi left a comment

Choose a reason for hiding this comment

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

looks nice

Copy link
Contributor

@hj-johannes-lee hj-johannes-lee left a comment

Choose a reason for hiding this comment

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

LGTM! :)

@hj-johannes-lee hj-johannes-lee merged commit 6861ef5 into intel:main Aug 22, 2023
69 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants