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

k8s.NewClient: Initialize Helm action configuration #1836

Merged
merged 2 commits into from
Jul 17, 2023

Conversation

michi-covalent
Copy link
Contributor

@michi-covalent michi-covalent commented Jul 13, 2023

2 commits:

this time i tested the fix locally. without this fix, i hit #1794 after ~5 minutes by running cilium status --wait repeatedly in a loop. with this fix, it hasn't hit the issue for more than 2 hours.

@maintainer-s-little-helper
Copy link

Commit 761761b does not contain "Signed-off-by".

Please follow instructions provided in https://docs.cilium.io/en/stable/contributing/development/contributing_guide/#developer-s-certificate-of-origin

This reverts commit a1c8a1b.

Signed-off-by: Michi Mutsuzaki <michi@isovalent.com>
@maintainer-s-little-helper
Copy link

Commit 761761b does not contain "Signed-off-by".

Please follow instructions provided in https://docs.cilium.io/en/stable/contributing/development/contributing_guide/#developer-s-certificate-of-origin

@michi-covalent michi-covalent temporarily deployed to ci July 13, 2023 18:04 — with GitHub Actions Inactive
@michi-covalent michi-covalent temporarily deployed to ci July 13, 2023 18:15 — with GitHub Actions Inactive
@michi-covalent michi-covalent temporarily deployed to ci July 13, 2023 18:19 — with GitHub Actions Inactive
@michi-covalent michi-covalent changed the title struggle k8s.NewClient: Initialize Helm action configuration Jul 13, 2023
@michi-covalent michi-covalent marked this pull request as ready for review July 13, 2023 18:19
@michi-covalent michi-covalent requested review from a team as code owners July 13, 2023 18:19
Copy link
Member

@asauber asauber left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks for the fix for LatestRelease

@michi-covalent michi-covalent temporarily deployed to ci July 13, 2023 19:28 — with GitHub Actions Inactive
Initialize Helm's action.Configuration once in k8s.NewClient so that all
the schemes get registered before cilium-cli starts using the
k8s.Client.

According to https://github.com/kubernetes/apimachinery/blob/c9b3b3a37189129d0d80b894dc5d393ade9d3b79/pkg/runtime/scheme.go#L44-L45
runtime.Scheme is not thread-safe until the registration is complete.

Fixes: #1794

Signed-off-by: Michi Mutsuzaki <michi@isovalent.com>
@michi-covalent michi-covalent temporarily deployed to ci July 13, 2023 19:34 — with GitHub Actions Inactive
@michi-covalent michi-covalent added the priority/release-blocker This issue will prevent the release of the next version of Cilium. label Jul 13, 2023
@michi-covalent michi-covalent merged commit b98ada8 into main Jul 17, 2023
19 checks passed
@michi-covalent michi-covalent deleted the pr/michi/concurrent-struggle branch July 17, 2023 15:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority/release-blocker This issue will prevent the release of the next version of Cilium.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

fatal error: concurrent map read and map write
5 participants