kubernetes / test-infra

Test infrastructure for the Kubernetes project.
Apache License 2.0
3.85k stars 2.66k forks source link

Enable --cri-proxy-enabled for CI jobs to that need it #33808

Closed dims closed 1 week ago

dims commented 1 week ago

See https://github.com/kubernetes/kubernetes/issues/128835 for context. Additional screen shot here as well.

image

k8s-ci-robot commented 1 week ago

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dims

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files: - ~~[config/jobs/kubernetes/sig-node/OWNERS](https://github.com/kubernetes/test-infra/blob/master/config/jobs/kubernetes/sig-node/OWNERS)~~ [dims] Approvers can indicate their approval by writing `/approve` in a comment Approvers can cancel approval by writing `/approve cancel` in a comment
dims commented 1 week ago

xref: https://github.com/kubernetes/kubernetes/pull/128559/files

cc @lauralorenz

dims commented 1 week ago

/assign @kannon92 @upodroid @ameukam

dims commented 1 week ago

14 CI jobs in total needed this flag:

ci-cgroupv1-containerd-node-arm64-e2e-serial-ec2-eks
ci-cgroupv1-containerd-node-e2e-serial-ec2-eks
ci-cgroupv2-containerd-node-al2023-e2e-serial-ec2-eks
ci-cgroupv2-containerd-node-arm64-al2023-e2e-serial-ec2-eks
ci-cgroupv2-containerd-node-arm64-e2e-serial-ec2
ci-cgroupv2-containerd-node-e2e-serial-ec2
ci-cos-cgroupv1-containerd-node-e2e-serial
ci-cos-cgroupv2-containerd-node-e2e-serial
ci-kubernetes-node-arm64-ubuntu-serial
ci-kubernetes-node-kubelet-cgroupv1-serial-cri-o
ci-kubernetes-node-kubelet-cgroupv2-serial-cri-o
ci-kubernetes-node-kubelet-serial-containerd
ci-kubernetes-node-swap-fedora-serial
ci-kubernetes-node-swap-ubuntu-serial
kannon92 commented 1 week ago

Maybe we should separate the CRI-PRoxy jobs from the normal jobs.

This seems a bit like a code smell where we have some jobs injecting proxy for CRI while the other tests are using the actual CRI?

cc @SergeyKanzhelev

dims commented 1 week ago

@kannon92 that can be a follow up. right now i am trying to get 🟢 signal from jobs quickly and we are past code/test freeze :(

kannon92 commented 1 week ago

@kannon92 that can be a follow up. right now i am trying to get 🟢 signal from jobs quickly and we are past code/test freeze :(

I would rather filter out this job and maybe evaluate how to run this test with injecting CRI Proxy without also impacting all the other jobs. I don't know this feature that well but we are injecting a mock CRI and also running tests with a real container runtime. Will this impact the actual container runtime?

dims commented 1 week ago

@kannon92 all of these are node-e2e jobs.

kannon92 commented 1 week ago

@kannon92 all of these are node-e2e jobs.

Yes. I am not sure what point you are bringing up here.

dims commented 1 week ago

@kannon92 i meant that there are no non-node e2e jobs (i wasn't sure if that's what you were concerned about....)

dims commented 1 week ago

i am happy to close this and wait for all of you to do something in k/k repo.

/close

k8s-ci-robot commented 1 week ago

@dims: Closed this PR.

In response to [this](https://github.com/kubernetes/test-infra/pull/33808#issuecomment-2483621759): >i am happy to close this and wait for all of you to do something in k/k repo. > >/close Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes-sigs/prow](https://github.com/kubernetes-sigs/prow/issues/new?title=Prow%20issue:) repository.