Skip to content

Conversation

@jdambly
Copy link

@jdambly jdambly commented Nov 14, 2024

Summary

This PR introduces significant updates to the Multus Helm chart, improving its functionality, simplifying its setup, and aligning it with Helm 3 best practices. Key changes include updating the chart to support the new multus-thin entry point, automating configuration, and improving the overall chart logic.

Key Changes

  1. Thin Entry Point:

    • Updated the chart to utilize the new multus-thin entry point, streamlining the deployment process for this mode.
  2. Auto Configuration:

    • Switched to auto configuration, eliminating the need for a ConfigMap. This reduces the complexity of setup and makes the chart easier to manage.
  3. CRD Best Practices:

    • Moved the CustomResourceDefinitions (CRDs) into the crds/ directory, adhering to Helm 3 best practices for CRD management.
  4. Chart Logic Cleanup:

    • Replaced complex if statements with with blocks to simplify the chart logic, making it more readable and maintainable.
    • Improved support for all Multus command-line arguments, ensuring flexibility and better customization.
  5. Command Flag Support:

    • Added full support for configuring all command-line flags for the multus-thin entry point, offering enhanced control over the Multus deployment.
  6. Helm Docs Integration:

    • Integrated helm-docs to automatically generate the README.md file from the values.yaml, ensuring that documentation stays up-to-date with the chart’s configurable values.
  7. InitContainer for Installation:

    • Moved the Multus installation logic into an initContainer, improving the management of Multus binaries and configuration during startup.
  8. CI Integration:

    • Added a GitHub Action workflow for linting and testing the chart. This ensures that the chart is validated against Helm best practices, and any issues are caught during development.

Rationale

  • Simplified Setup: Auto configuration and the removal of the ConfigMap simplify deployment, reducing the number of manual steps required.
  • Alignment with Best Practices: Moving CRDs and improving logic structure ensures the chart follows Helm 3 guidelines and improves long-term maintainability.
  • Better Control and Flexibility: Supporting all command-line flags and switching to multus-thin gives users more control over their Multus configuration.

Testing & Validation

  • GitHub Action CI: The included CI workflow automatically lint tests and runs basic validation on each PR or push, ensuring consistency and quality.
  • Manual Testing: The updated chart has been manually tested in various Kubernetes environments to validate the changes and confirm proper Multus deployment.

Next Steps

  • Future enhancements will include further optimizations and possible additional testing scenarios, but this update lays the foundation for a more streamlined, manageable, and flexible Multus Helm chart.

* intial commit for multus upgrade

* intial commit for multus upgrade

* Allows manually triggering the workflow
* Trigger GitHub Action

* seems to be an issue with whereabouts that needs to be updated

* remove the dry-run flag

* break out github action per chart
* Trigger GitHub Action

* seems to be an issue with whereabouts that needs to be updated

* remove the dry-run flag

* break out github action per chart

* added better support for autoconfig

* created new readme file with helm-docs

* always create a service account

* always create a daemonset

* always create a cluster role

* always create a cluster role

* removed annoation of configmap in favour of using auto config

* refactor node selector and tolerations

* updated notes

* added template for go docs, and updated vaules file for better help ouput

* updated chart yaml with corrected urls

* updated resources and docs for this config

* cleaned up white space for notes
@oujonny
Copy link

oujonny commented Apr 9, 2025

Any reason why this PR is not merged yet?

@oujonny
Copy link

oujonny commented Apr 15, 2025

@e0ne @dougbtv can you please unblock this PR and review the changes? Otherwise, I'm also open to reviewing such PR in the future, as we are using this chart heavily and relying on it.

Copy link
Collaborator

@e0ne e0ne left a comment

Choose a reason for hiding this comment

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

LGTM

@oujonny
Copy link

oujonny commented Apr 26, 2025

Can we merge now this PR? @e0ne

@ShutoAraki
Copy link

Any updates on this? Any reason why this is not getting merged? @jdambly @e0ne

@ejstacey
Copy link

ejstacey commented Aug 20, 2025

I pulled @jdambly's repo and compiled/pushed it locally, and I hit an issue.

I run k3s, so I have a weird location for cni/cnibin dirs. I can use the values args.cniConfDir/args.cniBinDir to specify that they get used for the script that gets run, but the volumes/mounts of the daemonsets still use the default locations. This causes the daemonsets to crash.

This fixed it for me (dirty, hacky, only for me):

diff --git a/multus/templates/daemonSet.yaml b/multus/templates/daemonSet.yaml
index bcbf0b1..e7fee37 100644
--- a/multus/templates/daemonSet.yaml
+++ b/multus/templates/daemonSet.yaml
@@ -139,9 +139,9 @@ spec:
         {{- end }}
         volumeMounts:
         - name: cni
-          mountPath: /host/etc/cni/net.d
+          mountPath: /host/var/lib/rancher/k3s/agent/etc/cni/net.d
         - name: cnibin
-          mountPath: /host/opt/cni/bin
+          mountPath: /host/var/lib/rancher/k3s/data/cni/
       initContainers:
       - name: install-multus-binary
         image: {{ .Values.image.repository }}:{{ .Values.image.tag }}
@@ -163,9 +163,9 @@ spec:
       volumes:
         - name: cni
           hostPath:
-            path: /etc/cni/net.d
+            path: /var/lib/rancher/k3s/agent/etc/cni/net.d
         - name: cnibin
           hostPath:
-            path: /opt/cni/bin
+            path: /var/lib/rancher/k3s/data/cni/
 {{- end }}

This works when I have helm values like so:

        args:
          cniConfDir: /host/var/lib/rancher/k3s/agent/etc/cni/net.d
          cniBinDir: /host/var/lib/rancher/k3s/data/cni/
          multusAutoconfigDir: /host/var/lib/rancher/k3s/agent/etc/cni/net.d
          multusCNIConfDir: /host/var/lib/rancher/k3s/agent/etc/cni/multus/net.d
          multusKubeConfigFileHost: /var/lib/rancher/k3s/agent/etc/cni/net.d/multus.d/multus.kubeconfig 

Just FYI.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants