e2e: make demo pod logged only when failed running

[hj-johannes-lee]

fixes: #1252

[codecov-commenter]

Codecov Report

Merging #1470 (042be9d) into main (4c58a78) will not change coverage.
Report is 2 commits behind head on main.
The diff coverage is n/a.

❗ Current head 042be9d differs from pull request most recent head de0b4f6. Consider uploading reports for the commit de0b4f6 to get more accurate results

@@           Coverage Diff           @@
##             main    #1470   +/-   ##
=======================================
  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

[hj-johannes-lee]

There are some parts that we did not put log printing after running a demo pod.

intel-device-plugins-for-kubernetes/test/e2e/fpga/fpga.go

Lines 111 to 117 in c7feb2d

	ginkgo.By("waiting the pod to finish successfully")
	e2epod.NewPodClient(fmw).WaitForSuccess(ctx, pod.ObjectMeta.Name, 60*time.Second)
	// If WaitForSuccess fails, ginkgo doesn't show the logs of the failed container.
	// Replacing WaitForSuccess with WaitForFinish + 'kubelet logs' would show the logs
	//fmw.PodClient().WaitForFinish(pod.ObjectMeta.Name, 60*time.Second)
	//framework.RunKubectlOrDie(fmw.Namespace.Name, "logs", pod.ObjectMeta.Name)

intel-device-plugins-for-kubernetes/test/e2e/qat/qatplugin_dpdk.go

Lines 167 to 181 in c7feb2d

	ginkgo.It("deploys a crypto pod requesting QAT resources", func(ctx context.Context) {
	ginkgo.By("submitting a crypto pod requesting QAT resources")
	e2ekubectl.RunKubectlOrDie(f.Namespace.Name, "apply", "-k", filepath.Dir(cryptoTestYamlPath))
	ginkgo.By("waiting the crypto pod to finish successfully")
	e2epod.NewPodClient(f).WaitForSuccess(ctx, "qat-dpdk-test-crypto-perf-tc1", 60*time.Second)
	})
	ginkgo.It("deploys a compress pod requesting QAT resources", func(ctx context.Context) {
	ginkgo.By("submitting a compress pod requesting QAT resources")
	e2ekubectl.RunKubectlOrDie(f.Namespace.Name, "apply", "-k", filepath.Dir(compressTestYamlPath))
	ginkgo.By("waiting the compress pod to finish successfully")
	e2epod.NewPodClient(f).WaitForSuccess(ctx, "qat-dpdk-test-compress-perf-tc1", 60*time.Second)
	})

intel-device-plugins-for-kubernetes/test/e2e/sgx/sgx.go

Lines 117 to 119 in c7feb2d

	ginkgo.By("waiting the pod to finish successfully")
	e2epod.NewPodClient(f).WaitForSuccess(ctx, pod.ObjectMeta.Name, 60*time.Second)

Then, do we add it in this PR? Or was there any reason that we do not need to check the logs from those?

[mythi]

Then, do we add it in this PR? Or was there any reason that we do not need to check the logs from those?

No reason. In the past each e2e module was worked on independently by different individuals and that made the implementations to differ. I hope your work can make them to be more consistent.

If this PR plans to fix #1252, let's do that for all of the devices then and so that they work the same. Note that the ask for #1252 to not log output when there are no errors and only log when the sample app fails.

[hj-johannes-lee]

No reason. In the past each e2e module was worked on independently by different individuals and that made the implementations to differ. I hope your work can make them to be more consistent.

I see. I felt that it would be good to have a common function for pod logging, then it would be consistent in any case. The function does: 1. check if the error is nil or not 2. if not nil, then prints logs 3 and then fail with the error

If this PR plans to fix #1252, let's do that for all of the devices then and so that they work the same. Note that the ask for #1252 to not log output when there are no errors and only log when the sample app fails.

Ok, with the function I made, we can also achieve that only when there are errors, logs are printed.

[hj-johannes-lee]

Ready for review!

[tkatila]

nitpick, can you squash commits into one or two? I don't think each device merits a separate commit.

Otherwise, looks good!

[hj-johannes-lee]

Idk why, but after changing to gomega way, linter govet (shadow the declaration of err earlier) complains a lot in many random places though there was no change there.

Actually in the first place when I changed the structure of e2e tests to have layers like Context() or BeforeEach(), we should have considered this matter.

But, anyway, I included in this PR in the first commit to prevent all the possible problems in the future.!

[hj-johannes-lee]

Something else to be chaged?

[mythi]

Something else to be chaged?

@tkatila needs to approve to unblock merging but e2e-* are also not healthy so we will want to get everything working again before merging

[hj-johannes-lee]

@tkatila Sorry, could you approve again? I should have ran make lint before pushing.
@mythi maybe ready to merge?

[mythi]

@tkatila Sorry, could you approve again? I should have ran make lint before pushing. @mythi maybe ready to merge?

I'd propose we wait e2e-fpga back online since this is not urgent