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

sysdump: Detect features from cilium-config ConfigMap #2004

Merged
merged 1 commit into from
Oct 6, 2023

Conversation

michi-covalent
Copy link
Contributor

  • Call FeatureSet.ExtractFromConfigMap to extract features from cilium-config.
  • Add tasks related to SPIRE server if and only if "mutual-auth-spiffe" feature is enabled.

Ref: #1962

- Call FeatureSet.ExtractFromConfigMap to extract features from
  cilium-config.
- Add tasks related to SPIRE server if and only if "mutual-auth-spiffe"
  feature is enabled.

Ref: #1962

Signed-off-by: Michi Mutsuzaki <[email protected]>
@michi-covalent michi-covalent temporarily deployed to ci October 4, 2023 17:19 — with GitHub Actions Inactive
@michi-covalent michi-covalent marked this pull request as ready for review October 4, 2023 18:03
@michi-covalent michi-covalent requested a review from a team as a code owner October 4, 2023 18:03
@michi-covalent michi-covalent requested review from derailed, a team and meyskens and removed request for a team October 4, 2023 18:03
@michi-covalent
Copy link
Contributor Author

requesting review from service mesh team 🚀🙏

@@ -263,6 +267,14 @@ func NewCollector(k KubernetesClient, o Options, startTime time.Time, cliVersion
}
c.log("ℹ️ %s ConfigMap not found in %s namespace", ciliumConfigMapName, c.Options.CiliumNamespace)
}
if c.CiliumConfigMap != nil && len(c.CiliumPods) > 0 {
ciliumVersion, err := c.Client.GetCiliumVersion(context.Background(), c.CiliumPods[0])
Copy link
Contributor

Choose a reason for hiding this comment

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

Yikes! hopefully we'll have a better way to retrieve version info... perhaps helm values??

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 this is not great, but it needs to work even when you install cilium without doing helm install, like helm template | kubectl apply - or similar.

Copy link
Contributor

@derailed derailed left a comment

Choose a reason for hiding this comment

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

@michi-covalent Nice work!

Copy link
Member

@meyskens meyskens left a comment

Choose a reason for hiding this comment

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

LGTM from service mesh

@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 Oct 6, 2023
@michi-covalent michi-covalent merged commit a037542 into main Oct 6, 2023
1 check passed
@michi-covalent michi-covalent deleted the pr/michi/lets-go branch October 6, 2023 17:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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.

3 participants