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

feat: New host collector and analyzer for Kernel Configs #1546

Merged
merged 11 commits into from
May 26, 2024

Conversation

nvanthao
Copy link
Member

Description, Motivation and Context

This PR introduces new host collector+analyzer Kernel Configs
The collector will collect kernel configurations and the analyzer can check existence of kernel config key and value.

This will help with pre-flight checks for software like k0s and Embedded Cluster
https://docs.k0sproject.io/stable/external-runtime-deps/#linux-kernel-configuration

One thing to highlight in this change is that for this collector, all pass outcomes are evaluated, and the outcome will be set to fail if when condition does not match.

Sample spec

apiVersion: troubleshoot.sh/v1beta2
kind: SupportBundle
metadata:
  name: kernel-configs
spec:
  hostCollectors:
    - kernelConfigs: {}
  hostAnalyzers:
    - kernelConfigs:
        collectorName: "Kernel Configs Check"
        strict: true
        outcomes:
          - pass:
              when: "CONFIG_CGROUP_FREEZER=y"
              message: "The cgroup freezer is enabled"
          - pass:
              when: "CONFIG_NETFILTER_XTABLES=m"
              message: "There is netfilter xtable module"

Checklist

  • New and existing tests pass locally with introduced changes.
  • Tests for the changes have been added (for bug fixes / features)
  • The commit message(s) are informative and highlight any breaking changes
  • Any documentation required has been added/updated. For changes to https://troubleshoot.sh/ create a PR here

Does this PR introduce a breaking change?

  • Yes
  • No

@nvanthao nvanthao added the type::feature New feature or request label May 15, 2024
@nvanthao nvanthao self-assigned this May 15, 2024
@nvanthao nvanthao requested a review from a team as a code owner May 15, 2024 12:04
@nvanthao nvanthao force-pushed the gerard/sc-104829/kernel-config branch 2 times, most recently from 1df4af7 to 958ceca Compare May 16, 2024 23:39
DexterYan
DexterYan previously approved these changes May 19, 2024
DexterYan
DexterYan previously approved these changes May 20, 2024
Copy link
Member

@banjoh banjoh left a comment

Choose a reason for hiding this comment

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

One thing to highlight in this change is that for this collector, all pass outcomes are evaluated, and the outcome will be set to fail if when condition does not match.

This approach will confuse people. It differs from how outcomes have been designed where when one of the outcomes evaluates to true, evaluation of outcomes for that analyser stops.

Other analysers follow an approach where inputs to evaluate against are passed in as separate parameters. Here is an example of how a kernel config analyser might look. Note that the list of kernel config values are passed in as selectedKernelConfigs input value, not defining each in an outcome.

apiVersion: troubleshoot.sh/v1beta2
kind: SupportBundle
spec:
  hostCollectors:
    - kernelConfigs: {}
  hostAnalyzers:
    - kernelConfigs:
        collectorName: "Kernel Configs Check"
        strict: true
        selectedKernelConfigs:
          - CONFIG_CGROUP_FREEZER=y
          - CONFIG_NETFILTER_XTABLES=m
        outcomes:
          - pass:
              when: "true"
              message: "Selected kernel configs are present"
          - fail:
              message: "{{ .ConfigsNotFound }} kernel configs are missing"

There are exceptions such as velero and ceph analysers where the outcomes are hardcoded in the analyser itself. This is acceptable because users do not define outcomes.

@banjoh
Copy link
Member

banjoh commented May 21, 2024

The collector implementation looks good

@nvanthao
Copy link
Member Author

One thing to highlight in this change is that for this collector, all pass outcomes are evaluated, and the outcome will be set to fail if when condition does not match.

This approach will confuse people. It differs from how outcomes have been designed where when one of the outcomes evaluates to true, evaluation of outcomes for that analyser stop.

Other analysers follow an approach where inputs to evaluate against are passed in as separate parameters. Here is an example of how a kernel config analyser might look. Note that the list of kernel config values are passed in as selectedKernelConfigs input value, not defining each in an outcome.

apiVersion: troubleshoot.sh/v1beta2
kind: SupportBundle
spec:
  hostCollectors:
    - kernelConfigs: {}
  hostAnalyzers:
    - kernelConfigs:
        collectorName: "Kernel Configs Check"
        strict: true
        selectedKernelConfigs:
          - CONFIG_CGROUP_FREEZER=y
          - CONFIG_NETFILTER_XTABLES=m
        outcomes:
          - pass:
              when: "true"
              message: "Selected kernel configs are present"
          - fail:
              message: "{{ .ConfigsNotFound }} kernel configs are missing"

There are exceptions such as velero and ceph analysers where the outcomes are hardcoded in the analyser itself. This is acceptable because users do not define outcomes.

That's a great idea on selectedKernelConfigs, let me update the code + doc and update us

@nvanthao nvanthao force-pushed the gerard/sc-104829/kernel-config branch from 77e36de to add21c3 Compare May 22, 2024 06:31
@nvanthao nvanthao requested a review from banjoh May 24, 2024 03:56
@nvanthao
Copy link
Member Author

Hi @banjoh ,

I have made the requested changes, please let me know how it goes

Sample spec

apiVersion: troubleshoot.sh/v1beta2
kind: SupportBundle
metadata:
  name: kernel-configs
spec:
  hostCollectors:
    - kernelConfigs: {}
  hostAnalyzers:
    - kernelConfigs:
        collectorName: "Gerard Kernel Configs Test"
        strict: true
        selectedConfigs:
        - CONFIG_CGROUP_FREEZER=y
        - CONFIG_NETFILTER_XTABLES=m
        outcomes:
          - pass:
              message: "all kernel configs are available"
          - fail:
              message: "missing kernel config(s): {{ .ConfigsNotFound }}"

Copy link
Member

@banjoh banjoh left a comment

Choose a reason for hiding this comment

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

LGTM

@nvanthao nvanthao merged commit 80e5fac into main May 26, 2024
27 checks passed
@nvanthao nvanthao deleted the gerard/sc-104829/kernel-config branch May 26, 2024 23:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type::feature New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants