cilium icon indicating copy to clipboard operation
cilium copied to clipboard

Fix complaint about nil IP address on restore of cilium_host

Open christarazi opened this issue 3 years ago • 9 comments

  • daemon/cmd: Fix complaint about nil IP address on restore of cilium_host
  • pkg/datapath/loader: Export SetupBaseDevice
  • daemon/cmd: Add new privileged test suite
  • pkg/netns: Expand godoc on ReplaceNetNSWithName

Fixes: https://github.com/cilium/cilium/issues/18118

christarazi avatar Aug 01 '22 19:08 christarazi

/test

christarazi avatar Aug 01 '22 21:08 christarazi

/test

Job 'Cilium-PR-K8s-1.16-kernel-4.9' failed:

Click to show.

Test Name

K8sPolicyTest Multi-node policy test with L7 policy using connectivity-check to check datapath

Failure Output

FAIL: cannot install connectivity-check

If it is a flake and a GitHub issue doesn't already exist to track it, comment /mlh new-flake Cilium-PR-K8s-1.16-kernel-4.9 so I can create one.

christarazi avatar Aug 01 '22 21:08 christarazi

/test

christarazi avatar Aug 02 '22 19:08 christarazi

/test

Edit: CI passed except for relevant runtime test. It failed because the cilium_host device already exists and so the LinkAdd is failing. Added a new check to make this code idempotent.

christarazi avatar Aug 02 '22 21:08 christarazi

/test-runtime

Edit: Runtime failed because my test moves cilium_host to the a network namespace, but the namespace is deleted after my test runs, so we need to move the interface back to the original netns to maintain idempotentcy

christarazi avatar Aug 03 '22 05:08 christarazi

/test-runtime

Edit: Runtime failed again due to netns management issues. I've simplified the code to create the cilium_host under the new netns which means we don't need to handle it already existing in the host netns and maintaining idempotency.

christarazi avatar Aug 03 '22 23:08 christarazi

/test-runtime

christarazi avatar Aug 04 '22 22:08 christarazi

/test-runtime

christarazi avatar Aug 05 '22 21:08 christarazi

CI has passed and we have approving reviews. Marking ready to merge.

christarazi avatar Aug 10 '22 04:08 christarazi

Commit 205810f2785ec09f149251ec827a880ff7560e26 added by this PR caused merge conflicts on backport to v1.10 and v1.11 because it relies on changes from #20453. AFAICT, the main reason we want to backport this PR is the first commit 08205dde0f6dad5cfaf18a947d17e7c67f60cbcc which applies cleanly. So I've only picked that one for now.

@christarazi do you think this is ok or should the remaining commits still be backported?

tklauser avatar Aug 17 '22 08:08 tklauser