-
Notifications
You must be signed in to change notification settings - Fork 2
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
[traffic-gen-in-vm] Attach traffic-gen's ConfigMap to traffic gen VMI #148
[traffic-gen-in-vm] Attach traffic-gen's ConfigMap to traffic gen VMI #148
Conversation
8032aee
to
86edecb
Compare
Change: Rebase |
In order to attach the TRex ConfigMap to the traffic gen VMI: 1. Add a function to attach a volume pointing to an existing ConfigMap 2. Add a function to add a disk pointing to the ConfigMap volume. Signed-off-by: Orel Misan <[email protected]>
In order to mount the files from the TRex ConfigMap, additional boot commands should be passed in cloudInit. Enable the addition of boot commands to the function generating the cloud init string. Signed-off-by: Orel Misan <[email protected]>
86edecb
to
8b04d52
Compare
Change: Rebase |
Tested against an OpenShift Virtualization 4.14 cluster. |
func WithConfigMapDisk(name, serial string) Option { | ||
return func(vmi *kvcorev1.VirtualMachineInstance) { | ||
vmi.Spec.Domain.Devices.Disks = append(vmi.Spec.Domain.Devices.Disks, | ||
kvcorev1.Disk{ |
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.
When I ran it on the GUI it chose the sata
bus. AFAIK it is not a default value so maybe we should hard code it here.
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.
Some component in KubeVirt added the sata
bus automatically.
The example doesn't use sata
[1].
[1] http://kubevirt.io/user-guide/virtual_machines/disks_and_volumes/#configmap
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.
The example is U/S. When I tried it on my D/S it didn't seem to work.
On the other hand I didn't really find a D/S document on it, so I rely on what the D/S GUI gave me as the "practical documentation"
In the end - if it works and the file is propagated, then maybe I did something wrong on my manual try, so we can continue, but please make sure it works on the D/S cluster.
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.
It works on the SNO.
@@ -70,13 +70,15 @@ func New(client kubeVirtVMIClient, namespace string, checkupConfig config.Config | |||
const randomStringLen = 5 | |||
randomSuffix := rand.String(randomStringLen) | |||
|
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.
commit typo: ConfigMao
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.
Thanks.
Done.
@@ -65,15 +65,23 @@ func newVMIUnderTest(name string, checkupConfig config.Config) *kvcorev1.Virtual | |||
return vmi.New(name, optionsToApply...) | |||
} | |||
|
|||
func newTrafficGen(name string, checkupConfig config.Config) *kvcorev1.VirtualMachineInstance { | |||
func newTrafficGen(name string, checkupConfig config.Config, configMapName string) *kvcorev1.VirtualMachineInstance { | |||
const configDiskSerial = "DEADBEEF" |
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.
nit: DEADBEEF is usually used when you can only use HEX letters, so if you want you have more options..
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.
I don't have a smarter idea.
Feel free to suggest another value ;)
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.
randomize?
again, only a nit, so I wouldn't mind keeping it the way it is..
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.
We can always change it.
It has no affect on the user.
Pass the name of the ConfigMap when creating the traffic gen VMI. Use it to attach the ConfigMap volume. Add a disk to the traffic gen VMI, which will have the files from the ConfigMap. Add the required cloud init boot commands to mount the above disk [1]. [1] http://kubevirt.io/user-guide/virtual_machines/disks_and_volumes/#configmap Signed-off-by: Orel Misan <[email protected]>
8b04d52
to
142c2fd
Compare
Change: Fixed a typo in the last commit message. |
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.
/lgtm
/approve
In order to get the Trex configuration, the ConfigMap object generated by the checkup, should be attached to the traffic-gen VMI, and mounted inside the guest [1].
The generated ConfigMap is currently empty, until a follow-up PR will fill it up.
Based on PR #147, please review only the last three commits.[1] http://kubevirt.io/user-guide/virtual_machines/disks_and_volumes/#configmap