Skip to content
This repository was archived by the owner on Oct 20, 2025. It is now read-only.

Fix handling of empty podIPs annotation#1397

Merged
fasaxc merged 1 commit into
projectcalico:masterfrom
fasaxc:fix-pod-ip-check
Apr 7, 2021
Merged

Fix handling of empty podIPs annotation#1397
fasaxc merged 1 commit into
projectcalico:masterfrom
fasaxc:fix-pod-ip-check

Conversation

@fasaxc

@fasaxc fasaxc commented Apr 1, 2021

Copy link
Copy Markdown
Member

Description

  • Fix that IsFinished wasn't checking whether the podIPs annotation was actually present. Since the zero value for a string is "", we need to check for presence too or we'll be confused for non-Calico CNIs that don't set the annotation.
  • Fix up various other inconsistencies:
    • A "finished" pod shouldn't be "ready"
    • HasIPAddress and getPodIPs didn't honour the annotation.

Todos

  • Tests
  • Documentation
  • Release note

Release Note

CHANGE REVERTED: Fix that Felix would incorrectly treat any deleted pod as immediately finished.  This meant that pods networked with non-Calico CNIs would not have connectivity in the termination grace period.

@fasaxc fasaxc force-pushed the fix-pod-ip-check branch from 04f87b2 to 6eb8867 Compare April 6, 2021 10:56
@fasaxc fasaxc marked this pull request as ready for review April 6, 2021 12:32
@fasaxc fasaxc merged commit 428793c into projectcalico:master Apr 7, 2021
@fasaxc fasaxc deleted the fix-pod-ip-check branch April 7, 2021 08:21
caseydavenport added a commit that referenced this pull request Apr 8, 2021
…-#1397-origin-release-v3.18

Automated cherry pick of #1397: Fix handling of empty podIPs annotation
fasaxc added a commit that referenced this pull request Apr 21, 2021
marvin-tigera added a commit that referenced this pull request Apr 21, 2021
…rry-pick-of-#1397-origin-release-v3.18

Revert "Automated cherry pick of #1397: Fix handling of empty podIPs annotation"
@wy1079385180

Copy link
Copy Markdown

why rollout restart sts can still see pod ip in annotation?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants