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

Reopen log file support #1450

Merged
merged 3 commits into from
Mar 15, 2018
Merged

Reopen log file support #1450

merged 3 commits into from
Mar 15, 2018

Conversation

mrunalp
Copy link
Member

@mrunalp mrunalp commented Mar 12, 2018

- What I did
Implemented container reopen log.

- How I did it
Added code to call conmon ctl file with the message to reopen log file.

- How to verify it
crictl tests pass

- Description for the changelog

@mrunalp mrunalp requested a review from runcom as a code owner March 12, 2018 23:17
@k8s-ci-robot k8s-ci-robot added cncf-cla: yes size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Mar 12, 2018
@mrunalp
Copy link
Member Author

mrunalp commented Mar 12, 2018

/test all

@mrunalp
Copy link
Member Author

mrunalp commented Mar 13, 2018

I will add some synchronization, lack of which may be cause of the failures. Hold on restarting tests before I update the PR.

}
defer controlFile.Close()

_, err = fmt.Fprintf(controlFile, "%d %d %d\n", 2, 0, 0)
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this be done in one line, since you are never resetting err field.

@mrunalp
Copy link
Member Author

mrunalp commented Mar 13, 2018

@runcom @nalind Seeing some image cleanup failures in integration tests here which shouldn't be related to my changes.

@runcom
Copy link
Member

runcom commented Mar 14, 2018

looking into this

@runcom
Copy link
Member

runcom commented Mar 14, 2018

critest_fedora failure seems legit somehow:


�[91m�[1m[Fail] �[0m�[90m[k8s.io] Container �[0m�[0mruntime should support log �[0m�[91m�[1m[It] runtime should support reopening container log [Conformance] �[0m
�[37m/go/src/github.com/kubernetes-incubator/cri-tools/pkg/validate/container.go:245�[0m

Looking at integration tests now

@runcom
Copy link
Member

runcom commented Mar 14, 2018

/test integration
/test critest_fedora

@mrunalp
Copy link
Member Author

mrunalp commented Mar 14, 2018

@runcom Yeah, I am adding sync code to fix that critest failure but the others don't look related.

@mrunalp
Copy link
Member Author

mrunalp commented Mar 14, 2018

Updated.

@mrunalp
Copy link
Member Author

mrunalp commented Mar 14, 2018

/test critest

@mrunalp
Copy link
Member Author

mrunalp commented Mar 14, 2018

@runcom The critests pass here now.

@mrunalp
Copy link
Member Author

mrunalp commented Mar 14, 2018

I'll do another round just to check if there are no races anymore.

@mrunalp
Copy link
Member Author

mrunalp commented Mar 14, 2018

/test critest

@mrunalp
Copy link
Member Author

mrunalp commented Mar 14, 2018

okay critests are fine here now.

@mrunalp
Copy link
Member Author

mrunalp commented Mar 14, 2018

/test integration

@mrunalp
Copy link
Member Author

mrunalp commented Mar 14, 2018

crictl images is failing because we are returning empty Image structs:

time="2018-03-14 15:33:48.250590806-07:00" level=debug msg="ListImagesResponse: &ListImagesResponse{Images:[&Image{Id:,RepoTags:[],RepoDigests:[],Size_:0,Uid:nil,Username:,} &Image{Id:,RepoTags:[],RepoDigests:[],Size_:0,Uid:nil,Username:,} &Image{Id:,RepoTags:[],RepoDigests:[],Size_:0,Uid:nil,Username:,} &Image{Id:,RepoTags:[],RepoDigests:[],Size_:0,Uid:nil,Username:,} &Image{Id:,RepoTags:[],RepoDigests:[],Size_:0,Uid:nil,Username:,} &Image{Id:,RepoTags:[],RepoDigests:[],Size_:0,Uid:nil,Username:,} &Image{Id:,RepoTags:[],RepoDigests:[],Size_:0,Uid:nil,Username:,} &Image{Id:,RepoTags:[],RepoDigests:[],Size_:0,Uid:nil,Username:,} &Image{Id:,RepoTags:[],RepoDigests:[],Size_:0,Uid:nil,Username:,} &Image{Id:,RepoTags:[],RepoDigests:[],Size_:0,Uid:nil,Username:,} &Image{Id:,RepoTags:[],RepoDigests:[],Size_:0,Uid:nil,Username:,} &Image{Id:,RepoTags:[],RepoDigests:[],Size_:0,Uid:nil,Username:,} &Image{Id:,RepoTags:[],RepoDigests:[],Size_:0,Uid:nil,Username:,} &Image{Id:,RepoTags:[],RepoDigests:[],Size_:0,Uid:nil,Username:,} &Image{Id:,RepoTags:[],RepoDigests:[],Size_:0,Uid:nil,Username:,} &Image{Id:,RepoTags:[],RepoDigests:[],Size_:0,Uid:nil,Username:,} &Image{Id:,RepoTags:[],RepoDigests:[],Size_:0,Uid:nil,Username:,} &Image{Id:,RepoTags:[],RepoDigests:[],Size_:0,Uid:nil,Username:,} &Image{Id:,RepoTags:[],RepoDigests:[],Size_:0,Uid:nil,Username:,} &Image{Id:,RepoTags:[],RepoDigests:[],Size_:0,Uid:nil,Username:,} &Image{Id:,RepoTags:[],RepoDigests:[],Size_:0,Uid:nil,Username:,} &Image{Id:,RepoTags:[],RepoDigests:[],Size_:0,Uid:nil,Username:,} &Image{Id:,RepoTags:[],RepoDigests:[],Size_:0,Uid:nil,Username:,} &Image{Id:,RepoTags:[],RepoDigests:[],Size_:0,Uid:nil,Username:,} &Image{Id:,RepoTags:[],RepoDigests:[],Size_:0,Uid:nil,Username:,} &Image{Id:396d37281960e41e91173651516be6ba36ebae3c0da16474fc81019f4da4d476,RepoTags:[docker.io/kubernetes/pause:latest],RepoDigests:[docker.io/kubernetes/pause@sha256:1192104265e01401ff0215573bf9c85603ff7895812cd9897b5d58668854354f],Size_:250665,Uid:nil,Username:,} &Image{Id:8880276d6a8cb192788fa4789d55ed1d23e63c9860d497d0e4caeab286efa190,RepoTags:[gcr.io/google_containers/k8s-dns-sidecar-amd64:1.14.4],RepoDigests:[gcr.io/google_containers/k8s-dns-sidecar-amd64@sha256:97074c951046e37d3cbb98b82ae85ed15704a290cce66a8314e7f846404edde9],Size_:42079330,Uid:nil,Username:nobody,} &Image{Id:c4aa6d42293a2858290a9308eb0f1e57ef773ef6bf6c0f75ac8bbe6e0e309719,RepoTags:[gcr.io/google_containers/k8s-dns-kube-dns-amd64:1.14.4],RepoDigests:[gcr.io/google_containers/k8s-dns-kube-dns-amd64@sha256:40790881bbe9ef4ae4ff7fe8b892498eecb7fe6dcc22661402f271e03f7de344],Size_:49647014,Uid:nil,Username:,} &Image{Id:a2f0854d5ab95840aef19abdff22cbcdc33a4d0fc5d97a8346acc8d961ea96a3,RepoTags:[gcr.io/google_containers/k8s-dns-dnsmasq-nanny-amd64:1.14.4],RepoDigests:[gcr.io/google_containers/k8s-dns-dnsmasq-nanny-amd64@sha256:aeeb994acbc505eabc7415187cd9edb38cbb5364dc1c2fc748154576464b3dc2],Size_:41685612,Uid:nil,Username:,} &Image{Id:408b428dafe484f28f487bffa13d4f51b8441896d817952246e586345e5094b6,RepoTags:[docker.io/library/busybox:1.26],RepoDigests:[docker.io/library/busybox@sha256:be3c11fdba7cfe299214e46edc642e09514dbb9bbefcd0d3836c05a1e0cd0642],Size_:1310706,Uid:nil,Username:,} &Image{Id:d8d858bfd928567662e049939cdc716d95c28b6ae05579379787e2148a57fe93,RepoTags:[docker.io/library/nginx:latest],RepoDigests:[docker.io/library/nginx@sha256:2e6775f4300fc79b9d7fe6bb60c83b5fefe584258d9318ed408746789af48885],Size_:112003199,Uid:nil,Username:,} &Image{Id:68252d6b27f75722d51647ba63df3274bfe09a43019615a6a9ea46ae2c33b3e3,RepoTags:[gcr.io/google_containers/nonewprivs:1.2],RepoDigests:[gcr.io/google_containers/nonewprivs@sha256:a343c87a3611810956279be783986ab0771c460d3b7a967cccc64e66ccfb1820],Size_:5985195,Uid:nil,Username:,} &Image{Id:2338acd9adb909e828383ca25bb311941e9256e3e1093cae8634588067bd7209,RepoTags:[docker.io/library/httpd:2.4-alpine],RepoDigests:[docker.io/library/httpd@sha256:fd16f2add97536eb610c4d69e7ffab0b860b3f14a641c3f670e5984aac959e06],Size_:91097129,Uid:nil,Username:,} &Image{Id:a557d4a703aaef313034e1d8e8bfda455dc372e02cb5d2dfaeeeb53ee859616b,RepoTags:[],RepoDigests:[],Size_:2653153,Uid:nil,Username:,} &Image{Id:4e29d7ab9660c58fc02d72145a0d53d923029d631d28d0a4c29594bfa11e22e1,RepoTags:[gcr.io/google_containers/k8s-dns-kube-dns-amd64:1.14.7],RepoDigests:[gcr.io/google_containers/k8s-dns-kube-dns-amd64@sha256:f5bddc71efe905f4e4b96f3ca346414be6d733610c1525b98fff808f93966680],Size_:50534820,Uid:nil,Username:,} &Image{Id:18e364ac7fef79a6579299ebe81d98af9df9f6248af848a6231a583df7fc619e,RepoTags:[gcr.io/google_containers/k8s-dns-dnsmasq-nanny-amd64:1.14.7],RepoDigests:[gcr.io/google_containers/k8s-dns-dnsmasq-nanny-amd64@sha256:6cfb9f9c2756979013dbd3074e852c2d8ac99652570c5d17d152e0c0eb3321d6],Size_:41221814,Uid:nil,Username:,} &Image{Id:4036c0abd94efe51de6301b3c57aed1ea50211799f1d36f63d386c05846a4647,RepoTags:[gcr.io/google_containers/k8s-dns-sidecar-amd64:1.14.7],RepoDigests:[gcr.io/google_containers/k8s-dns-sidecar-amd64@sha256:f80f5f9328107dc516d67f7b70054354b9367d31d4946a3bffd3383d83d7efe8],Size_:42292834,Uid:nil,Username:nobody,} &Image{Id:9f4682667c615052e8584c3878766f0609724b8101d84ad08c34bed8414f7402,RepoTags:[],RepoDigests:[],Size_:2653153,Uid:nil,Username:,} &Image{Id:84ad8824135085872a4d98c22f5e886463f58f8fd63198235196cf223485c556,RepoTags:[docker.io/library/busybox:1.24],RepoDigests:[docker.io/library/busybox@sha256:3181b2fff8cbdefa2302a04b1eb2273251749e9320d0083b807d32e9f0e18961],Size_:1314823,Uid:nil,Username:,} &Image{Id:c1638070a7ea4061dc42c710292b530f51ef266fba1bdd3b9820f2ddd61fa79d,RepoTags:[],RepoDigests:[],Size_:2653153,Uid:nil,Username:,} &Image{Id:326cbbd22a6ab946674fac47ceb54e2a56e325adcb667399f9587edc996dfee2,RepoTags:[gcr.io/google_containers/busybox:latest],RepoDigests:[gcr.io/google_containers/busybox@sha256:6632a6043ee0735c8592e7c617b192687ee7154e603948d409b598a4ee865d1e],Size_:2653153,Uid:nil,Username:,} &Image{Id:6ad733544a6317992a6fac4eb19fe1df577d4dec7529efec28a5bd0edad0fd30,RepoTags:[],RepoDigests:[],Size_:1339880,Uid:nil,Username:,} &Image{Id:3fa822599e10c5f2080dcf647068c72022b111d31bbec0c5adb8a96e7eb5379b,RepoTags:[docker.io/library/centos:latest],RepoDigests:[docker.io/library/centos@sha256:3a32a170c945ffe18334b3f514fcb66f9c14001b2266c9ed8504c72db0acde11],Size_:212081496,Uid:nil,Username:,} &Image{Id:861cc310cd916417a1f84c0293c5ce3ea24c0c8d4c0e7a845cb6988ecf5a3241,RepoTags:[docker.io/library/redis:latest],RepoDigests:[docker.io/library/redis@sha256:0d9e5a4a3072948e1460a510bb07b2636d5c160ee84edb98daac8e779756f08d],Size_:110544076,Uid:nil,Username:,} &Image{Id:efb2a4d85bb46d42299e8270c823d2557332f896ae0bcbeadbc542a32c0bf064,RepoTags:[gcr.io/k8s-testimages/redis:e2e],RepoDigests:[gcr.io/k8s-testimages/redis@sha256:2c3f1112c32a23ad0f14a0a6ff8ac8006e23b5c27de89a4b5287467eb4844dad],Size_:6138512,Uid:nil,Username:,} &Image{Id:cb1ec54b370d4a91dff57d00f91fd880dc710160a58440adaa133e0f84ae999d,RepoTags:[docker.io/library/redis:alpine],RepoDigests:[docker.io/library/redis@sha256:28a852279e97b1d8683759a3f70b17b057d94b683447d3fe32be947d1cd04a1c],Size_:27414742,Uid:nil,Username:,} &Image{Id:80cc5ea4b547abe174d7550b82825ace40769e977cde90495df3427b3a0f4e75,RepoTags:[k8s.gcr.io/k8s-dns-kube-dns-amd64:1.14.8],RepoDigests:[k8s.gcr.io/k8s-dns-kube-dns-amd64@sha256:6d8e0da4fb46e9ea2034a3f4cab0e095618a2ead78720c12e791342738e5f85d],Size_:50717087,Uid:nil,Username:,} &Image{Id:c2ce1ffb51ed60c54057f53b8756231f5b4b792ce04113c6755339a1beb25943,RepoTags:[k8s.gcr.io/k8s-dns-dnsmasq-nanny-amd64:1.14.8],RepoDigests:[k8s.gcr.io/k8s-dns-dnsmasq-nanny-amd64@sha256:93c827f018cf3322f1ff2aa80324a0306048b0a69bc274e423071fb0d2d29d8b],Size_:41219759,Uid:nil,Username:,} &Image{Id:6f7f2dc7fab5d7e7f99dc4ac176683a981a9ff911d643b9f29ffa146838deda3,RepoTags:[k8s.gcr.io/k8s-dns-sidecar-amd64:1.14.8],RepoDigests:[k8s.gcr.io/k8s-dns-sidecar-amd64@sha256:23df717980b4aa08d2da6c4cfa327f1b730d92ec9cf740959d2d5911830d82fb],Size_:42471003,Uid:nil,Username:nobody,} &Image{Id:f6e427c148a766d2d6c117d67359a0aa7d133b5bc05830a7ff6e8b64ff6b1d1d,RepoTags:[docker.io/library/busybox:latest],RepoDigests:[docker.io/library/busybox@sha256:c7b0a24019b0e6eda714ec0fa137ad42bc44a754d9cea17d14fba3a80ccc1ee4],Size_:1362408,Uid:nil,Username:,}],}" 

@nalind @runcom

@mrunalp
Copy link
Member Author

mrunalp commented Mar 14, 2018

#1415 seems to have broken us. If I revert it then everything is fine.

mrunalp added 2 commits March 14, 2018 16:15
This version has an additional test for container log reopen.

Signed-off-by: Mrunal Patel <[email protected]>
@mrunalp
Copy link
Member Author

mrunalp commented Mar 14, 2018

Added a commit for the list image issue.

@mrunalp
Copy link
Member Author

mrunalp commented Mar 14, 2018

/test all

@mrunalp
Copy link
Member Author

mrunalp commented Mar 14, 2018

/test critest_rhel

@mrunalp
Copy link
Member Author

mrunalp commented Mar 15, 2018

@adelton this should fix the crictl images issue for you.

@mrunalp
Copy link
Member Author

mrunalp commented Mar 15, 2018

@rhatdan @runcom This is green!

@runcom runcom merged commit 9f58023 into cri-o:master Mar 15, 2018
@runcom
Copy link
Member

runcom commented Mar 15, 2018

LGTM

@adelton
Copy link
Contributor

adelton commented Mar 15, 2018

@adelton this should fix the crictl images issue for you.

Do you mean kubernetes-sigs/cri-tools#265? I confirm that with this change, I no longer see the <none> <none> records without IMAGE ID I talked about in kubernetes-sigs/cri-tools#268 (comment), if this is what this change addresses.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants