ctmap: do not call InitMapInfo() in init()

[kkourt]

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 address #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.

see the commit message for more details

Fixes: cac5218
Fixes: d639905
Fixes: #15141
Fixes: #15337

Signed-off-by: Kornilios Kourtis kornilios@isovalent.com

[kkourt]

test-me-please

[brb]

This will fix #15337, right?

[kkourt]

This will fix #15337, right?

That's the intention, yes.

[kkourt]

test-me-please

[kkourt]

test-me-please

[kkourt]

test-me-please

[kkourt]

test-me-please

[kkourt]

test-gke

[kkourt]

retest-net-next

[kkourt]

retest-runtime

[kkourt]

There are currently two failures: one for gke-stable with a "Build Scaling cluster failed" and another one that looks like a flake: #15621. So, I will mark the PR as Ready for review.

[brb]

@kkourt I think there is still one case where we might wrongly initialize the maps. We do ctmap.InitMapInfo() before finishKubeProxyReplacement() which might disable the NodePort BPF.

[kkourt]

I guess you mean finishKubeProxyReplacementInit()

Here's the order of events:

• runDaemon() starts

  cilium/daemon/cmd/daemon_main.go

  Line 1511 in 27eda2c

  func runDaemon() {

  • calls NewDaemon()

    cilium/daemon/cmd/daemon_main.go

    Line 1580 in 27eda2c

    d, restoredEndpoints, err := NewDaemon(ctx, cancel,

    • calls ctmap.InitMapinfo()

      cilium/daemon/cmd/daemon.go

      Line 307 in 27eda2c

      ctmap.InitMapInfo(option.Config.CTMapEntriesGlobalTCP, option.Config.CTMapEntriesGlobalAny,

    • calls finishKubeProxyReplacementInit()

      cilium/daemon/cmd/daemon.go

      Line 573 in 27eda2c

      finishKubeProxyReplacementInit(isKubeProxyReplacementStrict)

  • spawns goroutine that will delete maps

    cilium/daemon/cmd/daemon_main.go

    Lines 1657 to 1668 in 27eda2c

    	go func() {
    	if restoreComplete != nil {
    	<-restoreComplete
    	}
    	d.dnsNameManager.CompleteBootstrap()
    	ms := maps.NewMapSweeper(&EndpointMapManager{
    	EndpointManager: d.endpointManager,
    	})
    	ms.CollectStaleMapGarbage()
    	ms.RemoveDisabledMaps()
    	}()

I think if the maps are initialized before we disable NodePort BPF, then it would mean that we will not delete the maps. So I think that the map controller error might not trigger. On the other hand, this would mean that for the case were NodePort is disabled late (i.e., in finishKubeProxyReplacementInit) we do not delete the snat maps.

[kkourt]

I would suggest that, assuming this PR fixes the faulting map controller, we should tackle the issue mentioned by @brb
by restructuring initialization #13573 and properly deal with the various dependencies.