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

v1.15 Backports 2024-03-26 #31601

Merged
merged 15 commits into from
Mar 28, 2024
Merged

v1.15 Backports 2024-03-26 #31601

merged 15 commits into from
Mar 28, 2024

Commits on Mar 28, 2024

  1. endpointmanager: fix data race in test

    [ upstream commit 234e7d0 ]
    
    Go's race detector was unhappy with this test due to unserialised
    concurrent access to the last value of the fake gauge. Use an atomic
    float value instead, to ensure no weirdness can occur, and placate the
    race detector.
    
    Race detector warning (mildly edited):
    
    WARNING: DATA RACE
    Read at 0x00c000930488 by goroutine 205:
      github.com/cilium/cilium/pkg/endpointmanager.TestPolicyMapPressure.TestPolicyMapPressure.func1.func2()
          cilium/pkg/endpointmanager/policymap_pressure_test.go:27 +0x69
      github.com/stretchr/testify/assert.Eventually.func1()
          cilium/vendor/github.com/stretchr/testify/assert/assertions.go:1902 +0x33
    
    Previous write at 0x00c000930488 by goroutine 203:
      github.com/cilium/cilium/pkg/endpointmanager.(*fakeGague).Set()
          cilium/pkg/endpointmanager/policymap_pressure_test.go:45 +0x30
      github.com/cilium/cilium/pkg/endpointmanager.(*policyMapPressure).update()
          cilium/pkg/endpointmanager/policymap_pressure.go:82 +0x32a
      github.com/cilium/cilium/pkg/endpointmanager.newPolicyMapPressure.func1()
          cilium/pkg/endpointmanager/policymap_pressure.go:57 +0x2e
      github.com/cilium/cilium/pkg/trigger.(*Trigger).waiter()
          cilium/pkg/trigger/trigger.go:201 +0x771
      github.com/cilium/cilium/pkg/trigger.NewTrigger.gowrap1()
          cilium/pkg/trigger/trigger.go:122 +0x33
    
    Goroutine 205 (running) created at:
      github.com/stretchr/testify/assert.Eventually()
          cilium/vendor/github.com/stretchr/testify/assert/assertions.go:1902 +0x3d5
      github.com/stretchr/testify/assert.(*Assertions).Eventually()
          cilium/vendor/github.com/stretchr/testify/assert/assertion_forward.go:319 +0xc7
      github.com/cilium/cilium/pkg/endpointmanager.TestPolicyMapPressure.func1()
          cilium/pkg/endpointmanager/policymap_pressure_test.go:26 +0x2a8
      github.com/cilium/cilium/pkg/endpointmanager.TestPolicyMapPressure()
          cilium/pkg/endpointmanager/policymap_pressure_test.go:30 +0x205
    
    Goroutine 203 (running) created at:
      github.com/cilium/cilium/pkg/trigger.NewTrigger()
          cilium/pkg/trigger/trigger.go:122 +0x36d
      github.com/cilium/cilium/pkg/endpointmanager.newPolicyMapPressure()
          cilium/pkg/endpointmanager/policymap_pressure.go:52 +0x287
      github.com/cilium/cilium/pkg/endpointmanager.TestPolicyMapPressure()
          cilium/pkg/endpointmanager/policymap_pressure_test.go:18 +0x84
    
    Fixes: 28ce005 (endpointmanager: fix bpf policy pressure getting stuck.)
    
    Signed-off-by: David Bimmler <david.bimmler@isovalent.com>
    Signed-off-by: Tam Mach <tam.mach@cilium.io>
    bimmlerd authored and sayboras committed Mar 28, 2024
    Configuration menu
    Copy the full SHA
    44c4bff View commit details
    Browse the repository at this point in the history
  2. idpool: return pointer to pool

    [ upstream commit 0b56a21 ]
    
    IDPool contains a mutex, passing copies around is a potential footgun. I
    don't think we ever used it incorrectly, but I don't see a reason for
    all the copying either.
    
    Signed-off-by: David Bimmler <david.bimmler@isovalent.com>
    Signed-off-by: Tam Mach <tam.mach@cilium.io>
    bimmlerd authored and sayboras committed Mar 28, 2024
    Configuration menu
    Copy the full SHA
    2ca771b View commit details
    Browse the repository at this point in the history
  3. endpointmanager: idallocator: remove checkmate

    [ upstream commit 59a3100 ]
    
    Transform the test from using checkmate to standard Go tests, as it was
    not using any of the features anyway.
    
    Signed-off-by: David Bimmler <david.bimmler@isovalent.com>
    Signed-off-by: Tam Mach <tam.mach@cilium.io>
    bimmlerd authored and sayboras committed Mar 28, 2024
    Configuration menu
    Copy the full SHA
    aa82d05 View commit details
    Browse the repository at this point in the history
  4. endpointmanager: make idallocator a struct

    [ upstream commit 58ad35a ]
    
    The endpointmanagers idallocator package was using a package global pool
    for its identifier allocation. That's fine for running the agent, but
    causes flakes in testing when multiple tests access the same pool. It's
    also not idiomatic Go.
    
    This patch makes the local endpoint identifier allocator a struct, and
    the next patch will move it into the endpointmanager package itself, as
    there is no other consumer.
    
    While at it, also ensure that the RemoveAll method is only called from a
    testing context, by taking a testing.TB as an argument. We cannot simply
    move the method into the _test.go files, as tests from other packages
    use it.
    
    Signed-off-by: David Bimmler <david.bimmler@isovalent.com>
    bimmlerd authored and sayboras committed Mar 28, 2024
    Configuration menu
    Copy the full SHA
    2278d00 View commit details
    Browse the repository at this point in the history
  5. endpointmanager: move EP identifier alloc into pkg

    [ upstream commit a1a03cf ]
    
    The endpoint manager assumed it was the only consumer of the idallocator
    pkg anyway. Having a pkg that only has one consumer is pointless, hence
    move it, including tests. This also allows unexporting everything, and
    reducing API surface.
    
    Signed-off-by: David Bimmler <david.bimmler@isovalent.com>
    Signed-off-by: Tam Mach <tam.mach@cilium.io>
    bimmlerd authored and sayboras committed Mar 28, 2024
    Configuration menu
    Copy the full SHA
    0fd5c33 View commit details
    Browse the repository at this point in the history
  6. endpointmanager: deflake TestLookup

    [ upstream commit 0f27591 ]
    
    TestLookup could fail with
    
    --- FAIL: Test (0.12s)
        --- FAIL: Test/EndpointManagerSuite (0.12s)
            --- FAIL: Test/EndpointManagerSuite/TestLookup (0.00s)
                manager_test.go:438:
                    ... value *endpoint.Endpoint = &endpoint.Endpoint{[...]")
                    ... Test Name: endpoint does not exist
    
    In about 0.02% of the runs. Specifically, it would fail iff the endpoint
    created happened to randomly get ID 1234, out of the pool of 4096 IDs.
    
    Fix this by not creating an endpoint unconditionally in the test, so
    that there is no chance of creating one with the "wrong" id.
    
    Signed-off-by: David Bimmler <david.bimmler@isovalent.com>
    Signed-off-by: Tam Mach <tam.mach@cilium.io>
    bimmlerd authored and sayboras committed Mar 28, 2024
    Configuration menu
    Copy the full SHA
    f05b635 View commit details
    Browse the repository at this point in the history
  7. endpointmanager: check expose errors in test

    [ upstream commit fce1121 ]
    
    For some reason, lots of calls to expose didn't check the error
    returned. Specifically in the test context, this is not great, as it
    makes debugging test failures more difficult than necessary.
    
    Signed-off-by: David Bimmler <david.bimmler@isovalent.com>
    Signed-off-by: Tam Mach <tam.mach@cilium.io>
    bimmlerd authored and sayboras committed Mar 28, 2024
    Configuration menu
    Copy the full SHA
    64a272b View commit details
    Browse the repository at this point in the history
  8. endpointmanager: test: fix fakeGauge spelling

    [ upstream commit 6045c61 ]
    
    Signed-off-by: David Bimmler <david.bimmler@isovalent.com>
    Signed-off-by: Tam Mach <tam.mach@cilium.io>
    bimmlerd authored and sayboras committed Mar 28, 2024
    Configuration menu
    Copy the full SHA
    f291b3c View commit details
    Browse the repository at this point in the history
  9. endpointmanager: remove RemoveAll from interface

    [ upstream commit e2fbd38 ]
    
    Instead of having to import the testing pkg in non-testing code, let's
    remove the blocker on just having this method in the _test.go files.
    
    The endpoint manager in the daemon_test is re-initialized for every
    test, and since we don't have package global state any more we can just
    remove the call to RemoveAll, which solves all the problem nicely.
    
    Signed-off-by: David Bimmler <david.bimmler@isovalent.com>
    Signed-off-by: Tam Mach <tam.mach@cilium.io>
    bimmlerd authored and sayboras committed Mar 28, 2024
    Configuration menu
    Copy the full SHA
    cbe64ae View commit details
    Browse the repository at this point in the history
  10. datapath: Move IPsec logic to get local IPs

    [ upstream commit bee8535 ]
    
    Those helper functions to retrieve the local IPs are all IPsec specific
    so let's move them to the ipsec.go file. No functional changes in this
    commit.
    
    Signed-off-by: Paul Chaignon <paul.chaignon@gmail.com>
    pchaigno authored and sayboras committed Mar 28, 2024
    Configuration menu
    Copy the full SHA
    7a7741b View commit details
    Browse the repository at this point in the history
  11. ipsec: Refactor logic to get default IPsec interface

    [ upstream commit 4687325 ]
    
    This commit extends getDefaultEncryptionInterface to also handle
    tunneling mode. That change allows us to start using
    getDefaultEncryptionInterface everywhere we need to retrieve the default
    IPsec interface.
    
    The unit test for IPsec in subnet encryption mode (ENI and Azure IPAM
    modes) must be updated. Subnet encryption is only ever possible in
    native routing mode. If we were doing subnet encryption with tunneling,
    it would cause undefined behaviors (in the test would fail :)).
    
    Signed-off-by: Paul Chaignon <paul.chaignon@gmail.com>
    Signed-off-by: Tam Mach <tam.mach@cilium.io>
    pchaigno authored and sayboras committed Mar 28, 2024
    Configuration menu
    Copy the full SHA
    2ea7709 View commit details
    Browse the repository at this point in the history
  12. cmd, datapath: Support --devices for encryption interfaces

    [ upstream commit 3c6f957 ]
    
    The agent supported attaching the IPsec decryption logic to interfaces
    given via --devices. In that case, this logic was contained in bpf_host
    instead of bpf_network. This support is partially covered in ginkgo
    end-to-end tests.
    
    That support is however broken, as there doesn't seem to be anything
    preventing bpf_network from being reloaded in place of bpf_host on the
    same interfaces.
    
    This commit fixes it by implementing proper support for --devices in
    IPsec. If no devices flag is given then we fallback to using the
    encrypt-interface flag. That should allow us to deprecate
    encrypt-interface at a latter time.
    
    Signed-off-by: Paul Chaignon <paul.chaignon@gmail.com>
    pchaigno authored and sayboras committed Mar 28, 2024
    Configuration menu
    Copy the full SHA
    e03b2a0 View commit details
    Browse the repository at this point in the history
  13. workflows: Cover support for devices in IPsec tests

    [ upstream commit f153f42 ]
    
    Signed-off-by: Paul Chaignon <paul.chaignon@gmail.com>
    pchaigno authored and sayboras committed Mar 28, 2024
    Configuration menu
    Copy the full SHA
    2f132b9 View commit details
    Browse the repository at this point in the history
  14. ingress/gateway-api: ordered envoy filterchain

    [ upstream commit d8082f9 ]
    
    Currently, while translating K8s Ingress or Gateway API resources into Envoy resources,
    the filterchain is in random order. This leads to situations (especially in combination
    with Shared Ingress) where the order of the filterchains isn't guaranteed -
    resulting in unnecessary reconciliations.
    
    Therefore, this commit orders the filterchains within a Envoy Listener by the namespace
    and name of the TLS secret. This makes the translation deterministic.
    
    Signed-off-by: Marco Hofstetter <marco.hofstetter@isovalent.com>
    Signed-off-by: Tam Mach <tam.mach@cilium.io>
    mhofstetter authored and sayboras committed Mar 28, 2024
    Configuration menu
    Copy the full SHA
    8944d57 View commit details
    Browse the repository at this point in the history
  15. ingress/gateway-api: ordered envoy filterchain for TLS listener

    [ upstream commit 305ea74 ]
    
    Currently, while translating K8s Ingress or Gateway API resources into Envoy resources,
    the filterchain for TLS listeners is in random order. This leads to situations (especially in combination
    with Shared Ingress) where the order of the filterchains isn't guaranteed -
    resulting in unnecessary reconciliations.
    
    Therefore, this commit orders the filterchains within a Envoy Listener by the name of the backends.
    This makes the translation deterministic.
    
    Signed-off-by: Marco Hofstetter <marco.hofstetter@isovalent.com>
    Signed-off-by: Tam Mach <tam.mach@cilium.io>
    mhofstetter authored and sayboras committed Mar 28, 2024
    Configuration menu
    Copy the full SHA
    cde6274 View commit details
    Browse the repository at this point in the history