hooks: functional tests

.gitignore

@@ -27,5 +27,4 @@ _out
 vendor/**/*_test.go
 **/polarion.xml
 tools/manifest-templator/manifest-templator
-.coverprofile
 tools/vms-generator/vms-generator

cluster/examples/hook-sidecar-consumer.yml → cluster/examples/vm-with-sidecar-hook.yaml

@@ -1,16 +1,13 @@
-status: {}
 apiVersion: kubevirt.io/v1alpha1
 kind: VirtualMachine
 metadata:
-  creationTimestamp: null
-  labels:
-    special: vm-hook-sidecar-consumer
-  name: vm-hook-sidecar-consumer
   annotations:
-    # Request the hook sidecar
     hooks.kubevirt.io/hookSidecars: '[{"image": "registry:5000/kubevirt/example-hook-sidecar:devel"}]'
-    # Overwrite base board manufacturer name
-    smbios.vm.kubevirt.io/baseBoardManufacturer: "Radical Edward"
+    smbios.vm.kubevirt.io/baseBoardManufacturer: Radical Edward
+  creationTimestamp: null
+  labels:
+    special: vm-with-sidecar-hook
+  name: vm-with-sidecar-hook
 spec:
   domain:
     devices:
@@ -39,4 +36,4 @@ spec:
         password: fedora
         chpasswd: { expire: False }
     name: cloudinitvolume
-status: {}
+status: {}

cmd/example-hook-sidecar/Dockerfile

@@ -4,4 +4,4 @@ MAINTAINER "The KubeVirt Project" <[email protected]>
 
 COPY example-hook-sidecar /example-hook-sidecar
 
-ENTRYPOINT [ "/example-hook-sidecar" ]
+ENTRYPOINT [ "/example-hook-sidecar" ]

cmd/example-hook-sidecar/README.md

@@ -14,15 +14,14 @@ annotations:
 
 ```shell
 # Create a VM requesting the hook sidecar
-cluster/kubectl.sh create -f cluster/examples/hook-sidecar-consumer.yml
+cluster/kubectl.sh create -f cluster/examples/hook-with-sidecar-hook.yml
 
 # Once the VM is ready, connect to its display and login using name and password "fedora"
-cluster/virtctl.sh vnc vm-hook-sidecar-consumer
+cluster/virtctl.sh vnc vm-with-sidecar-hook
 
 # Install dmidecode
 sudo dnf install -y dmidecode
 
 # Check whether the base board manufacturer value was successfully overwritten
 sudo dmidecode -s baseboard-manufacturer
 ```
-

cmd/example-hook-sidecar/smbios.go

@@ -41,7 +41,7 @@ func (s infoServer) Info(ctx context.Context, params *hooksInfo.InfoParams) (*ho
 type v1alpha1Server struct{}
 
 func (s v1alpha1Server) OnDefineDomain(ctx context.Context, params *hooksV1alpha1.OnDefineDomainParams) (*hooksV1alpha1.OnDefineDomainResult, error) {
-	log.Log.Info("Hook's OnDefinedComain callback method has been called")
+	log.Log.Info("Hook's OnDefineDomain callback method has been called")
 
 	vmJSON := params.GetVm()
 	vmSpec := vmSchema.VirtualMachine{}

cmd/virt-launcher/virt-launcher.go

@@ -50,6 +50,7 @@ import (
 
 const defaultStartTimeout = 3 * time.Minute
 const defaultWatchdogInterval = 5 * time.Second
+const hookSidecarsCollectTimeout = 10 * time.Second
 
 func markReady(readinessFile string) {
 	f, err := os.OpenFile(readinessFile, os.O_RDONLY|os.O_CREATE, 0666)
@@ -255,7 +256,7 @@ func main() {
 	readinessFile := flag.String("readiness-file", "/tmp/health", "Pod looks for this file to determine when virt-launcher is initialized")
 	gracePeriodSeconds := flag.Int("grace-period-seconds", 30, "Grace period to observe before sending SIGTERM to vm process")
 	useEmulation := flag.Bool("use-emulation", false, "Use software emulation")
-	requestedHooks := flag.Uint("requested-hooks", 0, "Number of requested hooks, virt-launcher will wait for all of them to become available")
+	hookSidecars := flag.Uint("hook-sidecars", 0, "Number of requested hook sidecars, virt-launcher will wait for all of them to become available")
 
 	// set new default verbosity, was set to 0 by glog
 	flag.Set("v", "2")
@@ -267,7 +268,7 @@ func main() {
 
 	// Block until all requested hookSidecars are ready
 	hookManager := hooks.GetManager()
-	err := hookManager.Collect(*requestedHooks)
+	err := hookManager.Collect(*hookSidecars, hookSidecarsCollectTimeout)
 	if err != nil {
 		panic(err)
 	}

glide.yaml

@@ -63,7 +63,7 @@ import:
 - package: github.com/ant31/crd-validation
   version: eabcf70a1bd73e9296fa0c5f57de604689200a1b
 - package: kubevirt.io/qe-tools
-  version: v0.1.1
+  version: 13fc8573324cd2af42c88043b5a7c6e03bf39f95
   repo: https://github.com/kubevirt/qe-tools.git
 - package: github.com/golang/protobuf
   subpackages:

pkg/hooks/info/api.proto

@@ -22,4 +22,4 @@ message HookPoint {
   string name = 1;
   // priority is used to sort hooks prior to their execution (second key is the name)
   int32 priority = 2;
-}
+}

pkg/hooks/manager.go

@@ -32,19 +32,18 @@ var manager *Manager
 var once sync.Once
 
 type Manager struct {
-	collected             bool
 	callbacksPerHookPoint map[string][]*callackClient
 }
 
 func GetManager() *Manager {
 	once.Do(func() {
-		manager = &Manager{collected: false}
+		manager = &Manager{callbacksPerHookPoint: make(map[string][]*callackClient)}
 	})
 	return manager
 }
 
-func (m *Manager) Collect(numberOfRequestedHookSidecars uint) error {
-	callbacksPerHookPoint, err := collectSideCarSockets(numberOfRequestedHookSidecars)
+func (m *Manager) Collect(numberOfRequestedHookSidecars uint, timeout time.Duration) error {
+	callbacksPerHookPoint, err := collectSideCarSockets(numberOfRequestedHookSidecars, timeout)
 	if err != nil {
 		return err
 	}
@@ -53,41 +52,48 @@ func (m *Manager) Collect(numberOfRequestedHookSidecars uint) error {
 	sortCallbacksPerHookPoint(callbacksPerHookPoint)
 	log.Log.Infof("Sorted all collected sidecar sockets per hook point based on their priority and name: %v", callbacksPerHookPoint)
 
-	m.collected = true
 	m.callbacksPerHookPoint = callbacksPerHookPoint
 
 	return nil
 }
 
-func collectSideCarSockets(numberOfRequestedHookSidecars uint) (map[string][]*callackClient, error) {
+// TODO: Handle sockets in parallel, when a socket appears, run a goroutine trying to read Info from it
+func collectSideCarSockets(numberOfRequestedHookSidecars uint, timeout time.Duration) (map[string][]*callackClient, error) {
 	callbacksPerHookPoint := make(map[string][]*callackClient)
 	processedSockets := make(map[string]bool)
 
+	timeoutCh := time.After(timeout)
+
 	for uint(len(processedSockets)) < numberOfRequestedHookSidecars {
 		sockets, err := ioutil.ReadDir(HookSocketsSharedDirectory)
 		if err != nil {
 			return nil, err
 		}
 
 		for _, socket := range sockets {
-			if _, processed := processedSockets[socket.Name()]; processed {
-				continue
-			}
+			select {
+			case <-timeoutCh:
+				return nil, fmt.Errorf("Failed to collect all expected sidecar hook sockets within given timeout")
+			default:
+				if _, processed := processedSockets[socket.Name()]; processed {
+					continue
+				}
 
-			callackClient, notReady, err := processSideCarSocket(HookSocketsSharedDirectory + "/" + socket.Name())
-			if notReady {
-				log.Log.Info("Sidecar server might not be ready yet, retrying in the next iteration")
-				continue
-			} else if err != nil {
-				log.Log.Reason(err).Infof("Failed to process sidecar socket: %s", socket.Name())
-				return nil, err
-			}
+				callackClient, notReady, err := processSideCarSocket(HookSocketsSharedDirectory + "/" + socket.Name())
+				if notReady {
+					log.Log.Info("Sidecar server might not be ready yet, retrying in the next iteration")
+					continue
+				} else if err != nil {
+					log.Log.Reason(err).Infof("Failed to process sidecar socket: %s", socket.Name())
+					return nil, err
+				}
 
-			for _, subsribedHookPoint := range callackClient.subsribedHookPoints {
-				callbacksPerHookPoint[subsribedHookPoint.GetName()] = append(callbacksPerHookPoint[subsribedHookPoint.GetName()], callackClient)
-			}
+				for _, subsribedHookPoint := range callackClient.subsribedHookPoints {
+					callbacksPerHookPoint[subsribedHookPoint.GetName()] = append(callbacksPerHookPoint[subsribedHookPoint.GetName()], callackClient)
+				}
 
-			processedSockets[socket.Name()] = true
+				processedSockets[socket.Name()] = true
+			}
 		}
 
 		time.Sleep(time.Second)
@@ -105,7 +111,9 @@ func processSideCarSocket(socketPath string) (*callackClient, bool, error) {
 	defer conn.Close()
 
 	infoClient := hooksInfo.NewInfoClient(conn)
-	info, err := infoClient.Info(context.Background(), &hooksInfo.InfoParams{})
+	ctx, cancel := context.WithTimeout(context.Background(), time.Second)
+	defer cancel()
+	info, err := infoClient.Info(ctx, &hooksInfo.InfoParams{})
 	if err != nil {
 		return nil, false, err
 	}
@@ -141,10 +149,6 @@ func sortCallbacksPerHookPoint(callbacksPerHookPoint map[string][]*callackClient
 }
 
 func (m *Manager) OnDefineDomain(domainSpec *virtwrapApi.DomainSpec, vm *v1.VirtualMachine) (*virtwrapApi.DomainSpec, error) {
-	if !m.collected {
-		return nil, fmt.Errorf("Hook sidecars have not been collected yet")
-	}
-
 	if callbacks, found := m.callbacksPerHookPoint[hooksInfo.OnDefineDomainHookPointName]; found {
 		for _, callback := range callbacks {
 			if callback.Version == hooksV1alpha1.Version {
@@ -166,7 +170,9 @@ func (m *Manager) OnDefineDomain(domainSpec *virtwrapApi.DomainSpec, vm *v1.Virt
 
 				client := hooksV1alpha1.NewCallbacksClient(conn)
 
-				result, err := client.OnDefineDomain(context.Background(), &hooksV1alpha1.OnDefineDomainParams{
+				ctx, cancel := context.WithTimeout(context.Background(), time.Second)
+				defer cancel()
+				result, err := client.OnDefineDomain(ctx, &hooksV1alpha1.OnDefineDomainParams{
 					DomainXML: domainSpecXML,
 					Vm:        vmJSON,
 				})
@@ -198,5 +204,6 @@ func dialSocket(socketPath string) (*grpc.ClientConn, error) {
 		grpc.WithDialer(func(addr string, timeout time.Duration) (net.Conn, error) {
 			return net.DialTimeout("unix", addr, timeout)
 		}),
+		grpc.WithTimeout(time.Second),
 	)
 }

pkg/hooks/v1alpha1/api.proto

@@ -16,4 +16,4 @@ message OnDefineDomainParams {
 message OnDefineDomainResult {
   // domainXML is processed libvirt domain specification
   bytes domainXML = 1;
-}
+}

pkg/virt-controller/services/template.go

@@ -219,13 +219,13 @@ func (t *templateService) RenderLaunchManifest(vmi *v1.VirtualMachineInstance) (
 
 	if len(requestedHookSidecarList) != 0 {
 		volumes = append(volumes, k8sv1.Volume{
-			Name: "hooks",
+			Name: "hook-sidecar-sockets",
 			VolumeSource: k8sv1.VolumeSource{
 				EmptyDir: &k8sv1.EmptyDirVolumeSource{},
 			},
 		})
 		volumesMounts = append(volumesMounts, k8sv1.VolumeMount{
-			Name:      "hooks",
+			Name:      "hook-sidecar-sockets",
 			MountPath: hooks.HookSocketsSharedDirectory,
 		})
 	}
@@ -237,7 +237,7 @@ func (t *templateService) RenderLaunchManifest(vmi *v1.VirtualMachineInstance) (
 		"--kubevirt-share-dir", t.virtShareDir,
 		"--readiness-file", "/tmp/healthy",
 		"--grace-period-seconds", strconv.Itoa(int(gracePeriodSeconds)),
-		"--requested-hooks", strconv.Itoa(len(requestedHookSidecarList)),
+		"--hook-sidecars", strconv.Itoa(len(requestedHookSidecarList)),
 	}
 
 	useEmulation, err := IsEmulationAllowed(t.store)
@@ -337,7 +337,7 @@ func (t *templateService) RenderLaunchManifest(vmi *v1.VirtualMachineInstance) (
 			ImagePullPolicy: requestedHookSidecar.ImagePullPolicy,
 			VolumeMounts: []k8sv1.VolumeMount{
 				k8sv1.VolumeMount{
-					Name:      "hooks",
+					Name:      "hook-sidecar-sockets",
 					MountPath: hooks.HookSocketsSharedDirectory,
 				},
 			},

pkg/virt-controller/services/template_test.go

@@ -76,8 +76,8 @@ var _ = Describe("Template", func() {
 					"--kubevirt-share-dir", "/var/run/kubevirt",
 					"--readiness-file", "/tmp/healthy",
 					"--grace-period-seconds", "45",
-					"--requested-hooks", "1"}))
-				Expect(pod.Spec.Containers[1].Name).To(Equal("hook-sidecar-some-image:v1"))
+					"--hook-sidecars", "1"}))
+				Expect(pod.Spec.Containers[1].Name).To(Equal("hook-sidecar-0"))
 				Expect(pod.Spec.Containers[1].Image).To(Equal("some-image:v1"))
 				Expect(pod.Spec.Containers[1].ImagePullPolicy).To(Equal(kubev1.PullPolicy("IfNotPresent")))
 				Expect(*pod.Spec.TerminationGracePeriodSeconds).To(Equal(int64(60)))
@@ -119,8 +119,8 @@ var _ = Describe("Template", func() {
 					"--kubevirt-share-dir", "/var/run/kubevirt",
 					"--readiness-file", "/tmp/healthy",
 					"--grace-period-seconds", "45",
-					"--requested-hooks", "1"}))
-				Expect(pod.Spec.Containers[1].Name).To(Equal("hook-sidecar-some-image:v1"))
+					"--hook-sidecars", "1"}))
+				Expect(pod.Spec.Containers[1].Name).To(Equal("hook-sidecar-0"))
 				Expect(pod.Spec.Containers[1].Image).To(Equal("some-image:v1"))
 				Expect(pod.Spec.Containers[1].ImagePullPolicy).To(Equal(kubev1.PullPolicy("IfNotPresent")))
 				Expect(pod.Spec.Containers[1].VolumeMounts[0].MountPath).To(Equal(hooks.HookSocketsSharedDirectory))