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

Switch vagrant test cluster runtime to use containerd directly #2583

Merged
merged 1 commit into from
Aug 18, 2021

Conversation

stanleywbwong
Copy link
Contributor

@stanleywbwong stanleywbwong commented Aug 12, 2021

Fixes #2545

Signed-off-by: Stan Wong swong394@gmail.com

Copy link
Contributor

@antoninbas antoninbas left a comment

Choose a reason for hiding this comment

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

Thanks for working on this. I left a few comments. I will test the PR after these are addressed.

@stanleywbwong stanleywbwong force-pushed the replace-test-runtime branch 2 times, most recently from 33fabc9 to 0397fd0 Compare August 12, 2021 20:51
Signed-off-by: Stan Wong <swong394@gmail.com>
Copy link
Contributor

@antoninbas antoninbas left a comment

Choose a reason for hiding this comment

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

LGTM

@antoninbas
Copy link
Contributor

/skip-all

@antoninbas
Copy link
Contributor

@tnqn @lzhecheng @srikartati and others, any objection to this change?

@codecov-commenter
Copy link

codecov-commenter commented Aug 17, 2021

Codecov Report

Merging #2583 (d3a2f11) into main (3248515) will increase coverage by 0.04%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #2583      +/-   ##
==========================================
+ Coverage   60.18%   60.22%   +0.04%     
==========================================
  Files         281      282       +1     
  Lines       22257    22411     +154     
==========================================
+ Hits        13396    13498     +102     
- Misses       7439     7486      +47     
- Partials     1422     1427       +5     
Flag Coverage Δ
kind-e2e-tests 47.10% <ø> (+0.02%) ⬆️
unit-tests 42.02% <ø> (-0.49%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
...ver/registry/controlplane/nodestatssummary/rest.go 50.00% <0.00%> (-50.00%) ⬇️
pkg/agent/controller/networkpolicy/packetin.go 70.83% <0.00%> (-1.85%) ⬇️
pkg/monitor/controller.go 27.61% <0.00%> (-1.50%) ⬇️
...gent/controller/noderoute/node_route_controller.go 57.60% <0.00%> (-1.30%) ⬇️
pkg/agent/openflow/client.go 57.79% <0.00%> (-1.29%) ⬇️
pkg/agent/openflow/pipeline.go 72.79% <0.00%> (-0.77%) ⬇️
pkg/ovs/openflow/ofctrl_bridge.go 49.65% <0.00%> (-0.35%) ⬇️
pkg/agent/agent.go 50.31% <0.00%> (-0.02%) ⬇️
pkg/ovs/ovsctl/interface.go 0.00% <0.00%> (ø)
...ntroller/networkpolicy/networkpolicy_controller.go 69.45% <0.00%> (ø)
... and 7 more

Copy link
Member

@srikartati srikartati left a comment

Choose a reason for hiding this comment

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

LGTM.
No objection. It seems docker runtime production support is removed in K8s v1.22.

Comment on lines +35 to +40
[plugins.scheduler]
pause_threshold = 0.02
deletion_threshold = 0
mutation_threshold = 100
schedule_delay = 0
startup_delay = "100ms"
Copy link
Member

Choose a reason for hiding this comment

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

Out of curiosity. How are these values picked? Is there any recommended configuartion?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Member

@tnqn tnqn left a comment

Choose a reason for hiding this comment

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

LGTM

@antoninbas
Copy link
Contributor

I tested the latest version locally and confirmed that it works

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Switch from docker to containerd for vagrant-based test cluster
5 participants