-
Notifications
You must be signed in to change notification settings - Fork 267
passt: Fix docs #916
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
passt: Fix docs #916
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -62,16 +62,74 @@ the passt plugin needs to: | |||||
| And in detail: | ||||||
|
|
||||||
| ### Passt CNI deployment on nodes | ||||||
| The CNI plugin binary can be retrieved directly from the kubevirt release | ||||||
| assets (on GitHub) or to be built from its | ||||||
|
|
||||||
| #### v1.6.0 and above | ||||||
| Kubevirt releases a container image `quay.io/kubevirt/network-passt-binding-cni:v1.6.0`, | ||||||
oshoval marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
| which can be used via daemonSet to copy the CNI binary to the nodes. | ||||||
|
|
||||||
| Create the following DaemonSet: | ||||||
oshoval marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
|
|
||||||
| ```yaml | ||||||
| apiVersion: apps/v1 | ||||||
| kind: DaemonSet | ||||||
| metadata: | ||||||
| name: passt-binding-cni | ||||||
| namespace: kubevirt | ||||||
| labels: | ||||||
| tier: node | ||||||
| app: passt-binding-cni | ||||||
| spec: | ||||||
| selector: | ||||||
| matchLabels: | ||||||
| name: passt-binding-cni | ||||||
| updateStrategy: | ||||||
| type: RollingUpdate | ||||||
| rollingUpdate: | ||||||
| maxUnavailable: 10% | ||||||
| template: | ||||||
| metadata: | ||||||
| labels: | ||||||
| name: passt-binding-cni | ||||||
| tier: node | ||||||
| app: passt-binding-cni | ||||||
| annotations: | ||||||
| description: passt-binding-cni installs 'passt binding' CNI on cluster nodes | ||||||
| spec: | ||||||
| priorityClassName: system-cluster-critical | ||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: Do we really need to define a high priority class?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. on clusters that are on their resource edge, yes |
||||||
| containers: | ||||||
| - name: installer | ||||||
| image: quay.io/kubevirt/network-passt-binding-cni:v1.6.0 | ||||||
| command: [ "/bin/sh", "-ce" ] | ||||||
| args: | ||||||
| - | | ||||||
| ls -la "/cni/kubevirt-passt-binding" | ||||||
| cp -f "/cni/kubevirt-passt-binding" "/opt/cni/bin" | ||||||
| echo "passt binding CNI plugin installation complete..sleep infinity" | ||||||
| sleep 2147483647 | ||||||
| resources: | ||||||
| requests: | ||||||
| cpu: "10m" | ||||||
| memory: "15Mi" | ||||||
| securityContext: | ||||||
| privileged: true | ||||||
| volumeMounts: | ||||||
| - name: cnibin | ||||||
| mountPath: /opt/cni/bin | ||||||
| volumes: | ||||||
| - name: cnibin | ||||||
| hostPath: | ||||||
| path: /opt/cni/bin | ||||||
| ``` | ||||||
|
|
||||||
| #### prior to v1.6.0 | ||||||
| The CNI binary can be retrieved directly from the kubevirt release assets (on GitHub) or to be built from its | ||||||
| [sources](https://github.com/kubevirt/kubevirt/tree/release-1.1/cmd/cniplugins/passt-binding). | ||||||
|
|
||||||
| > **Note**: The kubevirt project uses Bazel to build the binaries and container images. | ||||||
| > For more information in how to build the whole project, visit the developer | ||||||
| > [getting started guide](https://github.com/kubevirt/kubevirt/blob/release-1.1/docs/getting-started.md). | ||||||
|
|
||||||
| Once the binary is ready, you may rename it to a meaningful name | ||||||
| (e.g. `kubevirt-passt-binding`). | ||||||
| Once the binary is ready, you may rename it to a meaningful name (e.g. `kubevirt-passt-binding`). | ||||||
| This name is used in the NetworkAttachmentDefinition configuration. | ||||||
|
|
||||||
| Copy the binary to each node in your cluster. | ||||||
|
|
@@ -114,11 +172,11 @@ specified in the Kubevirt CR when registering the network binding plugin. | |||||
| ### Feature Gate | ||||||
| For KubeVirt versions prior to v1.5, make sure to enable the `NetworkBindingPlugins` FG. | ||||||
| ``` | ||||||
| kubectl patch kubevirts -n kubevirt kubevirt --type=json -p='[{"op": "add", "path": "/spec/configuration/developerConfiguration/featureGates/-", "value": "NetworkBindingPlugins"}]' | ||||||
| kubectl patch kubevirt -n kubevirt kubevirt --type=merge -p='{"spec":{"configuration":{"developerConfiguration":{"featureGates":["NetworkBindingPlugins"]}}}}' | ||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could you please elaborate on the advantages of the proposed approach?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. the previous fails if annotation doesnt exist
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Which annotation?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. not annotation, it doesnt work if this path doesnt exists beforehand if you spin kubevirt and try you will see |
||||||
| ``` | ||||||
|
|
||||||
| > **Note**: The specific passt plugin has no FG by its own. It is up to the cluster | ||||||
| > admin to decide if the plugin is to be available in the cluster. | ||||||
| **Note**: | ||||||
| > It is up to the cluster admin to decide if the plugin is to be available in the cluster. | ||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think that it is implicit since users have to manually deploy it on their clusters.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. same text that was before, i just removed the "The specific passt plugin has no FG by its own"
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't see the removal, but this is not a blocker.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. hmm it is double there now, will update
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. now done |
||||||
| > The passt binding is still in evaluation, use it with care. | ||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please consider the following alternative wording:
Suggested change
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. i just changed the place, i prefer to not change it on this PR
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't see the removal, but this is not a blocker.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||||||
|
|
||||||
| ### Passt Registration | ||||||
|
|
@@ -131,7 +189,7 @@ kubectl patch kubevirts -n kubevirt kubevirt --type=json -p='[{"op": "add", "pat | |||||
| "binding": { | ||||||
| "passt": { | ||||||
| "networkAttachmentDefinition": "default/netbindingpasst", | ||||||
| "sidecarImage": "quay.io/kubevirt/network-passt-binding:20231205_29a16d5c9", | ||||||
| "sidecarImage": "quay.io/kubevirt/network-passt-binding:v1.6.0", | ||||||
| "migration": {}, | ||||||
| "computeResourceOverhead": { | ||||||
| "requests": { | ||||||
|
|
@@ -210,7 +268,7 @@ spec: | |||||
| terminationGracePeriodSeconds: 0 | ||||||
| volumes: | ||||||
| - containerDisk: | ||||||
| image: quay.io/kubevirt/fedora-with-test-tooling-container-disk:v1.1.0 | ||||||
| image: quay.io/kubevirt/fedora-with-test-tooling-container-disk:v1.6.0 | ||||||
| name: containerdisk | ||||||
| - cloudInitNoCloud: | ||||||
| networkData: | | ||||||
|
|
||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since the option to deploy the CNI via a daemonset is only available starting v1.6, I suggest that we will keep both deployment options.
Please also consider giving a few words about the relevant artifacts KubeVirt releases (CNI binary and a container image containing the CNI binary).