WIP: Bug 2062899 #5723
Closed
WIP: Bug 2062899 #5723
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
There's no reason for us to keep maintaining our own copy of typeurl, let's directly rely on the `github.com/containerd/typeurl` instead. Signed-off-by: Fabiano Fidêncio <fidencio@redhat.com>
As this function is only used by runtimeVM::ContainerStats(), let's move it to the runtime_vm.go file, making our life easier when doing upcoming changes on runtimeVM::ContainerStats(). Signed-off-by: Fabiano Fidêncio <fidencio@redhat.com>
CRI-O has been using `containers/libpod/pkg/cgroups` in order to get metrics and, later on, convert it to CRI Stats. Although this approach is fine (and desired) for the OCI runtime type. we can't rely on that for the VM runtime type as the data sent by Kata Containers comes from `containerd/cgroup`. Signed-off-by: Fabiano Fidêncio <fidencio@redhat.com>
WorkingSetBytes is the bit that needs to be set in order to provide
kubelet the pod's memory information.
Although it's calculated in a slightly different way for "oci" runtime
type, the logic is quite similar for the "vm" runtime type, with the
only difference being where the TotalInactiveFile information comes
from.
This is the last bit needed in order to have `kubectl top pod $pod`
working, as shown below:
```
[fidencio@localhost cri-o]$ kubectl get pods
NAME READY STATUS RESTARTS AGE
example-fedora 1/1 Running 0 130m
[fidencio@localhost cri-o]$ kubectl get pod example-fedora -o yaml | grep runtimeClassName
{"apiVersion":"v1","kind":"Pod","metadata":{"annotations":{},"labels":{"app":"example-fedora-app"},"name":"example-fedora","namespace":"default"},"spec":{"containers":[{"args":["-m","http.server","8080"],"command":["python3"],"image":"fedora:33","name":"example-fedora","ports":[{"containerPort":8080}]}],"runtimeClassName":"kata"}}
f:runtimeClassName: {}
runtimeClassName: kata
[fidencio@localhost cri-o]$ kubectl top pod
NAME CPU(cores) MEMORY(bytes)
example-fedora 1m 9Mi
```
Signed-off-by: Fabiano Fidêncio <fidencio@redhat.com>
Fix Bug 1942608, during the call of ListImages, do not list the image if the there's no manifest exist. Skip reporting the error locating manifest if the imaage is in the ImageBeingPulled list. Signed-off-by: Qi Wang <qiwan@redhat.com>
…pick-4470-to-release-1.21 [release-1.21] Properly implement metrics for Kata Containers when using CRI stats.
…pick-4726-to-release-1.21 [release-1.21] Bug 1942608: do not list the image with error locating manifest
Signed-off-by: Peter Hunt <pehunt@redhat.com>
If we fail to initially attach the namespaces, we need to cleanup the ones we've already created Also, we are currently unconditionally recreating the namespaces when restoring. I believe that's from a rebase issue Signed-off-by: Peter Hunt <pehunt@redhat.com>
Signed-off-by: Peter Hunt <pehunt@redhat.com>
[1.21] bump to containers/image 5.11.1
…pick-4758-to-release-1.21 [release-1.21] nsmgr: fix some leaks with GetNamespace
Signed-off-by: Peter Hunt <pehunt@redhat.com>
…pick-4796-to-release-1.21 [release-1.21] container server: fix silly typo
This is a partial revert of 975ffc6, where we started using the internal log when the context is avaiable. It turns out that we missed a few pieces during the review, where we end up using the internal log without having the interceptors set up. This was raised by @haircommander during the review, we checked a few parts of the patch, but we still missed a few others. We can only use the internal logging after https://github.com/cri-o/cri-o/blob/d3dbaec060e33870e5cb5c3f7ec4207837804b00/cmd/crio/main.go#L222 happens. From this moment, we start having different ways of logging in the very same file and consistency becomes a problem then. With the consistency in mind, I'd like to **only** use logrus for debugging as part of this file. Note: This was noticed when debugging cri-o#4798 Signed-off-by: Fabiano Fidêncio <fidencio@redhat.com>
as well as mark the feature as experimental Signed-off-by: Peter Hunt <pehunt@redhat.com>
Signed-off-by: Peter Hunt <pehunt@redhat.com>
…pick-4787-to-release-1.21 [release-1.21] workloads: update how overrides are specified
We now partially revert cri-o#4650 to still use the runtime internal context for its tasks. Fixes: cri-o#4798 Signed-off-by: Sascha Grunert <sgrunert@redhat.com>
…pick-4803-to-release-1.21 [release-1.21] Use extra context for runtime VM
…pick-4804-to-release-1.21 [release-1.21] main: still rely on logrus (rather than using the internal log)
This allows us to predictably shut down the node and help ensure clean shutdown. Signed-off-by: Mrunal Patel <mrunalp@gmail.com>
The internal seccomp profile (`RuntimeDefault`) should be ignored in the same way as it was before using the new field. This aligns the implementation with CRI-O releases before v1.21.0. Signed-off-by: Sascha Grunert <sgrunert@redhat.com>
Signed-off-by: Mrunal Patel <mrunalp@gmail.com>
…pick-4789-to-release-1.21 [release-1.21] Fix RuntimeDefault seccomp behavior if disabled
…pick-4766-to-release-1.21 [release-1.21] Add After=crio.service dependency to containers and conmon
[1.21] Fix podman name in README
Signed-off-by: Peter Hunt <pehunt@redhat.com>
Signed-off-by: Peter Hunt <pehunt@redhat.com>
…pick-4828-to-release-1.21 [release-1.21] Pin gocapability to v0.0.0-20180916011248-d98352740cb2
Signed-off-by: Peter Hunt <pehunt@redhat.com>
Signed-off-by: Peter Hunt <pehunt@redhat.com>
[1.21] prepend commands with taskset if InfraCtrCPUSet is configured
…pick-5503-to-release-1.21 [release-1.21] oci: always reap conmon zombies
previously, CRI-O assumed that the kubelet wouldn't bump the attempt number if it returned an error after a series of retries. in reality, the kubelet *is* doing that, which means situations where we have lots of load on a node means the issue is made worse by this workaround, not better. Mitigate this by waiting until the timeout is completed before returning the fact that the resource is ready. This will cause the kubelet to continue to believe the timing out has happened, and retry with the same attempt number. Signed-off-by: Peter Hunt <pehunt@redhat.com>
…pick-5602-to-release-1.21 [release-1.21]server: fix race with kubelet
Signed-off-by: Peter Hunt <pehunt@redhat.com>
Signed-off-by: Peter Hunt <pehunt@redhat.com>
[1.21] server: filter sysctls passed through CRI if host namespace
Signed-off-by: Peter Hunt <pehunt@redhat.com>
bump to 1.21.5
Signed-off-by: Harshal Patil <harpatil@redhat.com>
openshift-ci
bot
added
the
do-not-merge/work-in-progress
|
@harche: Adding the "do-not-merge/release-note-label-needed" label because no release-note block was detected, please follow our release note process to remove it. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
openshift-ci
bot
added
do-not-merge/release-note-label-needed
|
@harche: PR needs rebase. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
|
Thanks for your pull request. Before we can look at it, you'll need to add a 'DCO signoff' to your commits. 📝 Please follow instructions in the contributing guide to update your commits with the DCO Full details of the Developer Certificate of Origin can be found at developercertificate.org. The list of commits missing DCO signoff:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here. |
|
/close |
|
@harche: Closed this PR. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: harche The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
@harche: The following test failed, say
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here. |
|
@harche: The following tests failed, say
Full PR test history. Your PR dashboard. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What type of PR is this?
What this PR does / why we need it:
Which issue(s) this PR fixes:
Special notes for your reviewer:
Does this PR introduce a user-facing change?