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.14 Backports 2023-11-08 #29064

Merged
merged 3 commits into from Nov 9, 2023
Merged

v1.14 Backports 2023-11-08 #29064

merged 3 commits into from Nov 9, 2023

Conversation

aanm
Copy link
Member

@aanm aanm commented Nov 8, 2023

@aanm aanm added kind/backports This PR provides functionality previously merged into master. backport/1.14 This PR represents a backport for Cilium 1.14.x of a PR that was merged to main. labels Nov 8, 2023
@aanm
Copy link
Member Author

aanm commented Nov 8, 2023

/test-backport-1.14

@aanm aanm force-pushed the pr/v1.14-backport-2023-11-08 branch from 3233363 to ed2d25d Compare November 8, 2023 22:22
@aanm
Copy link
Member Author

aanm commented Nov 8, 2023

/test-backport-1.14

1 similar comment
@aanm
Copy link
Member Author

aanm commented Nov 8, 2023

/test-backport-1.14

@aanm aanm marked this pull request as ready for review November 8, 2023 23:20
@aanm aanm requested a review from a team as a code owner November 8, 2023 23:20
@maintainer-s-little-helper maintainer-s-little-helper bot added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Nov 9, 2023
Copy link
Member

@giorio94 giorio94 left a comment

Choose a reason for hiding this comment

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

One comment inline, which I missed during my previous review (and that might be worth addressing).

@@ -579,7 +594,7 @@ func (a *Allocator) lockedAllocate(ctx context.Context, key AllocatorKey) (idpoo
return 0, false, false, fmt.Errorf("Found master key after proceeding with new allocation for %s", k)
}

err = a.backend.AllocateIDIfLocked(ctx, id, key, lock)
key, err = a.backend.AllocateIDIfLocked(ctx, id, key, lock)
Copy link
Member

Choose a reason for hiding this comment

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

Sorry, I missed this during my last review. When AllocateIDIfLocked returns an error, the current implementation also returns a nil key. Which means that the error message below will be incorrect.

@maintainer-s-little-helper maintainer-s-little-helper bot removed ready-to-merge This PR has passed all tests and received consensus from code owners to merge. labels Nov 9, 2023
…RD mode

[ upstream commit e39fcae ]

In CRD mode, the Cilium agent uses CRD to create identities. After an
identity is created, the agent acquires a reference for that key. This
involves fetching the CRD from the local Kubernetes cache and checking
for an annotation applied by cilium-operator to mark the identity for
deletion. However, there may be a delay before the Cilium Identity is
cached locally, leading to the 'Key allocation attempt failed' error. This
patch ensures that we fallback to the newly allocated Cilium Identity if
it's not found in the Kubernetes cache.

Signed-off-by: André Martins <andre@cilium.io>
Signed-off-by: André Martins <andre@cilium.io>
[ upstream commit df05754 ]

We don't need to always DeepCopy Cilium Identity. We just need to
perform that operation if we are going perform writes.

Signed-off-by: André Martins <andre@cilium.io>
Signed-off-by: André Martins <andre@cilium.io>
[ upstream commit 3b2e1ad ]

Renamed 'slave' to 'secondary' in the error messages that are presented
to users.

Signed-off-by: André Martins <andre@cilium.io>
Signed-off-by: André Martins <andre@cilium.io>
@aanm aanm force-pushed the pr/v1.14-backport-2023-11-08 branch from ed2d25d to 93ee0bf Compare November 9, 2023 09:42
@aanm aanm requested a review from giorio94 November 9, 2023 09:43
@aanm
Copy link
Member Author

aanm commented Nov 9, 2023

/test-backport-1.14

@aanm aanm merged commit c3e096f into v1.14 Nov 9, 2023
194 of 198 checks passed
@aanm aanm deleted the pr/v1.14-backport-2023-11-08 branch November 9, 2023 11:27
@maintainer-s-little-helper maintainer-s-little-helper bot added ready-to-merge This PR has passed all tests and received consensus from code owners to merge. labels Nov 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport/1.14 This PR represents a backport for Cilium 1.14.x of a PR that was merged to main. kind/backports This PR provides functionality previously merged into master. ready-to-merge This PR has passed all tests and received consensus from code owners to merge.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants