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

Add reopen container log test. #242

Merged
merged 1 commit into from
Feb 13, 2018
Merged

Add reopen container log test. #242

merged 1 commit into from
Feb 13, 2018

Conversation

Random-Liu
Copy link
Contributor

@Random-Liu Random-Liu commented Feb 13, 2018 •
edited
Loading

Add test for ReopenContainerLog.

I've tested it with cri-containerd:

[k8s.io] Container runtime should support log 
  runtime should support reopening container log [Conformance]
  /home/lantaol/workspace/src/github.com/kubernetes-incubator/cri-tools/pkg/validate/container.go:226
[BeforeEach] [k8s.io] Container
  /home/lantaol/workspace/src/github.com/kubernetes-incubator/cri-tools/pkg/framework/framework.go:50
[BeforeEach] [k8s.io] Container
  /home/lantaol/workspace/src/github.com/kubernetes-incubator/cri-tools/pkg/validate/container.go:63
[BeforeEach] runtime should support log
  /home/lantaol/workspace/src/github.com/kubernetes-incubator/cri-tools/pkg/validate/container.go:194
STEP: create a PodSandbox with log directory
[It] runtime should support reopening container log [Conformance]
  /home/lantaol/workspace/src/github.com/kubernetes-incubator/cri-tools/pkg/validate/container.go:226
STEP: create container with log
STEP: create a container with log and name
STEP: Get image status for image: busybox:1.26
STEP: Create container.
Feb 13 01:44:53.692: INFO: Created container "4e12a1578c465c15a409a24cf279ccab38f191e57603a3c80699b15f26a8c7cb"

STEP: start container with log
STEP: Start container for containerID: 4e12a1578c465c15a409a24cf279ccab38f191e57603a3c80699b15f26a8c7cb
Feb 13 01:44:53.828: INFO: Started container "4e12a1578c465c15a409a24cf279ccab38f191e57603a3c80699b15f26a8c7cb"

Feb 13 01:44:53.828: INFO: Open log file /tmp/podLogTest973226121/PodSandbox-with-log-directory-7c4be0f2-105f-11e8-ac8e-42010af00002/container-reopen-log-test-7ccc3fcf-105f-11e8-ac8e-42010af00002.log
Feb 13 01:44:53.828: INFO: Parse container log succeed
STEP: rename container log
STEP: reopen container log
Feb 13 01:44:53.829: INFO: Open log file /tmp/podLogTest973226121/PodSandbox-with-log-directory-7c4be0f2-105f-11e8-ac8e-42010af00002/container-reopen-log-test-7ccc3fcf-105f-11e8-ac8e-42010af00002.log
Feb 13 01:44:53.829: INFO: Parse container log succeed
Feb 13 01:44:54.830: INFO: Open log file /tmp/podLogTest973226121/PodSandbox-with-log-directory-7c4be0f2-105f-11e8-ac8e-42010af00002/container-reopen-log-test-7ccc3fcf-105f-11e8-ac8e-42010af00002.log
Feb 13 01:44:54.830: INFO: Parse container log succeed
Feb 13 01:44:54.830: INFO: Open log file /tmp/podLogTest973226121/PodSandbox-with-log-directory-7c4be0f2-105f-11e8-ac8e-42010af00002/container-reopen-log-test-7ccc3fcf-105f-11e8-ac8e-42010af00002.log.new
Feb 13 01:44:54.830: INFO: Parse container log succeed
Feb 13 01:44:54.830: INFO: Open log file /tmp/podLogTest973226121/PodSandbox-with-log-directory-7c4be0f2-105f-11e8-ac8e-42010af00002/container-reopen-log-test-7ccc3fcf-105f-11e8-ac8e-42010af00002.log.new
Feb 13 01:44:54.830: INFO: Parse container log succeed
Feb 13 01:44:55.830: INFO: Open log file /tmp/podLogTest973226121/PodSandbox-with-log-directory-7c4be0f2-105f-11e8-ac8e-42010af00002/container-reopen-log-test-7ccc3fcf-105f-11e8-ac8e-42010af00002.log.new
Feb 13 01:44:55.830: INFO: Parse container log succeed
Feb 13 01:44:56.831: INFO: Open log file /tmp/podLogTest973226121/PodSandbox-with-log-directory-7c4be0f2-105f-11e8-ac8e-42010af00002/container-reopen-log-test-7ccc3fcf-105f-11e8-ac8e-42010af00002.log.new
Feb 13 01:44:56.831: INFO: Parse container log succeed
Feb 13 01:44:57.831: INFO: Open log file /tmp/podLogTest973226121/PodSandbox-with-log-directory-7c4be0f2-105f-11e8-ac8e-42010af00002/container-reopen-log-test-7ccc3fcf-105f-11e8-ac8e-42010af00002.log.new
Feb 13 01:44:57.831: INFO: Parse container log succeed
Feb 13 01:44:58.832: INFO: Open log file /tmp/podLogTest973226121/PodSandbox-with-log-directory-7c4be0f2-105f-11e8-ac8e-42010af00002/container-reopen-log-test-7ccc3fcf-105f-11e8-ac8e-42010af00002.log.new
Feb 13 01:44:58.832: INFO: Parse container log succeed
[AfterEach] runtime should support log
  /home/lantaol/workspace/src/github.com/kubernetes-incubator/cri-tools/pkg/validate/container.go:198
STEP: stop PodSandbox
STEP: delete PodSandbox
STEP: clean up the TempDir
[AfterEach] [k8s.io] Container
  /home/lantaol/workspace/src/github.com/kubernetes-incubator/cri-tools/pkg/framework/framework.go:51

• [SLOW TEST:7.404 seconds]
[k8s.io] Container
/home/lantaol/workspace/src/github.com/kubernetes-incubator/cri-tools/pkg/framework/framework.go:72
  runtime should support log
  /home/lantaol/workspace/src/github.com/kubernetes-incubator/cri-tools/pkg/validate/container.go:190
    runtime should support reopening container log [Conformance]
    /home/lantaol/workspace/src/github.com/kubernetes-incubator/cri-tools/pkg/validate/container.go:226

@feiskyer @yujuhong @mrunalp
Signed-off-by: Lantao Liu [email protected]

@k8s-ci-robot k8s-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Feb 13, 2018
@Random-Liu Random-Liu mentioned this pull request Feb 13, 2018
Merged
@feiskyer
Copy link
Member

feiskyer commented Feb 13, 2018 •
edited
Loading

@Random-Liu Dockershim doesn't support this operation yet, see https://github.com/kubernetes/kubernetes/blob/master/pkg/kubelet/dockershim/docker_logs.go#L28

Would you file a PR to fix it?

@Random-Liu
Copy link
Contributor Author

Random-Liu commented Feb 13, 2018 •
edited
Loading

@feiskyer We won't support it for dockershim, that's why I want to skip it in the CI.

We'll support it in both cri-containerd and cri-o I believe. I think it still should be conformance test, just dockershim is not fully CRI conformant.

@Random-Liu Random-Liu force-pushed the reopen-container-log-test branch from 21e5328 to 37fd236 Compare February 13, 2018 03:14
feiskyer
feiskyer reviewed Feb 13, 2018
View reviewed changes
hack/run-critest.sh Outdated
@@ -30,7 +30,8 @@ docker run --rm -v /usr/local/bin:/target jpetazzo/nsenter
sleep 10

# Run e2e test cases
critest v
# Skip reopen container log test because docker doesn't support it.
critest --ginkgo-flags="--skip=runtime\sshould\ssupport\sreopening\scontainer\slog" v
Copy link
Member

Choose a reason for hiding this comment

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

Use --skip instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

@mrunalp
Copy link
Member

mrunalp commented Feb 13, 2018

@Random-Liu From the output, it appears that the log file is changing. Shouldn't the name be the same? kubelet can archive and rename the old one.

@Random-Liu Random-Liu force-pushed the reopen-container-log-test branch from 37fd236 to e778671 Compare February 13, 2018 05:47
@Random-Liu
Copy link
Contributor Author

From the output, it appears that the log file is changing. Shouldn't the name be the same? kubelet can archive and rename the old one.

Kubelet will rotate the existing log file, and let container runtime to recreate original log file. The log file name is not changed.

@feiskyer
Copy link
Member

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Feb 13, 2018
@feiskyer feiskyer merged commit c87ea76 into kubernetes-sigs:master Feb 13, 2018
@Random-Liu Random-Liu deleted the reopen-container-log-test branch February 13, 2018 07:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@feiskyer feiskyer feiskyer left review comments

Assignees

@feiskyer feiskyer

Labels
cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@Random-Liu @feiskyer @mrunalp @k8s-ci-robot