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

Run acl test twice - with and without flow logging #1010

Merged
merged 3 commits into from
Aug 15, 2024

Conversation

milan-zededa
Copy link
Contributor

Soon EVE will allow to disable flow logging (and it will become the default behavior),
which reduces the amount of data published to the controller and simplifies iptables
rules installed by EVE.
See: lf-edge/eve-api#62

However, the ACL implementation is quite different between enabled and
disabled flow logging. Therefore, it makes sense to run the ACL test twice
to test both cases.

Signed-off-by: Milan Lenco <milan@zededa.com>
Signed-off-by: Milan Lenco <milan@zededa.com>
Soon EVE will allow to disable flow logging, which reduces the amount of
data published to the controller and simplifies iptables rules installed
by EVE.
See: lf-edge/eve-api#62

However, the ACL implementation is quite different between enabled and
disabled flow logging. Therefore, it makes sense to run the ACL test
twice to test both cases.

Signed-off-by: Milan Lenco <milan@zededa.com>
Copy link
Member

@uncleDecart uncleDecart left a comment

Choose a reason for hiding this comment

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

LGTM, do we need to bump EVE version as well, since we're updating API?

Edit: Okay, we can merge once EVE is released and then we will have to release new Eden

@milan-zededa
Copy link
Contributor Author

LGTM, do we need to bump EVE version as well, since we're updating API?

Edit: Okay, we can merge once EVE is released and then we will have to release new Eden

No need, test passes also for older EVE.
Actually, I would prefer to first merge this, release eden and bring this to EVE. Otherwise, my next EVE PR will fail networking tests.

@milan-zededa milan-zededa merged commit d70c4ea into lf-edge:master Aug 15, 2024
17 of 19 checks passed
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.

3 participants