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

feat(cni): taint controller #4650

Merged
merged 41 commits into from
Aug 4, 2022
Merged

feat(cni): taint controller #4650

merged 41 commits into from
Aug 4, 2022

Conversation

slonka
Copy link
Contributor

@slonka slonka commented Jul 21, 2022 •
edited
Loading

Signed-off-by: slonka slonka@users.noreply.github.com

Summary

Implementing the taint controller, more info in the issue attached.

Issues resolved

Fix #4560

Documentation

  • Link to the website documentation PR - I'll probably write more docs (than just which values to use) once all of the CNI stuff is ready

Testing

  • Unit tests - done it only on the E2E level the way the faq suggest
  • E2E tests
    - [ ] Manual testing on Universal do not apply here
  • Manual testing on Kubernetes

Backwards compatibility

- [ ] Update UPGRADE.md with any steps users will need to take when upgrading. - backwards compatible
- [ ] Add backport-to-stable label if the code follows our backporting policy - not subject to backport policy

slonka added 2 commits July 21, 2022 16:48
@slonka
feat(cni): add delay to the cni startup and a failing test
4d096dd 
Signed-off-by: slonka <slonka@users.noreply.github.com>
@slonka
feat(cni): make check pass
86476d4 
Signed-off-by: slonka <slonka@users.noreply.github.com>
@codecov-commenter
Copy link

codecov-commenter commented Jul 21, 2022 •
edited
Loading

Codecov Report

Merging #4650 (55a9aa8) into master (feb77b1) will decrease coverage by 0.13%.
The diff coverage is 1.43%.

@@            Coverage Diff             @@
##           master    #4650      +/-   ##
==========================================
- Coverage   46.49%   46.36%   -0.14%     
==========================================
  Files         689      690       +1     
  Lines       46833    46969     +136     
==========================================
+ Hits        21775    21777       +2     
- Misses      23129    23264     +135     
+ Partials     1929     1928       -1
Impacted Files Coverage Δ
app/cni/pkg/install/main.go 6.89% <0.00%> (-0.41%) ⬇️
app/kumactl/cmd/install/install_control_plane.go 73.97% <0.00%> (ø)
pkg/config/app/kuma-cp/config.go 39.45% <ø> (ø)
...ns/runtime/k8s/controllers/cni_taint_controller.go 0.00% <0.00%> (ø)
...d/install/context/install_control_plane_context.go 95.91% <100.00%> (ø)
pkg/plugins/runtime/gateway/route/sorter.go 71.79% <0.00%> (+5.12%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@slonka slonka changed the title feat(cni): add delay to the cni startup and a failing test [WIP] feat(cni): add delay to the cni startup and a failing test Jul 22, 2022
@slonka slonka changed the title [WIP] feat(cni): add delay to the cni startup and a failing test [WIP] feat(cni): taint controller Jul 22, 2022
slonka added 9 commits July 24, 2022 23:48
@slonka
feat(cni): taint controller e2e test passes
ac24d98 
Signed-off-by: slonka <slonka@users.noreply.github.com>
@slonka
feat(cni): add post meet comments and use kuma version instead of a h…
47abf1a 
…ardcoded one

Signed-off-by: slonka <slonka@users.noreply.github.com>
@slonka
feat(cni): hook up new test into CI
8644d88 
Signed-off-by: slonka <slonka@users.noreply.github.com>
@slonka
feat(cni): make check pass
f4516ab 
Signed-off-by: slonka <slonka@users.noreply.github.com>
@slonka
feat(cni): update golden files
5ac9872 
Signed-off-by: slonka <slonka@users.noreply.github.com>
@slonka
feat(cni): make k3d makefile target use k3d
8939e78 
Signed-off-by: slonka <slonka@users.noreply.github.com>
@slonka
feat(cni): fix target name
c795ea2 
Signed-off-by: slonka <slonka@users.noreply.github.com>
@slonka
feat(cni): update golden files
6e4d858 
Signed-off-by: slonka <slonka@users.noreply.github.com>
@slonka
feat(cni): add test that shows old cni is susceptible to the race con…
012ff3e 
…dition

Signed-off-by: slonka <slonka@users.noreply.github.com>
lahabana
lahabana reviewed Jul 27, 2022
View reviewed changes
test/e2e_k3d_only/cni/e2e_suite_test.go Outdated Show resolved Hide resolved
test/framework/interface.go Outdated Show resolved Hide resolved
slonka added 2 commits July 27, 2022 10:37
@slonka
feat(cni): clean up
9e498b4 
Signed-off-by: slonka <slonka@users.noreply.github.com>
@slonka
feat(cni): remove separate target
6553ac5 
Signed-off-by: slonka <slonka@users.noreply.github.com>
@slonka slonka changed the title [WIP] feat(cni): taint controller feat(cni): taint controller Jul 27, 2022
slonka added 2 commits July 27, 2022 15:20
@slonka
feat(cni): self review fixes
b651262 
Signed-off-by: slonka <slonka@users.noreply.github.com>
@slonka
feat(cni): move kind not supported flag to circleci config
87611c6 
Signed-off-by: slonka <slonka@users.noreply.github.com>
@slonka slonka marked this pull request as ready for review July 27, 2022 14:11
@slonka slonka requested a review from a team as a code owner July 27, 2022 14:11
slonka added 2 commits July 28, 2022 11:02
@slonka
feat(cni): move create delete node to k8s implementation
cc6e2a6 
Signed-off-by: slonka <slonka@users.noreply.github.com>
@slonka
feat(cni): move filter label back to makefile
8866b3c 
Signed-off-by: slonka <slonka@users.noreply.github.com>
lahabana
lahabana reviewed Jul 28, 2022
View reviewed changes
Copy link
Contributor

@lahabana lahabana left a comment

Choose a reason for hiding this comment

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

I'll have another look tomorrow

app/cni/pkg/install/main.go Outdated Show resolved Hide resolved
app/kumactl/cmd/install/testdata/install-control-plane.cni-enabled.golden.yaml Outdated Show resolved Hide resolved
deployments/charts/kuma/README.md Outdated Show resolved Hide resolved
deployments/charts/kuma/templates/cni-daemonset.yaml Outdated Show resolved Hide resolved
mk/e2e.new.mk Show resolved Hide resolved
slonka and others added 3 commits July 29, 2022 12:22
@slonka @lahabana
Update app/cni/pkg/install/main.go
66ae209 
Co-authored-by: Charly Molter <charly@molter.io>
Signed-off-by: Krzysztof Słonka <slonka@users.noreply.github.com>
@slonka
feat(cni): apply review suggestions
1dfad5f 
Signed-off-by: slonka <slonka@users.noreply.github.com>
@slonka
chore: merge with master
0564256 
Signed-off-by: slonka <slonka@users.noreply.github.com>
@slonka
feat(cni): remove no longer needed comment
4cdedd9 
Signed-off-by: slonka <slonka@users.noreply.github.com>
lahabana
lahabana reviewed Aug 2, 2022
View reviewed changes
pkg/plugins/runtime/k8s/controllers/cni_taint_controller.go Outdated Show resolved Hide resolved
pkg/plugins/runtime/k8s/controllers/cni_taint_controller.go Outdated Show resolved Hide resolved
pkg/plugins/runtime/k8s/controllers/cni_taint_controller.go Outdated Show resolved Hide resolved
pkg/plugins/runtime/k8s/controllers/cni_taint_controller.go Outdated Show resolved Hide resolved
pkg/plugins/runtime/k8s/controllers/cni_taint_controller.go Outdated Show resolved Hide resolved
pkg/plugins/runtime/k8s/controllers/cni_taint_controller.go Show resolved Hide resolved
pkg/plugins/runtime/k8s/controllers/cni_taint_controller.go Outdated Show resolved Hide resolved
pkg/plugins/runtime/k8s/controllers/cni_taint_controller.go Outdated Show resolved Hide resolved
deployments/charts/kuma/templates/_helpers.tpl Show resolved Hide resolved
test/e2e/cni/e2e_suite_test.go Show resolved Hide resolved
@slonka
feat(cni): apply review suggestions
b58865f 
Signed-off-by: slonka <slonka@users.noreply.github.com>
slonka added 4 commits August 2, 2022 13:59
@slonka
feat(cni): remove if around sleep
ee78af6 
Signed-off-by: slonka <slonka@users.noreply.github.com>
@slonka
feat(cni): fix tests
f66140a 
Signed-off-by: slonka <slonka@users.noreply.github.com>
@slonka
feat(cni): shorten the waiting time
c35b5c6 
Signed-off-by: slonka <slonka@users.noreply.github.com>
@slonka
feat(cni): remove arm not supported label from new cni test
fec6dd5 
Signed-off-by: slonka <slonka@users.noreply.github.com>
slonka added 6 commits August 3, 2022 11:22
@slonka
fix(cni): tmp
30bb406 
Signed-off-by: slonka <slonka@users.noreply.github.com>
@slonka
feat(cni): apply review suggestions
7990c47 
Signed-off-by: slonka <slonka@users.noreply.github.com>
@slonka
chore: merge with master
67fbd7a 
Signed-off-by: slonka <slonka@users.noreply.github.com>
@slonka
feat(cni): update golden files
a1ac880 
Signed-off-by: slonka <slonka@users.noreply.github.com>
@slonka
fix(cni): make check pass
7e1259b 
Signed-off-by: slonka <slonka@users.noreply.github.com>
@slonka
fix(cni): remove filtering of nodes
63c05ee 
Signed-off-by: slonka <slonka@users.noreply.github.com>
@slonka
chore: fix bad merge
55a9aa8 
Signed-off-by: slonka <slonka@users.noreply.github.com>
@slonka
fix(cni): check that node name exists for both old and new object
b9c1cf4 
Signed-off-by: slonka <slonka@users.noreply.github.com>
slonka added 2 commits August 4, 2022 13:31
@slonka
Revert "fix(cni): check that node name exists for both old and new ob…
8dad501 
…ject"

This reverts commit b9c1cf4.

Signed-off-by: slonka <slonka@users.noreply.github.com>
@slonka
fix(cni): udpate comment
e801dc1 
Signed-off-by: slonka <slonka@users.noreply.github.com>
@slonka slonka force-pushed the feat/add-cni-taint-controller branch from 883fed8 to e801dc1 Compare August 4, 2022 11:31
slonka added 2 commits August 4, 2022 13:33
@slonka
fix(cni): change log level
f6c9a77 
Signed-off-by: slonka <slonka@users.noreply.github.com>
@slonka
fix(cni): put pod filtering in mapper because it is more performant
92cacba 
Signed-off-by: slonka <slonka@users.noreply.github.com>
@slonka slonka merged commit db95c0e into kumahq:master Aug 4, 2022
@slonka slonka deleted the feat/add-cni-taint-controller branch August 4, 2022 12:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lukidzi lukidzi lukidzi left review comments

@lahabana lahabana lahabana left review comments

@michaelbeaumont michaelbeaumont michaelbeaumont approved these changes

@lobkovilya lobkovilya Awaiting requested review from lobkovilya

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Guard against possible race condition in new CNI
6 participants
@slonka @codecov-commenter @lahabana @michaelbeaumont @lobkovilya @lukidzi