airship/vino
Code Issues Proposed changes

Make CRD networks plural

Browse Source 
This makes the Vino CR networks section plural, rather than being
a single network.  Likewise it makes each network's routes plural.

This also moves the VM bridge definition out of `networks`, because
it needs to be exactly one interface name regardless of how many
networks you have.

Change-Id: I0618fcbcc4e315e5c74c5af2cd85864423ec089b
This commit is contained in:
Matt McEuen 2021-02-24 17:54:24 -06:00
parent 2f125cc98b
commit b1bef49d57
8 changed files with 101 additions and 61 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
README.md
View File

@ -136,7 +136,8 @@ vino-system vino-controller-manager-788b994c74-sbf26 2/2 Running 0
```
#### Configure the VM Bridge Interface
To configure the bare metal networking interface that should be used for the VM Bridge,
please specify it in your vino CR at field spec.networks.vmInterfaceName
please specify it in your vino CR at field spec.vmBridge.
#### Test basic functionality
```

57
config/crd/bases/airship.airshipit.org_vinoes.yaml
View File

@ -66,33 +66,34 @@ spec:
type: object
networks:
description: Define network parameters
properties:
allocationStart:
type: string
allocationStop:
type: string
dns_servers:
items:
items:
description: Network defines libvirt networks
properties:
allocationStart:
type: string
type: array
name:
description: Network Parameter defined
type: string
routes:
description: VMRoutes defined
properties:
to:
allocationStop:
type: string
dns_servers:
items:
type: string
via:
type: string
type: object
subnet:
type: string
vmInterfaceName:
description: VMinterfaceName defines the interface name to be used
as bridge for virtual machines
type: string
type: object
type: array
name:
description: Network Parameter defined
type: string
routes:
items:
description: VMRoutes defined
properties:
to:
type: string
via:
type: string
type: object
type: array
subnet:
type: string
type: object
type: array
nodeSelector:
description: Define nodelabel parameters
properties:
@ -167,6 +168,12 @@ spec:
type: object
type: object
type: array
vmBridge:
description: VMBridge defines the single interface name to be used as
a bridge for VMs
type: string
required:
- vmBridge
type: object
status:
description: VinoStatus defines the observed state of Vino

17
config/samples/vino_cr.yaml
View File

@ -12,4 +12,19 @@ spec:
redfishCredentialSecret:
name: redfishSecret
networks:
vmInterfaceName: lo
- name: management
subnet: 192.168.2.0/20
allocationStart: 192.168.2.10
allocationStop: 192.168.2.14 # docs should specify that the range should = number of vms (to permit future expansion over multiple vino crs etc)
routes:
- to: 10.0.0.0/24
via: $vino.nodebridgegw # vino will need to populate this from the nodelabel value `airshipit.org/vino.nodebridgegw`
dns_servers: ["135.188.34.124"]
- name: external
subnet: 169.0.0.0/24
routes:
- to: 0.0.0.0/0
via: 169.0.0.1
allocationStart: 169.0.0.10
allocationStop: 169.0.0.254
vmBridge: lo

39
docs/api/vino.md
View File

@ -487,24 +487,13 @@ string
<code>routes</code><br>
<em>
<a href="#airship.airshipit.org/v1.VMRoutes">
VMRoutes
[]VMRoutes
</a>
</em>
</td>
<td>
</td>
</tr>
<tr>
<td>
<code>vmInterfaceName</code><br>
<em>
string
</em>
</td>
<td>
<p>VMinterfaceName defines the interface name to be used as bridge for virtual machines</p>
</td>
</tr>
</tbody>
</table>
</div>
@ -887,7 +876,7 @@ CPUConfiguration
<code>networks</code><br>
<em>
<a href="#airship.airshipit.org/v1.Network">
Network
[]Network
</a>
</em>
</td>
@ -921,6 +910,17 @@ DaemonSetOptions
<p>DaemonSetOptions defines how vino will spawn daemonset on nodes</p>
</td>
</tr>
<tr>
<td>
<code>vmBridge</code><br>
<em>
string
</em>
</td>
<td>
<p>VMBridge defines the single interface name to be used as a bridge for VMs</p>
</td>
</tr>
</table>
</td>
</tr>
@ -988,7 +988,7 @@ CPUConfiguration
<code>networks</code><br>
<em>
<a href="#airship.airshipit.org/v1.Network">
Network
[]Network
</a>
</em>
</td>
@ -1022,6 +1022,17 @@ DaemonSetOptions
<p>DaemonSetOptions defines how vino will spawn daemonset on nodes</p>
</td>
</tr>
<tr>
<td>
<code>vmBridge</code><br>
<em>
string
</em>
</td>
<td>
<p>VMBridge defines the single interface name to be used as a bridge for VMs</p>
</td>
</tr>
</tbody>
</table>
</div>

18
pkg/api/v1/vino_types.go
View File

@ -42,11 +42,13 @@ type VinoSpec struct {
// Define CPU configuration
CPUConfiguration *CPUConfiguration `json:"configuration,omitempty"`
// Define network parameters
Network Network `json:"networks,omitempty"`
Networks []Network `json:"networks,omitempty"`
// Define node details
Node []NodeSet `json:"nodes,omitempty"`
// DaemonSetOptions defines how vino will spawn daemonset on nodes
DaemonSetOptions DaemonSetOptions `json:"daemonSetOptions,omitempty"`
// VMBridge defines the single interface name to be used as a bridge for VMs
VMBridge string `json:"vmBridge"`
}
// NodeSelector identifies nodes to create VMs on
@ -64,14 +66,12 @@ type CPUConfiguration struct {
// Network defines libvirt networks
type Network struct {
//Network Parameter defined
Name string `json:"name,omitempty"`
SubNet string `json:"subnet,omitempty"`
AllocationStart string `json:"allocationStart,omitempty"`
AllocationStop string `json:"allocationStop,omitempty"`
DNSServers []string `json:"dns_servers,omitempty"`
Routes *VMRoutes `json:"routes,omitempty"`
// VMinterfaceName defines the interface name to be used as bridge for virtual machines
VMInterfaceName string `json:"vmInterfaceName,omitempty"`
Name string `json:"name,omitempty"`
SubNet string `json:"subnet,omitempty"`
AllocationStart string `json:"allocationStart,omitempty"`
AllocationStop string `json:"allocationStop,omitempty"`
DNSServers []string `json:"dns_servers,omitempty"`
Routes []VMRoutes `json:"routes,omitempty"`
}
// VMRoutes defined

12
pkg/api/v1/zz_generated.deepcopy.go
View File

@ -215,8 +215,8 @@ func (in *Network) DeepCopyInto(out *Network) {
}
if in.Routes != nil {
in, out := &in.Routes, &out.Routes
*out = new(VMRoutes)
**out = **in
*out = make([]VMRoutes, len(*in))
copy(*out, *in)
}
}
@ -429,7 +429,13 @@ func (in *VinoSpec) DeepCopyInto(out *VinoSpec) {
*out = new(CPUConfiguration)
**out = **in
}
in.Network.DeepCopyInto(&out.Network)
if in.Networks != nil {
in, out := &in.Networks, &out.Networks
*out = make([]Network, len(*in))
for i := range *in {
(*in)[i].DeepCopyInto(&(*out)[i])
}
}
if in.Node != nil {
in, out := &in.Node, &out.Node
*out = make([]NodeSet, len(*in))

6
pkg/controllers/vino_controller.go
View File

@ -524,7 +524,7 @@ func (r *VinoReconciler) decorateDaemonSet(ctx context.Context, ds *appsv1.Daemo
// TODO develop logic to derive all required ENV variables from VINO CR, and pass them
// to setENV function instead
if vino.Spec.Network.VMInterfaceName != "" {
if vino.Spec.VMBridge != "" {
setEnv(ctx, ds, vino)
}
@ -546,7 +546,7 @@ func setEnv(ctx context.Context, ds *appsv1.DaemonSet, vino *vinov1.Vino) {
"container name", c.Name,
"value", envVar.Value,
)
ds.Spec.Template.Spec.Containers[i].Env[j].Value = vino.Spec.Network.VMInterfaceName
ds.Spec.Template.Spec.Containers[i].Env[j].Value = vino.Spec.VMBridge
set = true
break
}
@ -555,7 +555,7 @@ func setEnv(ctx context.Context, ds *appsv1.DaemonSet, vino *vinov1.Vino) {
ds.Spec.Template.Spec.Containers[i].Env = append(
ds.Spec.Template.Spec.Containers[i].Env, corev1.EnvVar{
Name: vinov1.EnvVarVMInterfaceName,
Value: vino.Spec.Network.VMInterfaceName,
Value: vino.Spec.VMBridge,
},
)
}

10
pkg/controllers/vino_controller_test.go
View File

@ -24,7 +24,7 @@ func testVINO() *vinov1.Vino {
return &vinov1.Vino{
ObjectMeta: v1.ObjectMeta{},
Spec: vinov1.VinoSpec{
Network: vinov1.Network{}}}
Networks: []vinov1.Network{}}}
}
var _ = Describe("Test Setting Env variables", func() {
@ -43,7 +43,7 @@ var _ = Describe("Test Setting Env variables", func() {
It("sets env interface variable to every container", func() {
vino := testVINO()
ifName := "eth0"
vino.Spec.Network.VMInterfaceName = ifName
vino.Spec.VMBridge = ifName
ds := testDS()
ds.Spec.Template.Spec.Containers = make([]corev1.Container, 3)
@ -61,7 +61,7 @@ var _ = Describe("Test Setting Env variables", func() {
It("overrides that variable in the container", func() {
vino := testVINO()
ifName := "eth0"
vino.Spec.Network.VMInterfaceName = ifName
vino.Spec.VMBridge = ifName
ds := testDS()
ds.Spec.Template.Spec.Containers = []corev1.Container{
{
@ -86,7 +86,7 @@ var _ = Describe("Test Setting Env variables", func() {
It("overrides that variable in the container", func() {
vino := testVINO()
ifName := "eth0"
vino.Spec.Network.VMInterfaceName = ifName
vino.Spec.VMBridge = ifName
ds := testDS()
ds.Spec.Template.Spec.Containers = []corev1.Container{
{
@ -138,7 +138,7 @@ var _ = Describe("Test Setting Env variables", func() {
It("it sets required variable only single time", func() {
vino := testVINO()
ifName := "eth0"
vino.Spec.Network.VMInterfaceName = ifName
vino.Spec.VMBridge = ifName
ds := testDS()
ds.Spec.Template.Spec.Containers = []corev1.Container{
{