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

Fix controller issue in handling empty pod labels for labelidentity #5404

Merged
merged 1 commit into from
Aug 22, 2023

Conversation

Dyanngg
Copy link
Contributor

@Dyanngg Dyanngg commented Aug 17, 2023

Fixes #5403

When a Pod is created without any labels at all, its labelidentity will include pod:<none> as defined by FormatLabels in apimachinery. This case is not handled correctly by the label group index and can cause the controller to crash.

@Dyanngg Dyanngg added kind/bug Categorizes issue or PR as related to a bug. action/backport Indicates a PR that requires backports. labels Aug 17, 2023
Copy link
Contributor

@antoninbas antoninbas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

pkg/controller/labelidentity/label_group_index.go Outdated Show resolved Hide resolved
luolanzone
luolanzone previously approved these changes Aug 18, 2023
Copy link
Contributor

@luolanzone luolanzone left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, please back-port this to the maintained releases.

pkg/controller/labelidentity/label_group_index.go Outdated Show resolved Hide resolved
Comment on lines 36 to 37
// As defined in https://github.com/kubernetes/apimachinery/blob/3e2600dc79feea6cdc8a9224bc8a6a7fcfee1466/pkg/labels/labels.go#L90
emptyLabels = "<none>"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we call labels.Set(...).String() instead of labels.FormatLabels(...) to return a clearer format? I think the latter is only used when printing the labels in kubectl for human readaibility.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah I think we could do that. Updated

When a Pod is created without any labels at all, its labelidentity
will include "pod:<none>" as defined by FormatLabels in apimachinery.
This case is not handled correctly by the label group index and can
cause the controller to crash.

Signed-off-by: Dyanngg <dingyang@vmware.com>
@Dyanngg Dyanngg force-pushed the fix-empty-label-crash branch from e9edd06 to c89b01f Compare August 18, 2023 21:36
@Dyanngg
Copy link
Contributor Author

Dyanngg commented Aug 18, 2023

/test-multicluster-e2e

Copy link
Member

@tnqn tnqn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@tnqn
Copy link
Member

tnqn commented Aug 21, 2023

/test-multicluster-e2e

2 similar comments
@tnqn
Copy link
Member

tnqn commented Aug 21, 2023

/test-multicluster-e2e

@Dyanngg
Copy link
Contributor Author

Dyanngg commented Aug 21, 2023

/test-multicluster-e2e

@tnqn
Copy link
Member

tnqn commented Aug 22, 2023

/skip-all

@tnqn tnqn merged commit 3e2e829 into antrea-io:main Aug 22, 2023
@tnqn tnqn added the action/release-note Indicates a PR that should be included in release notes. label Aug 22, 2023
@tnqn
Copy link
Member

tnqn commented Aug 22, 2023

@Dyanngg please backport it to applicable releases.

Dyanngg added a commit to Dyanngg/antrea that referenced this pull request Aug 28, 2023
…fter upgrade

This is a follow-up on PR antrea-io#5404 which intends to address issue antrea-io#5403.
In upgrade cases, Antrea controller will still receive LabelIdentity add events
for LabelIdentities created by previous controller, which it needs to handle
correctly. Also, the stale controller logic needs to be updated to ensure that
LabelIdentities created with old normalization method are cleaned up after upgrade.

Signed-off-by: Dyanngg <dingyang@vmware.com>
tnqn pushed a commit that referenced this pull request Aug 29, 2023
…fter upgrade (#5449)

This is a follow-up on PR #5404 which intends to address issue #5403.
In upgrade cases, Antrea controller will still receive LabelIdentity add events
for LabelIdentities created by previous controller, which it needs to handle
correctly. Also, the stale controller logic needs to be updated to ensure that
LabelIdentities created with old normalization method are cleaned up after upgrade.

Signed-off-by: Dyanngg <dingyang@vmware.com>
luolanzone pushed a commit to luolanzone/antrea that referenced this pull request Aug 29, 2023
…fter upgrade

This is a follow-up on PR antrea-io#5404 which intends to address issue antrea-io#5403.
In upgrade cases, Antrea controller will still receive LabelIdentity add events
for LabelIdentities created by previous controller, which it needs to handle
correctly. Also, the stale controller logic needs to be updated to ensure that
LabelIdentities created with old normalization method are cleaned up after upgrade.

Signed-off-by: Dyanngg <dingyang@vmware.com>
tnqn pushed a commit that referenced this pull request Aug 30, 2023
…fter upgrade (#5452)

This is a follow-up on PR #5404 which intends to address issue #5403.
In upgrade cases, Antrea controller will still receive LabelIdentity add events
for LabelIdentities created by previous controller, which it needs to handle
correctly. Also, the stale controller logic needs to be updated to ensure that
LabelIdentities created with old normalization method are cleaned up after upgrade.

Signed-off-by: Dyanngg <dingyang@vmware.com>
luolanzone pushed a commit to luolanzone/antrea that referenced this pull request Aug 31, 2023
…fter upgrade

This is a follow-up on PR antrea-io#5404 which intends to address issue antrea-io#5403.
In upgrade cases, Antrea controller will still receive LabelIdentity add events
for LabelIdentities created by previous controller, which it needs to handle
correctly. Also, the stale controller logic needs to be updated to ensure that
LabelIdentities created with old normalization method are cleaned up after upgrade.

Signed-off-by: Dyanngg <dingyang@vmware.com>
luolanzone pushed a commit to luolanzone/antrea that referenced this pull request Aug 31, 2023
…fter upgrade

This is a follow-up on PR antrea-io#5404 which intends to address issue antrea-io#5403.
In upgrade cases, Antrea controller will still receive LabelIdentity add events
for LabelIdentities created by previous controller, which it needs to handle
correctly. Also, the stale controller logic needs to be updated to ensure that
LabelIdentities created with old normalization method are cleaned up after upgrade.

Signed-off-by: Dyanngg <dingyang@vmware.com>
tnqn pushed a commit that referenced this pull request Aug 31, 2023
…fter upgrade (#5458)

This is a follow-up on PR #5404 which intends to address issue #5403.
In upgrade cases, Antrea controller will still receive LabelIdentity add events
for LabelIdentities created by previous controller, which it needs to handle
correctly. Also, the stale controller logic needs to be updated to ensure that
LabelIdentities created with old normalization method are cleaned up after upgrade.

Signed-off-by: Dyanngg <dingyang@vmware.com>
tnqn pushed a commit that referenced this pull request Aug 31, 2023
…fter upgrade (#5459)

This is a follow-up on PR #5404 which intends to address issue #5403.
In upgrade cases, Antrea controller will still receive LabelIdentity add events
for LabelIdentities created by previous controller, which it needs to handle
correctly. Also, the stale controller logic needs to be updated to ensure that
LabelIdentities created with old normalization method are cleaned up after upgrade.

Signed-off-by: Dyanngg <dingyang@vmware.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
action/backport Indicates a PR that requires backports. action/release-note Indicates a PR that should be included in release notes. kind/bug Categorizes issue or PR as related to a bug.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Antrea Multicluster] Antrea controller POD went in CrashLoopBackOff after enabling
5 participants