-
Notifications
You must be signed in to change notification settings - Fork 2.8k
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
Removing disabled maps causes controllers to fail #15141
Comments
Thanks @aanm for reporting this. I will take a look and wire a fix accordingly. |
@brb If Line 402 in a9374b5
cilium/pkg/maps/ctmap/ctmap.go Line 168 in 29c7f21
|
Thanks, @pchaigno for taking care of this. I was looking at it this morning but you were faster than me 😝 |
@pchaigno Yep. |
Hit this in #15213 test runs, I think: https://jenkins.cilium.io/job/Cilium-PR-K8s-1.20-kernel-4.9/861/
|
The IPv4 and IPv6 SNAT maps are only used if BPF NodePort is enabled. Commit cac5218 ("datapath: remove SNAT maps entries when kube-proxy is enabled") removed the maps on startup if BPF NodePort is disabled. We were however still creating them regardless of the BPF NodePort status. The creation started a controller which then fails once the actual map is removed. This commit fixes the issue by not creating the userspace object, including the controller, for the SNAT maps when BPF NodePort is disabled. Fixes: cac5218 ("datapath: remove SNAT maps entries when kube-proxy is enabled") Fixes: cilium#15141 Signed-off-by: Paul Chaignon <paul@cilium.io>
The IPv4 and IPv6 SNAT maps are only used if BPF NodePort is enabled. Commit cac5218 ("datapath: remove SNAT maps entries when kube-proxy is enabled") removed the maps on startup if BPF NodePort is disabled. We were however still creating them regardless of the BPF NodePort status. The creation started a controller which then fails once the actual map is removed. This commit fixes the issue by not creating the userspace object, including the controller, for the SNAT maps when BPF NodePort is disabled. Fixes: cac5218 ("datapath: remove SNAT maps entries when kube-proxy is enabled") Fixes: #15141 Signed-off-by: Paul Chaignon <paul@cilium.io>
[ upstream commit 602e5ce ] The IPv4 and IPv6 SNAT maps are only used if BPF NodePort is enabled. Commit cac5218 ("datapath: remove SNAT maps entries when kube-proxy is enabled") removed the maps on startup if BPF NodePort is disabled. We were however still creating them regardless of the BPF NodePort status. The creation started a controller which then fails once the actual map is removed. This commit fixes the issue by not creating the userspace object, including the controller, for the SNAT maps when BPF NodePort is disabled. Fixes: cac5218 ("datapath: remove SNAT maps entries when kube-proxy is enabled") Fixes: cilium#15141 Signed-off-by: Paul Chaignon <paul@cilium.io>
[ upstream commit 602e5ce ] The IPv4 and IPv6 SNAT maps are only used if BPF NodePort is enabled. Commit cac5218 ("datapath: remove SNAT maps entries when kube-proxy is enabled") removed the maps on startup if BPF NodePort is disabled. We were however still creating them regardless of the BPF NodePort status. The creation started a controller which then fails once the actual map is removed. This commit fixes the issue by not creating the userspace object, including the controller, for the SNAT maps when BPF NodePort is disabled. Fixes: cac5218 ("datapath: remove SNAT maps entries when kube-proxy is enabled") Fixes: cilium#15141 Signed-off-by: Paul Chaignon <paul@cilium.io>
[ upstream commit 602e5ce ] The IPv4 and IPv6 SNAT maps are only used if BPF NodePort is enabled. Commit cac5218 ("datapath: remove SNAT maps entries when kube-proxy is enabled") removed the maps on startup if BPF NodePort is disabled. We were however still creating them regardless of the BPF NodePort status. The creation started a controller which then fails once the actual map is removed. This commit fixes the issue by not creating the userspace object, including the controller, for the SNAT maps when BPF NodePort is disabled. Fixes: cac5218 ("datapath: remove SNAT maps entries when kube-proxy is enabled") Fixes: #15141 Signed-off-by: Paul Chaignon <paul@cilium.io>
[ upstream commit 602e5ce ] The IPv4 and IPv6 SNAT maps are only used if BPF NodePort is enabled. Commit cac5218 ("datapath: remove SNAT maps entries when kube-proxy is enabled") removed the maps on startup if BPF NodePort is disabled. We were however still creating them regardless of the BPF NodePort status. The creation started a controller which then fails once the actual map is removed. This commit fixes the issue by not creating the userspace object, including the controller, for the SNAT maps when BPF NodePort is disabled. Fixes: cac5218 ("datapath: remove SNAT maps entries when kube-proxy is enabled") Fixes: #15141 Signed-off-by: Paul Chaignon <paul@cilium.io>
PR #14721 introduced changes that removed the cilium_snat_v{4,6}_external maps if NodePort is not enabled. Issue #15141 was attributed to the above PR, where controllers were failing with: > error=Unable to get object /sys/fs/bpf/tc/globals/cilium_snat_v4_external: no such file or directory name=bpf-map-sync-cilium_snat_v4_external In an attempt to fix #15141, #15175 added a nodeport argument to InitMapInfo() so that the cilium_snat_v{4,6}_external maps are not created if NodePort is not enabled. PR #15141 did not fix the issue: #15337 (comment) This PR takes another shot at fixing #15141 by removing a call of InitMapInfo from init(), where the (new) nodeport argument is always true. Not that in init(), option.Config.EnableNodePort is not properly set yet, so we cannot pass the config option because it would always be false. For this change to properly work, this patch also adds explicit InitMapInfo() calls since removing init() means that this function is called in contexts such as tests and the cli. Fixes: cac5218 Fixes: d639905 Fixes: #15141 Fixes: #15337 Signed-off-by: Kornilios Kourtis <kornilios@isovalent.com>
PR #14721 introduced changes that removed the cilium_snat_v{4,6}_external maps if NodePort is not enabled. Issue #15141 was attributed to the above PR, where controllers were failing with: > error=Unable to get object /sys/fs/bpf/tc/globals/cilium_snat_v4_external: no such file or directory name=bpf-map-sync-cilium_snat_v4_external In an attempt to fix #15141, #15175 added a nodeport argument to InitMapInfo() so that the cilium_snat_v{4,6}_external maps are not created if NodePort is not enabled. PR #15141 did not fix the issue: #15337 (comment) This PR takes another shot at fixing #15141 by removing a call of InitMapInfo from init(), where the (new) nodeport argument is always true. Not that in init(), option.Config.EnableNodePort is not properly set yet, so we cannot pass the config option because it would always be false. For this change to properly work, this patch also adds explicit InitMapInfo() calls since removing init() means that this function is called in contexts such as tests and the cli. Fixes: cac5218 Fixes: d639905 Fixes: #15141 Fixes: #15337 Signed-off-by: Kornilios Kourtis <kornilios@isovalent.com>
[ upstream commit e83dd53 ] PR #14721 introduced changes that removed the cilium_snat_v{4,6}_external maps if NodePort is not enabled. Issue #15141 was attributed to the above PR, where controllers were failing with: > error=Unable to get object /sys/fs/bpf/tc/globals/cilium_snat_v4_external: no such file or directory name=bpf-map-sync-cilium_snat_v4_external In an attempt to fix #15141, #15175 added a nodeport argument to InitMapInfo() so that the cilium_snat_v{4,6}_external maps are not created if NodePort is not enabled. PR #15141 did not fix the issue: #15337 (comment) This PR takes another shot at fixing #15141 by removing a call of InitMapInfo from init(), where the (new) nodeport argument is always true. Not that in init(), option.Config.EnableNodePort is not properly set yet, so we cannot pass the config option because it would always be false. For this change to properly work, this patch also adds explicit InitMapInfo() calls since removing init() means that this function is called in contexts such as tests and the cli. Fixes: cac5218 Fixes: d639905 Fixes: #15141 Fixes: #15337 Signed-off-by: Kornilios Kourtis <kornilios@isovalent.com> Signed-off-by: Gilberto Bertin <gilberto@isovalent.com>
[ upstream commit e83dd53 ] PR #14721 introduced changes that removed the cilium_snat_v{4,6}_external maps if NodePort is not enabled. Issue #15141 was attributed to the above PR, where controllers were failing with: > error=Unable to get object /sys/fs/bpf/tc/globals/cilium_snat_v4_external: no such file or directory name=bpf-map-sync-cilium_snat_v4_external In an attempt to fix #15141, #15175 added a nodeport argument to InitMapInfo() so that the cilium_snat_v{4,6}_external maps are not created if NodePort is not enabled. PR #15141 did not fix the issue: #15337 (comment) This PR takes another shot at fixing #15141 by removing a call of InitMapInfo from init(), where the (new) nodeport argument is always true. Not that in init(), option.Config.EnableNodePort is not properly set yet, so we cannot pass the config option because it would always be false. For this change to properly work, this patch also adds explicit InitMapInfo() calls since removing init() means that this function is called in contexts such as tests and the cli. Fixes: cac5218 Fixes: d639905 Fixes: #15141 Fixes: #15337 Signed-off-by: Kornilios Kourtis <kornilios@isovalent.com> Signed-off-by: Gilberto Bertin <gilberto@isovalent.com>
[ upstream commit e83dd53 ] PR cilium#14721 introduced changes that removed the cilium_snat_v{4,6}_external maps if NodePort is not enabled. Issue cilium#15141 was attributed to the above PR, where controllers were failing with: > error=Unable to get object /sys/fs/bpf/tc/globals/cilium_snat_v4_external: no such file or directory name=bpf-map-sync-cilium_snat_v4_external In an attempt to fix cilium#15141, cilium#15175 added a nodeport argument to InitMapInfo() so that the cilium_snat_v{4,6}_external maps are not created if NodePort is not enabled. PR cilium#15141 did not fix the issue: cilium#15337 (comment) This PR takes another shot at fixing cilium#15141 by removing a call of InitMapInfo from init(), where the (new) nodeport argument is always true. Not that in init(), option.Config.EnableNodePort is not properly set yet, so we cannot pass the config option because it would always be false. For this change to properly work, this patch also adds explicit InitMapInfo() calls since removing init() means that this function is called in contexts such as tests and the cli. Fixes: cac5218 Fixes: d639905 Fixes: cilium#15141 Fixes: cilium#15337 Signed-off-by: Kornilios Kourtis <kornilios@isovalent.com> Signed-off-by: Chris Tarazi <chris@isovalent.com>
[ upstream commit e83dd53 ] PR #14721 introduced changes that removed the cilium_snat_v{4,6}_external maps if NodePort is not enabled. Issue #15141 was attributed to the above PR, where controllers were failing with: > error=Unable to get object /sys/fs/bpf/tc/globals/cilium_snat_v4_external: no such file or directory name=bpf-map-sync-cilium_snat_v4_external In an attempt to fix #15141, #15175 added a nodeport argument to InitMapInfo() so that the cilium_snat_v{4,6}_external maps are not created if NodePort is not enabled. PR #15141 did not fix the issue: #15337 (comment) This PR takes another shot at fixing #15141 by removing a call of InitMapInfo from init(), where the (new) nodeport argument is always true. Not that in init(), option.Config.EnableNodePort is not properly set yet, so we cannot pass the config option because it would always be false. For this change to properly work, this patch also adds explicit InitMapInfo() calls since removing init() means that this function is called in contexts such as tests and the cli. Fixes: cac5218 Fixes: d639905 Fixes: #15141 Fixes: #15337 Signed-off-by: Kornilios Kourtis <kornilios@isovalent.com> Signed-off-by: Chris Tarazi <chris@isovalent.com>
Removing disabled maps in
cilium/pkg/datapath/maps/map.go
Lines 210 to 215 in cac5218
Causes the controllers to fail:
as seen in our CI https://jenkins.cilium.io/job/Cilium-PR-K8s-1.20-kernel-4.9/799/testReport/junit/Suite-k8s-1/20/K8sServicesTest_Checks_service_across_nodes_Checks_ClusterIP_Connectivity/
This change seems to have been introduced by cac5218
cc @brb @pchaigno @mazzy89
The text was updated successfully, but these errors were encountered: