merge

neonvm-daemon/cmd/main.go

@@ -102,10 +102,15 @@ func (s *cpuServer) handleSetCPUStatus(w http.ResponseWriter, r *http.Request) {
 	w.WriteHeader(http.StatusOK)
 }
 
-func (s *cpuServer) handleGetFileChecksum(w http.ResponseWriter, path string) {
+func (s *cpuServer) handleGetFileChecksum(w http.ResponseWriter, r *http.Request, path string) {
 	s.fileOperationsMutex.Lock()
 	defer s.fileOperationsMutex.Unlock()
 
+	if err := r.Context().Err(); err != nil {
+		w.WriteHeader(http.StatusRequestTimeout)
+		return
+	}
+
 	dir := filepath.Join(path, "..data")
 	checksum, err := util.ChecksumFlatDir(dir)
 	if err != nil {
@@ -129,6 +134,11 @@ func (s *cpuServer) handleUploadFile(w http.ResponseWriter, r *http.Request, pat
 	s.fileOperationsMutex.Lock()
 	defer s.fileOperationsMutex.Unlock()
 
+	if err := r.Context().Err(); err != nil {
+		w.WriteHeader(http.StatusRequestTimeout)
+		return
+	}
+
 	if r.Body == nil {
 		s.logger.Error("no body")
 		w.WriteHeader(http.StatusBadRequest)
@@ -200,7 +210,7 @@ func (s *cpuServer) run(addr string) {
 	mux.HandleFunc("/files/{path...}", func(w http.ResponseWriter, r *http.Request) {
 		path := fmt.Sprintf("/%s", r.PathValue("path"))
 		if r.Method == http.MethodGet {
-			s.handleGetFileChecksum(w, path)
+			s.handleGetFileChecksum(w, r, path)
 			return
 		} else if r.Method == http.MethodPut {
 			s.handleUploadFile(w, r, path)

neonvm-runner/cmd/disks.go

@@ -187,7 +187,7 @@ func createISO9660runtime(
 			if disk.MountPath != "" {
 				mounts = append(mounts, fmt.Sprintf(`/neonvm/bin/mkdir -p %s`, disk.MountPath))
 			}
-			if diskNeedsSynchronisation(disk) {
+			if disk.Watch != nil && *disk.Watch {
 				// do nothing as we will mount it into the VM via neonvm-daemon later
 				continue
 			}
@@ -278,22 +278,6 @@ func createISO9660runtime(
 	return nil
 }
 
-func diskNeedsSynchronisation(disk vmv1.Disk) bool {
-	if disk.DiskSource.Secret == nil {
-		// only supporting secrets at the moment
-		return false
-	}
-
-	// for now, `/var/sync` is our sentinal that this is synchronised.
-	// TODO: should we instead put a sync flag on the Disk object itself?
-	rel, err := filepath.Rel("/var/sync", disk.MountPath)
-	if err != nil {
-		return false
-	}
-
-	return filepath.IsLocal(rel)
-}
-
 func calcDirUsage(dirPath string) (int64, error) {
 	stat, err := os.Lstat(dirPath)
 	if err != nil {

neonvm-runner/cmd/main.go

@@ -673,25 +673,34 @@ func monitorFiles(ctx context.Context, logger *zap.Logger, wg *sync.WaitGroup, v
 	defer wg.Done()
 
 	secrets := make(map[string]string)
+	secretsOrd := []string{}
 	for _, disk := range vmSpec.Disks {
-		if diskNeedsSynchronisation(disk) {
-			// secrets are mounted using the atomicwriter utility, which loads the secret directory
-			// into `..data`.
+		if disk.Watch != nil && *disk.Watch {
+			// secrets/configmaps are mounted using the atomicwriter utility,
+			// which loads the directory into `..data`.
 			dataDir := fmt.Sprintf("/vm/mounts%s/..data", disk.MountPath)
 			secrets[dataDir] = disk.MountPath
+			secretsOrd = append(secretsOrd, dataDir)
 		}
 	}
 
+	// implicit watched disk added for TLS certificate
 	if vmSpec.TlsCertificateIssuer != nil {
 		secrets["/vm/mounts/var/tls/..data"] = "/var/tls"
+		secretsOrd = append(secretsOrd, "/vm/mounts/var/tls/..data")
+	}
+
+	if len(secretsOrd) == 0 {
+		return
 	}
 
 	// Faster loop for the initial upload.
 	// The VM might need the secrets in order for postgres to actually start up,
 	// so it's important we sync them as soon as the daemon is available.
 	for {
 		success := true
-		for hostpath, guestpath := range secrets {
+		for _, hostpath := range secretsOrd {
+			guestpath := secrets[hostpath]
 			if err := sendFilesToNeonvmDaemon(ctx, hostpath, guestpath); err != nil {
 				success = false
 				logger.Error("failed to upload file to vm guest", zap.Error(err))
@@ -703,11 +712,14 @@ func monitorFiles(ctx context.Context, logger *zap.Logger, wg *sync.WaitGroup, v
 
 		select {
 		case <-time.After(1 * time.Second):
+			continue
 		case <-ctx.Done():
-			logger.Warn("QEMU shut down too soon to start forwarding logs")
+			return
 		}
 	}
 
+	// For the entire duration the VM is alive, periodically check whether any of the watched disks
+	// still match what's inside the VM, and if not, send the update.
 	ticker := time.NewTicker(30 * time.Second)
 	defer ticker.Stop()
 

neonvm/apis/neonvm/v1/virtualmachine_types.go

@@ -513,6 +513,12 @@ type Disk struct {
 	// Path within the virtual machine at which the disk should be mounted.  Must
 	// not contain ':'.
 	MountPath string `json:"mountPath"`
+	// The disk source is monitored for changes if true, otherwise it is only read on VM startup (false or unspecified).
+	// This only works if the disk source is a configmap, a secret, or a projected volume.
+	// Defaults to false.
+	// +optional
+	// +kubebuilder:default:=false
+	Watch *bool `json:"watch,omitempty"`
 	// DiskSource represents the location and type of the mounted disk.
 	DiskSource `json:",inline"`
 }

neonvm/config/crd/bases/vm.neon.tech_virtualmachines.yaml

@@ -1189,6 +1189,13 @@ spec:
                       required:
                       - size
                       type: object
+                    watch:
+                      default: false
+                      description: |-
+                        The disk source is monitored for changes if true, otherwise it is only read on VM startup (false or unspecified).
+                        This only works if the disk source is a configmap, a secret, or a projected volume.
+                        Defaults to false.
+                      type: boolean
                   required:
                   - mountPath
                   - name

pkg/util/checksum.go

@@ -24,20 +24,31 @@ func ChecksumFlatDir(path string) (string, error) {
 	}
 	sort.Strings(keys)
 
+	// note: any changes to the hash need to be sychronised between neonvm-runner and neonvm-daemon.
+	// Since they are updated independantly, this is not trivial.
+	// If in doubt, make a new function and don't touch this one.
 	hasher, err := blake2b.New256(nil)
 	if err != nil {
 		return "", err
 	}
 
 	for _, filename := range keys {
 		data := files[filename]
-		var length []byte
 
-		// hash as "{name}\0{len(data)}{data}"
-		// this prevents any possible hash confusion problems
+		// File hash with the following encoding: "{name}\0{len(data)}{data}".
+		//
+		// This format prevents any possible (even if unrealistic) hash confusion problems.
+		// If we only hashed filename and data, then there's no difference between:
+		// 	 name = "file1"
+		// 	 data = []
+		// and
+		//   name = "file"
+		//   data = [b'1']
+		//
+		// We are trusting that filenames on linux cannot have a nul character.
 		hasher.Write([]byte(filename))
 		hasher.Write([]byte{0})
-		hasher.Write(binary.LittleEndian.AppendUint64(length, uint64(len(data))))
+		hasher.Write(binary.LittleEndian.AppendUint64([]byte{}, uint64(len(data))))
 		hasher.Write(data)
 	}
 

tests/e2e/vm-secret-sync/00-create-vm.yaml

@@ -7,6 +7,7 @@ kind: Secret
 metadata:
   name: example-secret
 data:
+  # "hello world"
   foo: aGVsbG8gd29ybGQ=
 ---
 apiVersion: vm.neon.tech/v1
@@ -34,3 +35,4 @@ spec:
       secretName: example-secret
     mountPath: /var/sync/example
     name: secret-foo
+    watch: true

tests/e2e/vm-secret-sync/01-update-secret.yaml

@@ -7,4 +7,5 @@ kind: Secret
 metadata:
   name: example-secret
 data:
+  # "goodbye world"
   foo: Z29vZGJ5ZSB3b3JsZA==