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

loader : Log upsert and remove route errors #15339

Merged
merged 1 commit into from
Mar 29, 2021

Conversation

h3llix
Copy link
Contributor

@h3llix h3llix commented Mar 14, 2021

Fixes : #15282

Signed-off-by: Gaurav Genani h3llix.pvt@gmail.com

@h3llix h3llix requested a review from a team March 14, 2021 11:45
@maintainer-s-little-helper

This comment has been minimized.

@maintainer-s-little-helper maintainer-s-little-helper bot added dont-merge/needs-sign-off The author needs to add signoff to their commits before merge. dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. labels Mar 14, 2021
@maintainer-s-little-helper maintainer-s-little-helper bot added this to In progress in 1.10.0 Mar 14, 2021
@h3llix h3llix requested a review from jrfastab March 14, 2021 11:46
@maintainer-s-little-helper maintainer-s-little-helper bot removed the dont-merge/needs-sign-off The author needs to add signoff to their commits before merge. label Mar 14, 2021
@pchaigno pchaigno self-requested a review March 14, 2021 13:47
@pchaigno pchaigno added the release-note/misc This PR makes changes that have no direct user impact. label Mar 14, 2021
@maintainer-s-little-helper maintainer-s-little-helper bot removed the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label Mar 14, 2021
@pchaigno pchaigno added the sig/datapath Impacts bpf/ or low-level forwarding details, including map management and monitor messages. label Mar 14, 2021
Copy link
Member

@pchaigno pchaigno 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 your contribution! 🚀

pkg/datapath/loader/loader.go Outdated Show resolved Hide resolved
pkg/datapath/loader/loader.go Outdated Show resolved Hide resolved
@h3llix h3llix requested a review from pchaigno March 15, 2021 09:40
@pchaigno pchaigno removed their assignment Mar 15, 2021
@h3llix
Copy link
Contributor Author

h3llix commented Mar 15, 2021

@pchaigno Thank You for the help .

@pchaigno pchaigno added the gsoc Potential GSoC project or pull request related to GSoC label Mar 15, 2021
Copy link
Member

@christarazi christarazi left a comment

Choose a reason for hiding this comment

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

Changes LGTM, please squash the commits together

Fixes : cilium#15282
Signed-off-by: Gaurav Genani <h3llix.pvt@gmail.com>
@pchaigno
Copy link
Member

test-me-please

if ep.RequireEndpointRoute() {
upsertEndpointRoute(ep, *ip.IPNet(32))
if err := upsertEndpointRoute(ep, *ip.IPNet(32)); err != nil {
scopedLog.WithError(err).Warn("Failed to upsert route")
Copy link
Member

Choose a reason for hiding this comment

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

@pchaigno these are warnings, which steps can users perform if they see these?

Copy link
Member

Choose a reason for hiding this comment

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

Same as for other similar warnings (e.g., warning when we fail to load the BPF program for an endpoint): they should investigate and/or report the warning, but it's probably not blocking/fatal for the Cilium agent.

@nathanjsweet
Copy link
Member

#14608 #15128 #14948 #15425 #15499 all known or seen-before flakes.

@nathanjsweet nathanjsweet merged commit 32284cd into cilium:master Mar 29, 2021
1.10.0 automation moved this from In progress to Done Mar 29, 2021
aanm pushed a commit that referenced this pull request Mar 31, 2021
This reverts commit 32284cd.

It seems like PR #15339 introduced a regression on Kernel 4.9 with the
two following tests failing:

- K8sDatapathConfig Encapsulation Check vxlan connectivity with per-endpoint routes
- K8sDatapathConfig Host firewall With VXLAN and endpoint routes

Signed-off-by: Nicolas Busseneau <nicolas@isovalent.com>
@pchaigno
Copy link
Member

#14608 #15128 #14948 #15425 #15499 all known or seen-before flakes.

@nathanjsweet One of those failures was legit and the changes ended up breaking master. I think in general we should avoid using closed flake issues as a validation that something is a flake.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
gsoc Potential GSoC project or pull request related to GSoC release-note/misc This PR makes changes that have no direct user impact. sig/datapath Impacts bpf/ or low-level forwarding details, including map management and monitor messages.
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

Log errors on upsert/delete of endpoint routes
6 participants