-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Binding plugin API #10284
Binding plugin API #10284
Changes from all commits
fe0a58b
b4c7ff6
234467a
6068a78
a79137b
82793ae
ba7bbb9
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 |
---|---|---|
|
@@ -58,3 +58,28 @@ func validateInterfaceStateValue(field *k8sfield.Path, spec *v1.VirtualMachineIn | |
} | ||
return causes | ||
} | ||
|
||
func validateInterfaceBinding(field *k8sfield.Path, spec *v1.VirtualMachineInstanceSpec) []metav1.StatusCause { | ||
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. Someone needs to call it :) Unit tests will be nice too. 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. oops:) done. 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: How about 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. It will be incorrect. It checks all binding configuration, even the old one. |
||
var causes []metav1.StatusCause | ||
for idx, iface := range spec.Domain.Devices.Interfaces { | ||
if iface.Binding != nil { | ||
if hasInterfaceBindingMethod(iface) { | ||
causes = append(causes, metav1.StatusCause{ | ||
Type: metav1.CauseTypeFieldValueInvalid, | ||
Message: fmt.Sprintf("logical %s interface cannot have both binding plugin and interface binding method", iface.Name), | ||
Field: field.Child("domain", "devices", "interfaces").Index(idx).Child("binding").String(), | ||
}) | ||
} | ||
} | ||
} | ||
return causes | ||
} | ||
|
||
func hasInterfaceBindingMethod(iface v1.Interface) bool { | ||
return iface.InterfaceBindingMethod.Bridge != nil || | ||
iface.InterfaceBindingMethod.Slirp != nil || | ||
iface.InterfaceBindingMethod.Masquerade != nil || | ||
iface.InterfaceBindingMethod.SRIOV != nil || | ||
iface.InterfaceBindingMethod.Macvtap != nil || | ||
iface.InterfaceBindingMethod.Passt != nil | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,53 @@ | ||
/* | ||
* This file is part of the KubeVirt project | ||
* | ||
* Licensed under the Apache License, Version 2.0 (the "License"); | ||
* you may not use this file except in compliance with the License. | ||
* You may obtain a copy of the License at | ||
* | ||
* http://www.apache.org/licenses/LICENSE-2.0 | ||
* | ||
* Unless required by applicable law or agreed to in writing, software | ||
* distributed under the License is distributed on an "AS IS" BASIS, | ||
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
* See the License for the specific language governing permissions and | ||
* limitations under the License. | ||
* | ||
* Copyright 2023 Red Hat, Inc. | ||
* | ||
*/ | ||
|
||
package services | ||
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 add the header. 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. done. |
||
|
||
import ( | ||
"fmt" | ||
|
||
v1 "kubevirt.io/api/core/v1" | ||
|
||
"kubevirt.io/kubevirt/pkg/hooks" | ||
) | ||
|
||
func NetBindingPluginSidecarList(vmi *v1.VirtualMachineInstance, config *v1.KubeVirtConfiguration) (hooks.HookSidecarList, error) { | ||
var pluginSidecars hooks.HookSidecarList | ||
bindingByName := map[string]v1.InterfaceBindingPlugin{} | ||
for _, iface := range vmi.Spec.Domain.Devices.Interfaces { | ||
if iface.Binding != nil { | ||
var exist bool | ||
var pluginInfo v1.InterfaceBindingPlugin | ||
if config.NetworkConfiguration != nil && config.NetworkConfiguration.Binding != nil { | ||
pluginInfo, exist = config.NetworkConfiguration.Binding[iface.Binding.Name] | ||
bindingByName[iface.Binding.Name] = pluginInfo | ||
} | ||
|
||
if !exist { | ||
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 adding validation for adding binding plugin to Kubevirt CR, at least that the image is not empty. It may also reduce some error checking like this one if its validated at the API level. 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. That is not recomended, we are not validating the KV CR and we should not add such dependencies in the webhook for inter components dependencies. |
||
return nil, fmt.Errorf("couldn't find configuration for network bindining: %s", iface.Binding.Name) | ||
} | ||
} | ||
} | ||
for _, pluginInfo := range bindingByName { | ||
if pluginInfo.SidecarImage != "" { | ||
pluginSidecars = append(pluginSidecars, hooks.HookSidecar{Image: pluginInfo.SidecarImage}) | ||
} | ||
} | ||
return pluginSidecars, nil | ||
} |
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.
This may not work well when we'll need to work with tap devices (or other future exceptions).
But these thoughts are futuristic, for now this seem fine.