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(kuma-cni): add a init container to validate that iptables rules are applied #9699

Merged
merged 15 commits into from
Apr 9, 2024
Merged

feat(kuma-cni): add a init container to validate that iptables rules are applied #9699

merged 15 commits into from
Apr 9, 2024

Conversation

jijiechen
Copy link
Member

@jijiechen jijiechen commented Mar 25, 2024 •
edited by lahabana
Loading

fixes #9431

Like what Istio does, I'm trying to introduce a container named "kuma-cni-validation" to validate the iptables rules.

Such a container can also mitegate the issue of "IP address assignment hang" during the postStart check.

Checklist prior to review

@jijiechen jijiechen added the ci/run-full-matrix PR: Runs all possible e2e test combination (expensive use carefully) label Mar 25, 2024
@jijiechen jijiechen requested a review from a team as a code owner March 25, 2024 09:27
@jijiechen jijiechen requested review from slonka, jakubdyszkiewicz and lobkovilya and removed request for a team March 25, 2024 09:27
@jijiechen jijiechen marked this pull request as draft March 25, 2024 09:29
@jijiechen jijiechen marked this pull request as ready for review March 26, 2024 10:14
@jijiechen jijiechen force-pushed the kuma-cni-validation branch 2 times, most recently from 48acac6 to b359972 Compare March 27, 2024 03:11
@jijiechen jijiechen force-pushed the kuma-cni-validation branch from b359972 to 32f9e13 Compare March 27, 2024 05:48
@jijiechen jijiechen force-pushed the kuma-cni-validation branch from 45748b7 to 518dd54 Compare March 27, 2024 09:55
slonka
slonka reviewed Apr 2, 2024
View reviewed changes
Copy link
Contributor

@slonka slonka left a comment

Choose a reason for hiding this comment

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

A bunch of questions. Might need another round here.

app/kumactl/cmd/install/install_transparent_proxy_validator.go Outdated Show resolved Hide resolved
app/kumactl/cmd/install/install_transparent_proxy_validator.go Show resolved Hide resolved
app/kumactl/cmd/install/install_transparent_proxy_validator.go Outdated Show resolved Hide resolved
app/kumactl/cmd/install/install_transparent_proxy_validator.go Outdated Show resolved Hide resolved
app/kumactl/cmd/install/install_transparent_proxy_validator.go Show resolved Hide resolved
pkg/transparentproxy/validate/validator.go Outdated Show resolved Hide resolved
pkg/transparentproxy/validate/validator.go Outdated Show resolved Hide resolved
pkg/transparentproxy/validate/validator.go Outdated Show resolved Hide resolved
pkg/transparentproxy/validate/validator.go Outdated Show resolved Hide resolved
pkg/transparentproxy/validate/validator_test.go Outdated Show resolved Hide resolved
@jijiechen jijiechen requested a review from slonka April 8, 2024 06:50
@jijiechen
Copy link
Member Author

Hi @slonka I've updated the code according to the discussions and your suggestions, some threads are left open and waiting for your further response.
Could you review them at your convenience?

@slonka
Copy link
Contributor

slonka commented Apr 8, 2024

@jijiechen - answered, just 2 small things and we can 👍

jijiechen and others added 2 commits April 8, 2024 16:35
@jijiechen @slonka
Apply suggestions from code review
97063a8 
Co-authored-by: Krzysztof Słonka <[email protected]>
Signed-off-by: Jay Jijie Chen <[email protected]>
@jijiechen
feat(kuma-cni): fix lint errors
9437215 
Signed-off-by: Jay Chen <[email protected]>
@jijiechen jijiechen merged commit 75b99bc into kumahq:master Apr 9, 2024
35 checks passed
@lahabana lahabana changed the title feat(kuma-cni): add a init container to validate iptables rules are applied feat(kuma-cni): add a init container to validate that iptables rules are applied Apr 11, 2024
@BrewTestBot BrewTestBot mentioned this pull request Apr 19, 2024
Merged
@jijiechen jijiechen mentioned this pull request Nov 29, 2024
Merged
lahabana pushed a commit that referenced this pull request Dec 6, 2024
@jijiechen @bartsmykla
docs(maintainer): maintainer nomination (#12137)
35f2d6b 
## Motivation

<!-- Why are we doing this change -->

Add Jay Jijie Chen as Kuma maintainer.

### Nominee's first and last name

Jay Chen

### Nominee's email address and GitHub user name

Email: [email protected]
Github: jijiechen

### An explanation of why the nominee should be a maintainer/reviewer

I wish to become a Kuma project maintainer to better engage in the
discussions, contributions, code reviews for substantial time.

Here are my significant contribution PRs and Issues for this project:
- [ci(actions): add the 'build-test-distribute' GitHub
Action](https://github.com/kumahq/kuma/pull/8360/files)
- [feat(transparent-proxy): deprecate argument
'redirect-inbound-port-v6' and introduce 'ip-family-mode'
](#8939)
- [feat(kuma-cni): add a init container to validate that iptables rules
are applied](#9699)
- [feat(kuma-dp): rework on the virtual probes to support probing tcp
and grpc ports](#10624)
- [test(framework): inspect clusters, stats and policies when DebugKube
on E2E failures](#11746)

## Implementation information

<!-- Explain how this was done and potentially alternatives considered
and discarded -->

## Supporting documentation

<!-- Is there a MADR? An Issue? A related PR? -->

<!--
> Changelog: skip
-->
<!--
Uncomment the above section to explicitly set a [`> Changelog:` entry
here](https://github.com/kumahq/kuma/blob/master/CONTRIBUTING.md#submitting-a-patch)?
-->

Signed-off-by: Jay Chen <[email protected]>
Signed-off-by: Bart Smykla <[email protected]>
Co-authored-by: Bart Smykla <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@slonka slonka slonka approved these changes

@jakubdyszkiewicz jakubdyszkiewicz Awaiting requested review from jakubdyszkiewicz jakubdyszkiewicz is a code owner automatically assigned from kumahq/kuma-maintainers

@lobkovilya lobkovilya Awaiting requested review from lobkovilya lobkovilya is a code owner automatically assigned from kumahq/kuma-maintainers

@michaelbeaumont michaelbeaumont Awaiting requested review from michaelbeaumont

@bartsmykla bartsmykla Awaiting requested review from bartsmykla

Assignees
No one assigned
Labels
ci/run-full-matrix PR: Runs all possible e2e test combination (expensive use carefully)
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Pod does not start when turnning on waitForDataplaneReady with kuma-cni enabled
2 participants
@jijiechen @slonka