Fix panic in felix flushHostIPUpdates

cyclinder commented 1 week ago

Description

Related issues/PRs

fixes https://github.com/projectcalico/calico/issues/9373

Todos

[ ] Tests
[ ] Documentation
[ ] Release note

Release Note

Fix a panic in Felix when accessing a nil address in flushing host addresses, i.e. flushHostIPUpdates function.

Reminder for the reviewer

Make sure that this PR has the correct labels and milestone set.

Every PR needs one docs-* label.

docs-pr-required: This change requires a change to the documentation that has not been completed yet.
docs-completed: This change has all necessary documentation completed.
docs-not-required: This change has no user-facing impact and requires no docs.

Every PR needs one release-note-* label.

release-note-required: This PR has user-facing changes. Most PRs should have this label.
release-note-not-required: This PR has no user-facing changes.

Other optional labels:

cherry-pick-candidate: This PR should be cherry-picked to an earlier release. For bug fixes only.
needs-operator-pr: This PR is related to install and requires a corresponding change to the operator.

mazdakn commented 1 week ago

/sem-approve

caseydavenport commented 1 week ago

@mazdakn @cyclinder do we know how we were getting nil values in this buffer in the first place?

mazdakn commented 1 week ago

@caseydavenport I've not spent time to find the root cause. If we want to do that then we need to spend sometime to try to reproduce the issue. Given that the issue happened in a dual stack cluster, it might be that IPv6 is enabled, but ipv6 address is missing.

cyclinder commented 1 week ago

It may be that there is an ipv6 address on the Calico startup, but then it is missing. when the ipv6 address is missing, I think updating it to an empty string is ok.

mazdakn commented 6 days ago

@cyclinder it seems the CI execution is not triggered. Can you please try pushing a dummy commit to trigger it.

mazdakn commented 6 days ago

/sem-approve

mazdakn commented 6 days ago

@cyclinder Thanks again for the fix. I merged and cherry picked this fix to v3.29 and v3.28 since we are going to ship v3.29.1 patch release soon.

projectcalico / calico