From eeabe975ea71739eda8c3bfc1091852ce9492871 Mon Sep 17 00:00:00 2001 From: Valentin Rothberg Date: Wed, 22 Feb 2023 13:44:57 +0100 Subject: [PATCH 01/14] sqlite: implement pod methods [NO NEW TESTS NEEDED] - the sqlite backend is still in development and is not enabled by default. Signed-off-by: Valentin Rothberg --- libpod/sqlite_state.go | 960 +++++++++----------------------- libpod/sqlite_state_internal.go | 72 ++- 2 files changed, 322 insertions(+), 710 deletions(-) diff --git a/libpod/sqlite_state.go b/libpod/sqlite_state.go index 867d23c1f6..48fb368d45 100644 --- a/libpod/sqlite_state.go +++ b/libpod/sqlite_state.go @@ -1150,8 +1150,7 @@ func (s *SQLiteState) SafeRewriteContainerConfig(ctr *Container, oldName, newNam // RewritePodConfig rewrites a pod's configuration. // WARNING: This function is DANGEROUS. Do not use without reading the full // comment on this function in state.go. -// TODO TODO TODO -func (s *SQLiteState) RewritePodConfig(pod *Pod, newCfg *PodConfig) error { +func (s *SQLiteState) RewritePodConfig(pod *Pod, newCfg *PodConfig) (defErr error) { if !s.valid { return define.ErrDBClosed } @@ -1160,38 +1159,41 @@ func (s *SQLiteState) RewritePodConfig(pod *Pod, newCfg *PodConfig) error { return define.ErrPodRemoved } - return define.ErrNotImplemented - - // newCfgJSON, err := json.Marshal(newCfg) - // if err != nil { - // return fmt.Errorf("marshalling new configuration JSON for pod %s: %w", pod.ID(), err) - // } - - // db, err := s.getDBCon() - // if err != nil { - // return err - // } - // defer s.deferredCloseDBCon(db) + json, err := json.Marshal(newCfg) + if err != nil { + return fmt.Errorf("error marshalling pod %s config JSON: %w", pod.ID(), err) + } - // err = db.Update(func(tx *bolt.Tx) error { - // podBkt, err := getPodBucket(tx) - // if err != nil { - // return err - // } + tx, err := s.conn.Begin() + if err != nil { + return fmt.Errorf("beginning transaction to rewrite pod %s config: %w", pod.ID(), err) + } + defer func() { + if defErr != nil { + if err := tx.Rollback(); err != nil { + logrus.Errorf("Rolling back transaction to rewrite pod %s config: %v", pod.ID(), err) + } + } + }() - // podDB := podBkt.Bucket([]byte(pod.ID())) - // if podDB == nil { - // pod.valid = false - // return fmt.Errorf("no pod with ID %s found in DB: %w", pod.ID(), define.ErrNoSuchPod) - // } + results, err := tx.Exec("UPDATE PodConfig SET Name=?, JSON=? WHERE ID=?;", newCfg.Name, json, pod.ID()) + if err != nil { + return fmt.Errorf("updating pod config table with new configuration for pod %s: %w", pod.ID(), err) + } + rows, err := results.RowsAffected() + if err != nil { + return fmt.Errorf("retrieving pod %s config rewrite rows affected: %w", pod.ID(), err) + } + if rows == 0 { + pod.valid = false + return define.ErrNoSuchPod + } - // if err := podDB.Put(configKey, newCfgJSON); err != nil { - // return fmt.Errorf("updating pod %s config JSON: %w", pod.ID(), err) - // } + if err := tx.Commit(); err != nil { + return fmt.Errorf("committing transaction to rewrite pod %s config: %w", pod.ID(), err) + } - // return nil - // }) - // return err + return nil } // RewriteVolumeConfig rewrites a volume's configuration. @@ -1242,7 +1244,6 @@ func (s *SQLiteState) RewriteVolumeConfig(volume *Volume, newCfg *VolumeConfig) } // Pod retrieves a pod given its full ID -// TODO TODO TODO func (s *SQLiteState) Pod(id string) (*Pod, error) { if id == "" { return nil, define.ErrEmptyID @@ -1252,33 +1253,21 @@ func (s *SQLiteState) Pod(id string) (*Pod, error) { return nil, define.ErrDBClosed } - return nil, define.ErrNotImplemented - - // podID := []byte(id) - - // pod := new(Pod) - // pod.config = new(PodConfig) - // pod.state = new(podState) - - // db, err := s.getDBCon() - // if err != nil { - // return nil, err - // } - // defer s.deferredCloseDBCon(db) - - // err = db.View(func(tx *bolt.Tx) error { - // podBkt, err := getPodBucket(tx) - // if err != nil { - // return err - // } + row := s.conn.QueryRow("SELECT JSON FROM PodConfig WHERE ID=?;", id) + var rawJSON string + if err := row.Scan(&rawJSON); err != nil { + if errors.Is(err, sql.ErrNoRows) { + return nil, define.ErrNoSuchPod + } + return nil, fmt.Errorf("retrieving pod %s config from DB: %w", id, err) + } - // return s.getPodFromDB(podID, pod, podBkt) - // }) - // if err != nil { - // return nil, err - // } + ctrCfg := new(ContainerConfig) + if err := json.Unmarshal([]byte(rawJSON), ctrCfg); err != nil { + return nil, fmt.Errorf("unmarshalling container %s config: %w", id, err) + } - // return pod, nil + return s.createPod(rawJSON) } // LookupPod retrieves a pod from a full or unique partial ID, or a name. @@ -1313,24 +1302,10 @@ func (s *SQLiteState) LookupPod(idOrName string) (*Pod, error) { return nil, define.ErrNoSuchPod } - pod := new(Pod) - pod.config = new(PodConfig) - pod.state = new(podState) - pod.runtime = s.runtime - - if err := json.Unmarshal([]byte(rawJSON), pod.config); err != nil { - return nil, fmt.Errorf("unmarshalling pod JSON: %w", err) - } - - if err := finalizePodSqlite(pod); err != nil { - return nil, err - } - - return pod, nil + return s.createPod(rawJSON) } // HasPod checks if a pod with the given ID exists in the state -// TODO TODO TODO func (s *SQLiteState) HasPod(id string) (bool, error) { if id == "" { return false, define.ErrEmptyID @@ -1340,47 +1315,22 @@ func (s *SQLiteState) HasPod(id string) (bool, error) { return false, define.ErrDBClosed } - return false, define.ErrNotImplemented - - // podID := []byte(id) - - // exists := false - - // db, err := s.getDBCon() - // if err != nil { - // return false, err - // } - // defer s.deferredCloseDBCon(db) - - // err = db.View(func(tx *bolt.Tx) error { - // podBkt, err := getPodBucket(tx) - // if err != nil { - // return err - // } - - // podDB := podBkt.Bucket(podID) - // if podDB != nil { - // if s.namespaceBytes != nil { - // podNS := podDB.Get(namespaceKey) - // if bytes.Equal(s.namespaceBytes, podNS) { - // exists = true - // } - // } else { - // exists = true - // } - // } + row := s.conn.QueryRow("SELECT 1 FROM PodConfig WHERE ID=?;", id) - // return nil - // }) - // if err != nil { - // return false, err - // } + var check int + if err := row.Scan(&check); err != nil { + if errors.Is(err, sql.ErrNoRows) { + return false, nil + } + return false, fmt.Errorf("looking up pod %s in database: %w", id, err) + } else if check != 1 { + return false, fmt.Errorf("check digit for pod %s lookup incorrect: %w", id, define.ErrInternal) + } - // return exists, nil + return true, nil } // PodHasContainer checks if the given pod has a container with the given ID -// TODO TODO TODO func (s *SQLiteState) PodHasContainer(pod *Pod, id string) (bool, error) { if id == "" { return false, define.ErrEmptyID @@ -1394,59 +1344,21 @@ func (s *SQLiteState) PodHasContainer(pod *Pod, id string) (bool, error) { return false, define.ErrPodRemoved } - return false, define.ErrNotImplemented - - // ctrID := []byte(id) - // podID := []byte(pod.ID()) - - // exists := false - - // db, err := s.getDBCon() - // if err != nil { - // return false, err - // } - // defer s.deferredCloseDBCon(db) - - // err = db.View(func(tx *bolt.Tx) error { - // podBkt, err := getPodBucket(tx) - // if err != nil { - // return err - // } - - // // Get pod itself - // podDB := podBkt.Bucket(podID) - // if podDB == nil { - // pod.valid = false - // return fmt.Errorf("pod %s not found in database: %w", pod.ID(), define.ErrNoSuchPod) - // } - - // // Get pod containers bucket - // podCtrs := podDB.Bucket(containersBkt) - // if podCtrs == nil { - // return fmt.Errorf("pod %s missing containers bucket in DB: %w", pod.ID(), define.ErrInternal) - // } - - // // Don't bother with a namespace check on the container - - // // We maintain the invariant that container namespaces must - // // match the namespace of the pod they join. - // // We already checked the pod namespace, so we should be fine. - - // ctr := podCtrs.Get(ctrID) - // if ctr != nil { - // exists = true - // } - - // return nil - // }) - // if err != nil { - // return false, err - // } + var check int + row := s.conn.QueryRow("SELECT 1 FROM ContainerConfig WHERE ID=? AND PodID=?;", id, pod.ID()) + if err := row.Scan(&check); err != nil { + if errors.Is(err, sql.ErrNoRows) { + return false, nil + } + return false, fmt.Errorf("checking if pod %s has container %s in database: %w", pod.ID(), id, err) + } else if check != 1 { + return false, fmt.Errorf("check digit for pod %s lookup incorrect: %w", id, define.ErrInternal) + } - // return exists, nil + return true, nil } // PodContainersByID returns the IDs of all containers present in the given pod -// TODO TODO TODO func (s *SQLiteState) PodContainersByID(pod *Pod) ([]string, error) { if !s.valid { return nil, define.ErrDBClosed @@ -1456,62 +1368,26 @@ func (s *SQLiteState) PodContainersByID(pod *Pod) ([]string, error) { return nil, define.ErrPodRemoved } - return nil, define.ErrNotImplemented - - // if s.namespace != "" && s.namespace != pod.config.Namespace { - // return nil, fmt.Errorf("pod %s is in namespace %q but we are in namespace %q: %w", pod.ID(), pod.config.Namespace, s.namespace, define.ErrNSMismatch) - // } - - // podID := []byte(pod.ID()) - - // ctrs := []string{} - - // db, err := s.getDBCon() - // if err != nil { - // return nil, err - // } - // defer s.deferredCloseDBCon(db) - - // err = db.View(func(tx *bolt.Tx) error { - // podBkt, err := getPodBucket(tx) - // if err != nil { - // return err - // } - - // // Get pod itself - // podDB := podBkt.Bucket(podID) - // if podDB == nil { - // pod.valid = false - // return fmt.Errorf("pod %s not found in database: %w", pod.ID(), define.ErrNoSuchPod) - // } - - // // Get pod containers bucket - // podCtrs := podDB.Bucket(containersBkt) - // if podCtrs == nil { - // return fmt.Errorf("pod %s missing containers bucket in DB: %w", pod.ID(), define.ErrInternal) - // } - - // // Iterate through all containers in the pod - // err = podCtrs.ForEach(func(id, val []byte) error { - // ctrs = append(ctrs, string(id)) + rows, err := s.conn.Query("SELECT ID FROM ContainerConfig WHERE PodID=?;", pod.ID()) + if err != nil { + return nil, fmt.Errorf("retrieving container IDs of pod %s from database: %w", pod.ID(), err) + } + defer rows.Close() - // return nil - // }) - // if err != nil { - // return err - // } + var ids []string + for rows.Next() { + var id string + if err := rows.Scan(&id); err != nil { + return nil, fmt.Errorf("scanning container from database: %w", err) + } - // return nil - // }) - // if err != nil { - // return nil, err - // } + ids = append(ids, id) + } - // return ctrs, nil + return ids, nil } // PodContainers returns all the containers present in the given pod -// TODO TODO TODO func (s *SQLiteState) PodContainers(pod *Pod) ([]*Container, error) { if !s.valid { return nil, define.ErrDBClosed @@ -1521,67 +1397,42 @@ func (s *SQLiteState) PodContainers(pod *Pod) ([]*Container, error) { return nil, define.ErrPodRemoved } - return nil, define.ErrNotImplemented - - // podID := []byte(pod.ID()) - - // ctrs := []*Container{} - - // db, err := s.getDBCon() - // if err != nil { - // return nil, err - // } - // defer s.deferredCloseDBCon(db) - - // err = db.View(func(tx *bolt.Tx) error { - // podBkt, err := getPodBucket(tx) - // if err != nil { - // return err - // } - - // ctrBkt, err := getCtrBucket(tx) - // if err != nil { - // return err - // } + rows, err := s.conn.Query("SELECT JSON FROM ContainerConfig WHERE PodID=?;", pod.ID()) + if err != nil { + return nil, fmt.Errorf("retrieving containers of pod %s from database: %w", pod.ID(), err) + } + defer rows.Close() - // // Get pod itself - // podDB := podBkt.Bucket(podID) - // if podDB == nil { - // pod.valid = false - // return fmt.Errorf("pod %s not found in database: %w", pod.ID(), define.ErrNoSuchPod) - // } + var ctrs []*Container + for rows.Next() { + var rawJSON string + if err := rows.Scan(&rawJSON); err != nil { + return nil, fmt.Errorf("scanning container from database: %w", err) + } - // // Get pod containers bucket - // podCtrs := podDB.Bucket(containersBkt) - // if podCtrs == nil { - // return fmt.Errorf("pod %s missing containers bucket in DB: %w", pod.ID(), define.ErrInternal) - // } + ctr := new(Container) + ctr.config = new(ContainerConfig) + ctr.state = new(ContainerState) + ctr.runtime = s.runtime - // // Iterate through all containers in the pod - // err = podCtrs.ForEach(func(id, val []byte) error { - // newCtr := new(Container) - // newCtr.config = new(ContainerConfig) - // newCtr.state = new(ContainerState) - // ctrs = append(ctrs, newCtr) + if err := json.Unmarshal([]byte(rawJSON), ctr.config); err != nil { + return nil, fmt.Errorf("unmarshalling container config: %w", err) + } - // return s.getContainerFromDB(id, newCtr, ctrBkt, false) - // }) - // if err != nil { - // return err - // } + ctrs = append(ctrs, ctr) + } - // return nil - // }) - // if err != nil { - // return nil, err - // } + for _, ctr := range ctrs { + if err := finalizeCtrSqlite(ctr); err != nil { + return nil, err + } + } - // return ctrs, nil + return ctrs, nil } // AddPod adds the given pod to the state. -// TODO TODO TODO -func (s *SQLiteState) AddPod(pod *Pod) error { +func (s *SQLiteState) AddPod(pod *Pod) (defErr error) { if !s.valid { return define.ErrDBClosed } @@ -1590,129 +1441,46 @@ func (s *SQLiteState) AddPod(pod *Pod) error { return define.ErrPodRemoved } - return define.ErrNotImplemented - - // podID := []byte(pod.ID()) - // podName := []byte(pod.Name()) - - // var podNamespace []byte - // if pod.config.Namespace != "" { - // podNamespace = []byte(pod.config.Namespace) - // } - - // podConfigJSON, err := json.Marshal(pod.config) - // if err != nil { - // return fmt.Errorf("marshalling pod %s config to JSON: %w", pod.ID(), err) - // } - - // podStateJSON, err := json.Marshal(pod.state) - // if err != nil { - // return fmt.Errorf("marshalling pod %s state to JSON: %w", pod.ID(), err) - // } - - // db, err := s.getDBCon() - // if err != nil { - // return err - // } - // defer s.deferredCloseDBCon(db) - - // err = db.Update(func(tx *bolt.Tx) error { - // podBkt, err := getPodBucket(tx) - // if err != nil { - // return err - // } - - // allPodsBkt, err := getAllPodsBucket(tx) - // if err != nil { - // return err - // } - - // idsBkt, err := getIDBucket(tx) - // if err != nil { - // return err - // } - - // namesBkt, err := getNamesBucket(tx) - // if err != nil { - // return err - // } - - // nsBkt, err := getNSBucket(tx) - // if err != nil { - // return err - // } - - // // Check if we already have something with the given ID and name - // idExist := idsBkt.Get(podID) - // if idExist != nil { - // err = define.ErrPodExists - // if allPodsBkt.Get(idExist) == nil { - // err = define.ErrCtrExists - // } - // return fmt.Errorf("ID \"%s\" is in use: %w", pod.ID(), err) - // } - // nameExist := namesBkt.Get(podName) - // if nameExist != nil { - // err = define.ErrPodExists - // if allPodsBkt.Get(nameExist) == nil { - // err = define.ErrCtrExists - // } - // return fmt.Errorf("name \"%s\" is in use: %w", pod.Name(), err) - // } - - // // We are good to add the pod - // // Make a bucket for it - // newPod, err := podBkt.CreateBucket(podID) - // if err != nil { - // return fmt.Errorf("creating bucket for pod %s: %w", pod.ID(), err) - // } - - // // Make a subbucket for pod containers - // if _, err := newPod.CreateBucket(containersBkt); err != nil { - // return fmt.Errorf("creating bucket for pod %s containers: %w", pod.ID(), err) - // } + configJSON, err := json.Marshal(pod.config) + if err != nil { + return fmt.Errorf("marshalling pod config json: %w", err) + } - // if err := newPod.Put(configKey, podConfigJSON); err != nil { - // return fmt.Errorf("storing pod %s configuration in DB: %w", pod.ID(), err) - // } + stateJSON, err := json.Marshal(pod.state) + if err != nil { + return fmt.Errorf("marshalling pod state json: %w", err) + } - // if err := newPod.Put(stateKey, podStateJSON); err != nil { - // return fmt.Errorf("storing pod %s state JSON in DB: %w", pod.ID(), err) - // } + tx, err := s.conn.Begin() + if err != nil { + return fmt.Errorf("beginning pod create transaction: %w", err) + } + defer func() { + if defErr != nil { + if err := tx.Rollback(); err != nil { + logrus.Errorf("Rolling back transaction to create pod: %v", err) + } + } + }() - // if podNamespace != nil { - // if err := newPod.Put(namespaceKey, podNamespace); err != nil { - // return fmt.Errorf("storing pod %s namespace in DB: %w", pod.ID(), err) - // } - // if err := nsBkt.Put(podID, podNamespace); err != nil { - // return fmt.Errorf("storing pod %s namespace in DB: %w", pod.ID(), err) - // } - // } + if _, err := tx.Exec("INSERT INTO PodConfig VALUES (?, ?, ?);", pod.ID(), pod.Name(), configJSON); err != nil { + return fmt.Errorf("adding pod config to database: %w", err) + } - // // Add us to the ID and names buckets - // if err := idsBkt.Put(podID, podName); err != nil { - // return fmt.Errorf("storing pod %s ID in DB: %w", pod.ID(), err) - // } - // if err := namesBkt.Put(podName, podID); err != nil { - // return fmt.Errorf("storing pod %s name in DB: %w", pod.Name(), err) - // } - // if err := allPodsBkt.Put(podID, podName); err != nil { - // return fmt.Errorf("storing pod %s in all pods bucket in DB: %w", pod.ID(), err) - // } + if _, err := tx.Exec("INSERT INTO PodState VALUES (?, ?, ?);", pod.ID(), pod.state.InfraContainerID, stateJSON); err != nil { + return fmt.Errorf("adding pod state to database: %w", err) + } - // return nil - // }) - // if err != nil { - // return err - // } + if err := tx.Commit(); err != nil { + return fmt.Errorf("committing transaction: %w", err) + } - // return nil + return nil } // RemovePod removes the given pod from the state. // Only empty pods can be removed. -// TODO TODO TODO -func (s *SQLiteState) RemovePod(pod *Pod) error { +func (s *SQLiteState) RemovePod(pod *Pod) (defErr error) { if !s.valid { return define.ErrDBClosed } @@ -1721,93 +1489,61 @@ func (s *SQLiteState) RemovePod(pod *Pod) error { return define.ErrPodRemoved } - return define.ErrNotImplemented - - // podID := []byte(pod.ID()) - // podName := []byte(pod.Name()) - - // db, err := s.getDBCon() - // if err != nil { - // return err - // } - // defer s.deferredCloseDBCon(db) - - // err = db.Update(func(tx *bolt.Tx) error { - // podBkt, err := getPodBucket(tx) - // if err != nil { - // return err - // } - - // allPodsBkt, err := getAllPodsBucket(tx) - // if err != nil { - // return err - // } - - // idsBkt, err := getIDBucket(tx) - // if err != nil { - // return err - // } - - // namesBkt, err := getNamesBucket(tx) - // if err != nil { - // return err - // } - - // nsBkt, err := getNSBucket(tx) - // if err != nil { - // return err - // } + tx, err := s.conn.Begin() + if err != nil { + return fmt.Errorf("beginning pod %s removal transaction: %w", pod.ID(), err) + } + defer func() { + if defErr != nil { + if err := tx.Rollback(); err != nil { + logrus.Errorf("Rolling back transaction to remove pod %s: %v", pod.ID(), err) + } + } + }() - // // Check if the pod exists - // podDB := podBkt.Bucket(podID) - // if podDB == nil { - // pod.valid = false - // return fmt.Errorf("pod %s does not exist in DB: %w", pod.ID(), define.ErrNoSuchPod) - // } + var check int + row := tx.QueryRow("SELECT 1 FROM ContainerConfig WHERE PodID=?;", pod.ID()) + if err := row.Scan(&check); err != nil { + if !errors.Is(err, sql.ErrNoRows) { + return fmt.Errorf("checking if pod %s has containers in database: %w", pod.ID(), err) + } + } else if check != 0 { + return fmt.Errorf("pod %s is not empty: %w", pod.ID(), define.ErrCtrExists) + } - // // Check if pod is empty - // // This should never be nil - // // But if it is, we can assume there are no containers in the - // // pod. - // // So let's eject the malformed pod without error. - // podCtrsBkt := podDB.Bucket(containersBkt) - // if podCtrsBkt != nil { - // cursor := podCtrsBkt.Cursor() - // if id, _ := cursor.First(); id != nil { - // return fmt.Errorf("pod %s is not empty: %w", pod.ID(), define.ErrCtrExists) - // } - // } + checkResult := func(result sql.Result) error { + rows, err := result.RowsAffected() + if err != nil { + return fmt.Errorf("retrieving pod %s delete rows affected: %w", pod.ID(), err) + } + if rows == 0 { + pod.valid = false + return define.ErrNoSuchPod + } + return nil + } - // // Pod is empty, and ready for removal - // // Let's kick it out - // if err := idsBkt.Delete(podID); err != nil { - // return fmt.Errorf("removing pod %s ID from DB: %w", pod.ID(), err) - // } - // if err := namesBkt.Delete(podName); err != nil { - // return fmt.Errorf("removing pod %s name (%s) from DB: %w", pod.ID(), pod.Name(), err) - // } - // if err := nsBkt.Delete(podID); err != nil { - // return fmt.Errorf("removing pod %s namespace from DB: %w", pod.ID(), err) - // } - // if err := allPodsBkt.Delete(podID); err != nil { - // return fmt.Errorf("removing pod %s ID from all pods bucket in DB: %w", pod.ID(), err) - // } - // if err := podBkt.DeleteBucket(podID); err != nil { - // return fmt.Errorf("removing pod %s from DB: %w", pod.ID(), err) - // } + result, err := tx.Exec("DELETE FROM PodConfig WHERE ID=?;", pod.ID()) + if err != nil { + return fmt.Errorf("removing pod %s config from database: %w", pod.ID(), err) + } + if err := checkResult(result); err != nil { + return err + } - // return nil - // }) - // if err != nil { - // return err - // } + result, err = tx.Exec("DELETE FROM PodState WHERE ID=?;", pod.ID()) + if err != nil { + return fmt.Errorf("removing pod %s state from database: %w", pod.ID(), err) + } + if err := checkResult(result); err != nil { + return err + } - // return nil + return nil } // RemovePodContainers removes all containers in a pod. -// TODO TODO TODO -func (s *SQLiteState) RemovePodContainers(pod *Pod) error { +func (s *SQLiteState) RemovePodContainers(pod *Pod) (defErr error) { if !s.valid { return define.ErrDBClosed } @@ -1816,119 +1552,36 @@ func (s *SQLiteState) RemovePodContainers(pod *Pod) error { return define.ErrPodRemoved } - return define.ErrNotImplemented - - // podID := []byte(pod.ID()) - - // db, err := s.getDBCon() - // if err != nil { - // return err - // } - // defer s.deferredCloseDBCon(db) - - // err = db.Update(func(tx *bolt.Tx) error { - // podBkt, err := getPodBucket(tx) - // if err != nil { - // return err - // } - - // ctrBkt, err := getCtrBucket(tx) - // if err != nil { - // return err - // } - - // allCtrsBkt, err := getAllCtrsBucket(tx) - // if err != nil { - // return err - // } - - // idsBkt, err := getIDBucket(tx) - // if err != nil { - // return err - // } - - // namesBkt, err := getNamesBucket(tx) - // if err != nil { - // return err - // } - - // // Check if the pod exists - // podDB := podBkt.Bucket(podID) - // if podDB == nil { - // pod.valid = false - // return fmt.Errorf("pod %s does not exist in DB: %w", pod.ID(), define.ErrNoSuchPod) - // } - - // podCtrsBkt := podDB.Bucket(containersBkt) - // if podCtrsBkt == nil { - // return fmt.Errorf("pod %s does not have a containers bucket: %w", pod.ID(), define.ErrInternal) - // } + tx, err := s.conn.Begin() + if err != nil { + return fmt.Errorf("beginning removal transaction for containers of pod %s: %w", pod.ID(), err) + } + defer func() { + if defErr != nil { + if err := tx.Rollback(); err != nil { + logrus.Errorf("Rolling back transaction to remove containers of pod %s: %v", pod.ID(), err) + } + } + }() - // // Traverse all containers in the pod with a cursor - // // for-each has issues with data mutation - // err = podCtrsBkt.ForEach(func(id, name []byte) error { - // // Get the container so we can check dependencies - // ctr := ctrBkt.Bucket(id) - // if ctr == nil { - // // This should never happen - // // State is inconsistent - // return fmt.Errorf("pod %s referenced nonexistent container %s: %w", pod.ID(), string(id), define.ErrNoSuchCtr) - // } - // ctrDeps := ctr.Bucket(dependenciesBkt) - // // This should never be nil, but if it is, we're - // // removing it anyways, so continue if it is - // if ctrDeps != nil { - // err = ctrDeps.ForEach(func(depID, name []byte) error { - // exists := podCtrsBkt.Get(depID) - // if exists == nil { - // return fmt.Errorf("container %s has dependency %s outside of pod %s: %w", string(id), string(depID), pod.ID(), define.ErrCtrExists) - // } - // return nil - // }) - // if err != nil { - // return err - // } - // } - - // // Dependencies are set, we're clear to remove - - // if err := ctrBkt.DeleteBucket(id); err != nil { - // return fmt.Errorf("deleting container %s from DB: %w", string(id), define.ErrInternal) - // } - - // if err := idsBkt.Delete(id); err != nil { - // return fmt.Errorf("deleting container %s ID in DB: %w", string(id), err) - // } - - // if err := namesBkt.Delete(name); err != nil { - // return fmt.Errorf("deleting container %s name in DB: %w", string(id), err) - // } - - // if err := allCtrsBkt.Delete(id); err != nil { - // return fmt.Errorf("deleting container %s ID from all containers bucket in DB: %w", string(id), err) - // } - - // return nil - // }) - // if err != nil { - // return err - // } + rows, err := tx.Query("SELECT ID FROM ContainerConfig WHERE PodID=?;", pod.ID()) + if err != nil { + return fmt.Errorf("retrieving container IDs of pod %s from database: %w", pod.ID(), err) + } + defer rows.Close() - // // Delete and recreate the bucket to empty it - // if err := podDB.DeleteBucket(containersBkt); err != nil { - // return fmt.Errorf("removing pod %s containers bucket: %w", pod.ID(), err) - // } - // if _, err := podDB.CreateBucket(containersBkt); err != nil { - // return fmt.Errorf("recreating pod %s containers bucket: %w", pod.ID(), err) - // } + for rows.Next() { + var id string + if err := rows.Scan(&id); err != nil { + return fmt.Errorf("scanning container from database: %w", err) + } - // return nil - // }) - // if err != nil { - // return err - // } + if err := s.removeContainerWithTx(id, tx); err != nil { + return err + } + } - // return nil + return nil } // AddContainerToPod adds the given container to an existing pod @@ -1976,7 +1629,6 @@ func (s *SQLiteState) RemoveContainerFromPod(pod *Pod, ctr *Container) error { } // UpdatePod updates a pod's state from the database. -// TODO TODO TODO func (s *SQLiteState) UpdatePod(pod *Pod) error { if !s.valid { return define.ErrDBClosed @@ -1986,54 +1638,29 @@ func (s *SQLiteState) UpdatePod(pod *Pod) error { return define.ErrPodRemoved } - return define.ErrNotImplemented - - // newState := new(podState) - - // db, err := s.getDBCon() - // if err != nil { - // return err - // } - // defer s.deferredCloseDBCon(db) - - // podID := []byte(pod.ID()) - - // err = db.View(func(tx *bolt.Tx) error { - // podBkt, err := getPodBucket(tx) - // if err != nil { - // return err - // } - - // podDB := podBkt.Bucket(podID) - // if podDB == nil { - // pod.valid = false - // return fmt.Errorf("no pod with ID %s found in database: %w", pod.ID(), define.ErrNoSuchPod) - // } - - // // Get the pod state JSON - // podStateBytes := podDB.Get(stateKey) - // if podStateBytes == nil { - // return fmt.Errorf("pod %s is missing state key in DB: %w", pod.ID(), define.ErrInternal) - // } + row := s.conn.QueryRow("SELECT JSON FROM PodState WHERE ID=?;", pod.ID()) - // if err := json.Unmarshal(podStateBytes, newState); err != nil { - // return fmt.Errorf("unmarshalling pod %s state JSON: %w", pod.ID(), err) - // } + var rawJSON string + if err := row.Scan(&rawJSON); err != nil { + if errors.Is(err, sql.ErrNoRows) { + // Pod was removed + pod.valid = false + return fmt.Errorf("no pod with ID %s found in database: %w", pod.ID(), define.ErrNoSuchPod) + } + } - // return nil - // }) - // if err != nil { - // return err - // } + newState := new(podState) + if err := json.Unmarshal([]byte(rawJSON), newState); err != nil { + return fmt.Errorf("unmarshalling pod %s state JSON: %w", pod.ID(), err) + } - // pod.state = newState + pod.state = newState - // return nil + return nil } // SavePod saves a pod's state to the database. -// TODO TODO TODO -func (s *SQLiteState) SavePod(pod *Pod) error { +func (s *SQLiteState) SavePod(pod *Pod) (defErr error) { if !s.valid { return define.ErrDBClosed } @@ -2042,104 +1669,71 @@ func (s *SQLiteState) SavePod(pod *Pod) error { return define.ErrPodRemoved } - return define.ErrNotImplemented - - // stateJSON, err := json.Marshal(pod.state) - // if err != nil { - // return fmt.Errorf("marshalling pod %s state to JSON: %w", pod.ID(), err) - // } - - // db, err := s.getDBCon() - // if err != nil { - // return err - // } - // defer s.deferredCloseDBCon(db) - - // podID := []byte(pod.ID()) - - // err = db.Update(func(tx *bolt.Tx) error { - // podBkt, err := getPodBucket(tx) - // if err != nil { - // return err - // } + stateJSON, err := json.Marshal(pod.state) + if err != nil { + return fmt.Errorf("marshalling pod %s state JSON: %w", pod.ID(), err) + } - // podDB := podBkt.Bucket(podID) - // if podDB == nil { - // pod.valid = false - // return fmt.Errorf("no pod with ID %s found in database: %w", pod.ID(), define.ErrNoSuchPod) - // } + tx, err := s.conn.Begin() + if err != nil { + return fmt.Errorf("beginning pod %s save transaction: %w", pod.ID(), err) + } + defer func() { + if defErr != nil { + if err := tx.Rollback(); err != nil { + logrus.Errorf("Rolling back transaction to save pod %s state: %v", pod.ID(), err) + } + } + }() - // // Set the pod state JSON - // if err := podDB.Put(stateKey, stateJSON); err != nil { - // return fmt.Errorf("updating pod %s state in database: %w", pod.ID(), err) - // } + result, err := tx.Exec("UPDATE PodState SET JSON=? WHERE ID=?;", stateJSON, pod.ID()) + if err != nil { + return fmt.Errorf("writing pod %s state: %w", pod.ID(), err) + } + rows, err := result.RowsAffected() + if err != nil { + return fmt.Errorf("retrieving pod %s save rows affected: %w", pod.ID(), err) + } + if rows == 0 { + pod.valid = false + return define.ErrNoSuchPod + } - // return nil - // }) - // if err != nil { - // return err - // } + if err := tx.Commit(); err != nil { + return fmt.Errorf("committing pod %s state: %w", pod.ID(), err) + } - // return nil + return nil } // AllPods returns all pods present in the state. -// TODO TODO TODO func (s *SQLiteState) AllPods() ([]*Pod, error) { if !s.valid { return nil, define.ErrDBClosed } - return nil, define.ErrNotImplemented - - // pods := []*Pod{} - - // db, err := s.getDBCon() - // if err != nil { - // return nil, err - // } - // defer s.deferredCloseDBCon(db) + pods := []*Pod{} + rows, err := s.conn.Query("SELECT JSON FROM PodConfig;") + if err != nil { + return nil, fmt.Errorf("retrieving all pods from database: %w", err) + } + defer rows.Close() - // err = db.View(func(tx *bolt.Tx) error { - // allPodsBucket, err := getAllPodsBucket(tx) - // if err != nil { - // return err - // } + for rows.Next() { + var rawJSON string + if err := rows.Scan(&rawJSON); err != nil { + return nil, fmt.Errorf("scanning pod from database: %w", err) + } - // podBucket, err := getPodBucket(tx) - // if err != nil { - // return err - // } + pod, err := s.createPod(rawJSON) + if err != nil { + return nil, err + } - // err = allPodsBucket.ForEach(func(id, name []byte) error { - // podExists := podBucket.Bucket(id) - // // This check can be removed if performance becomes an - // // issue, but much less helpful errors will be produced - // if podExists == nil { - // return fmt.Errorf("inconsistency in state - pod %s is in all pods bucket but pod not found: %w", string(id), define.ErrInternal) - // } - - // pod := new(Pod) - // pod.config = new(PodConfig) - // pod.state = new(podState) - - // if err := s.getPodFromDB(id, pod, podBucket); err != nil { - // if !errors.Is(err, define.ErrNSMismatch) { - // logrus.Errorf("Retrieving pod %s from the database: %v", string(id), err) - // } - // } else { - // pods = append(pods, pod) - // } - - // return nil - // }) - // return err - // }) - // if err != nil { - // return nil, err - // } + pods = append(pods, pod) + } - // return pods, nil + return pods, nil } // AddVolume adds the given volume to the state. It also adds ctrDepID to diff --git a/libpod/sqlite_state_internal.go b/libpod/sqlite_state_internal.go index e874cf4566..af83968268 100644 --- a/libpod/sqlite_state_internal.go +++ b/libpod/sqlite_state_internal.go @@ -266,17 +266,24 @@ func finalizeCtrSqlite(ctr *Container) error { } // Finalize a pod that was pulled out of the database. -func finalizePodSqlite(pod *Pod) error { - // Get the lock - lock, err := pod.runtime.lockManager.RetrieveLock(pod.config.LockID) +func (s *SQLiteState) createPod(rawJSON string) (*Pod, error) { + config := new(PodConfig) + if err := json.Unmarshal([]byte(rawJSON), config); err != nil { + return nil, fmt.Errorf("unmarshalling pod config: %w", err) + } + lock, err := s.runtime.lockManager.RetrieveLock(config.LockID) if err != nil { - return fmt.Errorf("retrieving lock for pod %s: %w", pod.ID(), err) + return nil, fmt.Errorf("retrieving lock for pod %s: %w", config.ID, err) } - pod.lock = lock + pod := new(Pod) + pod.config = config + pod.state = new(podState) + pod.lock = lock + pod.runtime = s.runtime pod.valid = true - return nil + return pod, nil } // Finalize a volume that was pulled out of the database @@ -343,10 +350,10 @@ func (s *SQLiteState) addContainer(ctr *Container) (defErr error) { } deps := ctr.Dependencies() - pod := sql.NullString{} + podID := sql.NullString{} if ctr.config.Pod != "" { - pod.Valid = true - pod.String = ctr.config.Pod + podID.Valid = true + podID.String = ctr.config.Pod } tx, err := s.conn.Begin() @@ -364,7 +371,7 @@ func (s *SQLiteState) addContainer(ctr *Container) (defErr error) { if _, err := tx.Exec("INSERT INTO IDNamespace VALUES (?);", ctr.ID()); err != nil { return fmt.Errorf("adding container id to database: %w", err) } - if _, err := tx.Exec("INSERT INTO ContainerConfig VALUES (?, ?, ?, ?);", ctr.ID(), ctr.Name(), pod, configJSON); err != nil { + if _, err := tx.Exec("INSERT INTO ContainerConfig VALUES (?, ?, ?, ?);", ctr.ID(), ctr.Name(), podID, configJSON); err != nil { return fmt.Errorf("adding container config to database: %w", err) } if _, err := tx.Exec("INSERT INTO ContainerState VALUES (?, ?, ?, ?);", ctr.ID(), int(ctr.state.State), ctr.state.ExitCode, stateJSON); err != nil { @@ -405,11 +412,13 @@ func (s *SQLiteState) addContainer(ctr *Container) (defErr error) { return nil } +// removeContainer remove the specified container from the database. func (s *SQLiteState) removeContainer(ctr *Container) (defErr error) { tx, err := s.conn.Begin() if err != nil { return fmt.Errorf("beginning container %s removal transaction: %w", ctr.ID(), err) } + defer func() { if defErr != nil { if err := tx.Rollback(); err != nil { @@ -418,32 +427,41 @@ func (s *SQLiteState) removeContainer(ctr *Container) (defErr error) { } }() + if err := s.removeContainerWithTx(ctr.ID(), tx); err != nil { + return err + } + + if err := tx.Commit(); err != nil { + return fmt.Errorf("committing container %s removal transaction: %w", ctr.ID(), err) + } + + return nil +} + +// removeContainerWithTx removes the container with the specified transaction. +// Callers are responsible for committing. +func (s *SQLiteState) removeContainerWithTx(id string, tx *sql.Tx) error { // TODO TODO TODO: // Need to verify that at least 1 row was deleted from ContainerConfig. // Otherwise return ErrNoSuchCtr. - if _, err := tx.Exec("DELETE FROM IDNamespace WHERE ID=?;", ctr.ID()); err != nil { - return fmt.Errorf("removing container %s id from database: %w", ctr.ID(), err) + if _, err := tx.Exec("DELETE FROM IDNamespace WHERE ID=?;", id); err != nil { + return fmt.Errorf("removing container %s id from database: %w", id, err) } - if _, err := tx.Exec("DELETE FROM ContainerConfig WHERE ID=?;", ctr.ID()); err != nil { - return fmt.Errorf("removing container %s config from database: %w", ctr.ID(), err) + if _, err := tx.Exec("DELETE FROM ContainerConfig WHERE ID=?;", id); err != nil { + return fmt.Errorf("removing container %s config from database: %w", id, err) } - if _, err := tx.Exec("DELETE FROM ContainerState WHERE ID=?;", ctr.ID()); err != nil { - return fmt.Errorf("removing container %s state from database: %w", ctr.ID(), err) + if _, err := tx.Exec("DELETE FROM ContainerState WHERE ID=?;", id); err != nil { + return fmt.Errorf("removing container %s state from database: %w", id, err) } - if _, err := tx.Exec("DELETE FROM ContainerDependency WHERE ID=?;", ctr.ID()); err != nil { - return fmt.Errorf("removing container %s dependencies from database: %w", ctr.ID(), err) + if _, err := tx.Exec("DELETE FROM ContainerDependency WHERE ID=?;", id); err != nil { + return fmt.Errorf("removing container %s dependencies from database: %w", id, err) } - if _, err := tx.Exec("DELETE FROM ContainerVolume WHERE ContainerID=?;", ctr.ID()); err != nil { - return fmt.Errorf("removing container %s volumes from database: %w", ctr.ID(), err) + if _, err := tx.Exec("DELETE FROM ContainerVolume WHERE ContainerID=?;", id); err != nil { + return fmt.Errorf("removing container %s volumes from database: %w", id, err) } - if _, err := tx.Exec("DELETE FROM ContainerExecSession WHERE ContainerID=?;", ctr.ID()); err != nil { - return fmt.Errorf("removing container %s exec sessions from database: %w", ctr.ID(), err) + if _, err := tx.Exec("DELETE FROM ContainerExecSession WHERE ContainerID=?;", id); err != nil { + return fmt.Errorf("removing container %s exec sessions from database: %w", id, err) } - - if err := tx.Commit(); err != nil { - return fmt.Errorf("committing container %s removal transaction: %w", ctr.ID(), err) - } - return nil } From 8c64c4370fd904491fb3b7d5f6fad3f080c3ed94 Mon Sep 17 00:00:00 2001 From: Valentin Rothberg Date: Thu, 23 Feb 2023 09:43:24 +0100 Subject: [PATCH 02/14] sqlite: move migration after table creation Otherwise we'll fail immediately as the schema version is returned as 0. Signed-off-by: Valentin Rothberg --- libpod/sqlite_state.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/libpod/sqlite_state.go b/libpod/sqlite_state.go index 48fb368d45..95152876c8 100644 --- a/libpod/sqlite_state.go +++ b/libpod/sqlite_state.go @@ -64,15 +64,15 @@ func NewSqliteState(runtime *Runtime) (_ State, defErr error) { return nil, fmt.Errorf("setting full fsync mode in db: %w", err) } - if err := state.migrateSchemaIfNecessary(); err != nil { - return nil, err - } - // Set up tables if err := sqliteInitTables(state.conn); err != nil { return nil, fmt.Errorf("creating tables: %w", err) } + if err := state.migrateSchemaIfNecessary(); err != nil { + return nil, err + } + state.valid = true state.runtime = runtime From 560805ac4c3ab2173e30e6d33292c9b911a0240c Mon Sep 17 00:00:00 2001 From: Valentin Rothberg Date: Thu, 23 Feb 2023 09:47:14 +0100 Subject: [PATCH 03/14] sqlite: AllContainers: fix inner join The base table was missing, so we caused a syntax error. Signed-off-by: Valentin Rothberg --- libpod/sqlite_state.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libpod/sqlite_state.go b/libpod/sqlite_state.go index 95152876c8..a573480955 100644 --- a/libpod/sqlite_state.go +++ b/libpod/sqlite_state.go @@ -716,7 +716,7 @@ func (s *SQLiteState) AllContainers(loadState bool) ([]*Container, error) { ctrs := []*Container{} if loadState { - rows, err := s.conn.Query("SELECT ContainerConfig.JSON, ContainerState.JSON AS StateJSON INNER JOIN ContainerState ON ContainerConfig.ID = ContainerState.ID;") + rows, err := s.conn.Query("SELECT ContainerConfig.JSON, ContainerState.JSON AS StateJSON FROM ContainerConfig INNER JOIN ContainerState ON ContainerConfig.ID = ContainerState.ID;") if err != nil { return nil, fmt.Errorf("retrieving all containers from database: %w", err) } From e74f7bcaf3a5b9bf191026aecdec051319e10072 Mon Sep 17 00:00:00 2001 From: Valentin Rothberg Date: Thu, 23 Feb 2023 09:54:14 +0100 Subject: [PATCH 04/14] sqlite: fix typo when removing exec sessions Signed-off-by: Valentin Rothberg --- libpod/sqlite_state.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libpod/sqlite_state.go b/libpod/sqlite_state.go index a573480955..372f354d19 100644 --- a/libpod/sqlite_state.go +++ b/libpod/sqlite_state.go @@ -1093,7 +1093,7 @@ func (s *SQLiteState) RemoveContainerExecSessions(ctr *Container) (defErr error) } }() - if _, err := tx.Exec("DELETE FROM ContainerExecSessions WHERE ContainerID=?;", ctr.ID()); err != nil { + if _, err := tx.Exec("DELETE FROM ContainerExecSession WHERE ContainerID=?;", ctr.ID()); err != nil { return fmt.Errorf("removing container %s exec sessions from database: %w", ctr.ID(), err) } From 7c11f7e17413c0a6bf266f21a843f4ac539f26aa Mon Sep 17 00:00:00 2001 From: Valentin Rothberg Date: Thu, 23 Feb 2023 10:16:30 +0100 Subject: [PATCH 05/14] sqlite: exit code: allow -1 The value of -1 is used when we do not _yet_ know the exit code of the container. Otherwise, the DB checks would error. There's probably a smarter than allowing -1 but for now, that will do the trick and let the tests progress. Signed-off-by: Valentin Rothberg --- libpod/sqlite_state.go | 2 +- libpod/sqlite_state_internal.go | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/libpod/sqlite_state.go b/libpod/sqlite_state.go index 372f354d19..87e194214a 100644 --- a/libpod/sqlite_state.go +++ b/libpod/sqlite_state.go @@ -856,7 +856,7 @@ func (s *SQLiteState) AddContainerExitCode(id string, exitCode int32) (defErr er }() if _, err := tx.Exec("INSERT INTO ContainerExitCode VALUES (?, ?, ?);", id, time.Now().Unix(), exitCode); err != nil { - return fmt.Errorf("adding container %s exit code: %w", id, err) + return fmt.Errorf("adding container %s exit code %d: %w", id, exitCode, err) } if err := tx.Commit(); err != nil { diff --git a/libpod/sqlite_state_internal.go b/libpod/sqlite_state_internal.go index af83968268..1128d01c77 100644 --- a/libpod/sqlite_state_internal.go +++ b/libpod/sqlite_state_internal.go @@ -86,7 +86,7 @@ func sqliteInitTables(conn *sql.DB) (defErr error) { ExitCode INTEGER, JSON TEXT NOT NULL, FOREIGN KEY (ID) REFERENCES ContainerConfig(ID) DEFERRABLE INITIALLY DEFERRED, - CHECK (ExitCode BETWEEN 0 AND 255) + CHECK (ExitCode BETWEEN -1 AND 255) );` const containerExecSession = ` @@ -120,7 +120,7 @@ func sqliteInitTables(conn *sql.DB) (defErr error) { ID TEXT PRIMARY KEY NOT NULL, Timestamp INTEGER NOT NULL, ExitCode INTEGER NOT NULL, - CHECK (ExitCode BETWEEN 0 AND 255) + CHECK (ExitCode BETWEEN -1 AND 255) );` const podConfig = ` From 3f96b0ef28ae30d45bb6b5f95974f97593bae7cf Mon Sep 17 00:00:00 2001 From: Valentin Rothberg Date: Thu, 23 Feb 2023 10:22:06 +0100 Subject: [PATCH 06/14] sqlite: SaveVolume: fix syntax error updating the volumes table Signed-off-by: Valentin Rothberg --- libpod/sqlite_state.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libpod/sqlite_state.go b/libpod/sqlite_state.go index 87e194214a..a4935c542a 100644 --- a/libpod/sqlite_state.go +++ b/libpod/sqlite_state.go @@ -1908,7 +1908,7 @@ func (s *SQLiteState) SaveVolume(volume *Volume) (defErr error) { } }() - results, err := tx.Exec("UPDATE TABLE VolumeState SET JSON=? WHERE Name=?;", stateJSON, volume.Name()) + results, err := tx.Exec("UPDATE VolumeState SET JSON=? WHERE Name=?;", stateJSON, volume.Name()) if err != nil { return fmt.Errorf("updating volume %s state in DB: %w", volume.Name(), err) } From 21fcc9070f0690adbbbfa7851bb55079efa192e3 Mon Sep 17 00:00:00 2001 From: Valentin Rothberg Date: Thu, 23 Feb 2023 10:48:11 +0100 Subject: [PATCH 07/14] sqlite: fix "UPDATE TABLE" typos "TABLE" should refer to the actual table. Signed-off-by: Valentin Rothberg --- libpod/sqlite_state.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/libpod/sqlite_state.go b/libpod/sqlite_state.go index a4935c542a..dacb7f041a 100644 --- a/libpod/sqlite_state.go +++ b/libpod/sqlite_state.go @@ -212,17 +212,17 @@ func (s *SQLiteState) Refresh() (defErr error) { }() for id, json := range ctrStates { - if _, err := tx.Exec("UPDATE TABLE ContainerState SET JSON=? WHERE ID=?;", json, id); err != nil { + if _, err := tx.Exec("UPDATE ContainerState SET JSON=? WHERE ID=?;", json, id); err != nil { return fmt.Errorf("updating container state: %w", err) } } for id, json := range podStates { - if _, err := tx.Exec("UPDATE TABLE PodState SET JSON=? WHERE ID=?;", json, id); err != nil { + if _, err := tx.Exec("UPDATE PodState SET JSON=? WHERE ID=?;", json, id); err != nil { return fmt.Errorf("updating pod state: %w", err) } } for name, json := range volumeStates { - if _, err := tx.Exec("UPDATE TABLE VolumeState SET JSON=? WHERE Name=?;", json, name); err != nil { + if _, err := tx.Exec("UPDATE VolumeState SET JSON=? WHERE Name=?;", json, name); err != nil { return fmt.Errorf("updating volume state: %w", err) } } From 1b1cdfa357778ca7a2d7296e3b7eb3dd5cf34d61 Mon Sep 17 00:00:00 2001 From: Valentin Rothberg Date: Thu, 23 Feb 2023 11:19:43 +0100 Subject: [PATCH 08/14] sqlite: fix AllContainers with state The state has been unmarshalled into the config which surfaced in wrong states. Signed-off-by: Valentin Rothberg --- libpod/sqlite_state.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/libpod/sqlite_state.go b/libpod/sqlite_state.go index dacb7f041a..add06dd926 100644 --- a/libpod/sqlite_state.go +++ b/libpod/sqlite_state.go @@ -736,8 +736,8 @@ func (s *SQLiteState) AllContainers(loadState bool) ([]*Container, error) { if err := json.Unmarshal([]byte(configJSON), ctr.config); err != nil { return nil, fmt.Errorf("unmarshalling container config: %w", err) } - if err := json.Unmarshal([]byte(stateJSON), ctr.config); err != nil { - return nil, fmt.Errorf("unmarshalling container %s config: %w", ctr.ID(), err) + if err := json.Unmarshal([]byte(stateJSON), ctr.state); err != nil { + return nil, fmt.Errorf("unmarshalling container %s state: %w", ctr.ID(), err) } ctrs = append(ctrs, ctr) From 565bb564546e6cbcb923d49f145b2e851672216c Mon Sep 17 00:00:00 2001 From: Valentin Rothberg Date: Thu, 23 Feb 2023 11:30:46 +0100 Subject: [PATCH 09/14] sqlite: AddContainerExitCode: allow to replace Allow to replace existing exit codes. A container may be started and stopped multiple times etc. Signed-off-by: Valentin Rothberg --- libpod/sqlite_state.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libpod/sqlite_state.go b/libpod/sqlite_state.go index add06dd926..f525411122 100644 --- a/libpod/sqlite_state.go +++ b/libpod/sqlite_state.go @@ -855,7 +855,7 @@ func (s *SQLiteState) AddContainerExitCode(id string, exitCode int32) (defErr er } }() - if _, err := tx.Exec("INSERT INTO ContainerExitCode VALUES (?, ?, ?);", id, time.Now().Unix(), exitCode); err != nil { + if _, err := tx.Exec("INSERT OR REPLACE INTO ContainerExitCode VALUES (?, ?, ?);", id, time.Now().Unix(), exitCode); err != nil { return fmt.Errorf("adding container %s exit code %d: %w", id, exitCode, err) } From e32bea937892b2539170fffd93a4b8c84178cb8d Mon Sep 17 00:00:00 2001 From: Valentin Rothberg Date: Thu, 23 Feb 2023 11:36:47 +0100 Subject: [PATCH 10/14] sqlite: LookupContainer: update error message As expected by the system tests. Signed-off-by: Valentin Rothberg --- libpod/sqlite_state.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libpod/sqlite_state.go b/libpod/sqlite_state.go index f525411122..7a22c0c492 100644 --- a/libpod/sqlite_state.go +++ b/libpod/sqlite_state.go @@ -521,7 +521,7 @@ func (s *SQLiteState) LookupContainer(idOrName string) (*Container, error) { foundResult = true } if !foundResult { - return nil, define.ErrNoSuchCtr + return nil, fmt.Errorf("no container with name or ID %q found: %w", idOrName, define.ErrNoSuchCtr) } ctr := new(Container) From 19c2f37ba51347b6b82e9d27dbb6345c7f20b4f0 Mon Sep 17 00:00:00 2001 From: Valentin Rothberg Date: Thu, 23 Feb 2023 13:19:31 +0100 Subject: [PATCH 11/14] sqlite: fix pod create/rm A number of fixes for pod creation and removal. The important part is that matching partial IDs requires a trailing `%` for SQL to interpret it as a wildcard. More information at https://www.sqlitetutorial.net/sqlite-like/ Signed-off-by: Valentin Rothberg --- libpod/sqlite_state.go | 31 ++++++++++++++++++++++++++----- 1 file changed, 26 insertions(+), 5 deletions(-) diff --git a/libpod/sqlite_state.go b/libpod/sqlite_state.go index 7a22c0c492..26270f1ff5 100644 --- a/libpod/sqlite_state.go +++ b/libpod/sqlite_state.go @@ -1280,7 +1280,7 @@ func (s *SQLiteState) LookupPod(idOrName string) (*Pod, error) { return nil, define.ErrDBClosed } - rows, err := s.conn.Query("SELECT JSON FROM PodConfig WHERE PodConfig.Name=? OR (PodConfig.ID LIKE ?);", idOrName, idOrName) + rows, err := s.conn.Query("SELECT JSON FROM PodConfig WHERE PodConfig.Name=? OR (PodConfig.ID LIKE ?);", idOrName, idOrName+"%") if err != nil { return nil, fmt.Errorf("looking up pod %q in database: %w", idOrName, err) } @@ -1441,6 +1441,13 @@ func (s *SQLiteState) AddPod(pod *Pod) (defErr error) { return define.ErrPodRemoved } + infraID := sql.NullString{} + if pod.state.InfraContainerID != "" { + if err := infraID.Scan(pod.state.InfraContainerID); err != nil { + return fmt.Errorf("scanning infra container ID %q: %w", pod.state.InfraContainerID, err) + } + } + configJSON, err := json.Marshal(pod.config) if err != nil { return fmt.Errorf("marshalling pod config json: %w", err) @@ -1463,11 +1470,13 @@ func (s *SQLiteState) AddPod(pod *Pod) (defErr error) { } }() + if _, err := tx.Exec("INSERT INTO IDNamespace VALUES (?);", pod.ID()); err != nil { + return fmt.Errorf("adding pod id to database: %w", err) + } if _, err := tx.Exec("INSERT INTO PodConfig VALUES (?, ?, ?);", pod.ID(), pod.Name(), configJSON); err != nil { return fmt.Errorf("adding pod config to database: %w", err) } - - if _, err := tx.Exec("INSERT INTO PodState VALUES (?, ?, ?);", pod.ID(), pod.state.InfraContainerID, stateJSON); err != nil { + if _, err := tx.Exec("INSERT INTO PodState VALUES (?, ?, ?);", pod.ID(), infraID, stateJSON); err != nil { return fmt.Errorf("adding pod state to database: %w", err) } @@ -1502,7 +1511,7 @@ func (s *SQLiteState) RemovePod(pod *Pod) (defErr error) { }() var check int - row := tx.QueryRow("SELECT 1 FROM ContainerConfig WHERE PodID=?;", pod.ID()) + row := tx.QueryRow("SELECT 1 FROM ContainerConfig WHERE PodID=? AND ID!=?;", pod.ID(), pod.state.InfraContainerID) if err := row.Scan(&check); err != nil { if !errors.Is(err, sql.ErrNoRows) { return fmt.Errorf("checking if pod %s has containers in database: %w", pod.ID(), err) @@ -1523,7 +1532,15 @@ func (s *SQLiteState) RemovePod(pod *Pod) (defErr error) { return nil } - result, err := tx.Exec("DELETE FROM PodConfig WHERE ID=?;", pod.ID()) + result, err := tx.Exec("DELETE FROM IDNamespace WHERE ID=?;", pod.ID()) + if err != nil { + return fmt.Errorf("removing pod %s id from database: %w", pod.ID(), err) + } + if err := checkResult(result); err != nil { + return err + } + + result, err = tx.Exec("DELETE FROM PodConfig WHERE ID=?;", pod.ID()) if err != nil { return fmt.Errorf("removing pod %s config from database: %w", pod.ID(), err) } @@ -1539,6 +1556,10 @@ func (s *SQLiteState) RemovePod(pod *Pod) (defErr error) { return err } + if err := tx.Commit(); err != nil { + return fmt.Errorf("committing pod %s removal transaction: %w", pod.ID(), err) + } + return nil } From efe7aeb1da2855e192ccd08da4980f624c4e309a Mon Sep 17 00:00:00 2001 From: Valentin Rothberg Date: Thu, 23 Feb 2023 13:42:41 +0100 Subject: [PATCH 12/14] sqlite: fix LookupPod To return the error message expected by the system tests. Signed-off-by: Valentin Rothberg --- libpod/sqlite_state.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libpod/sqlite_state.go b/libpod/sqlite_state.go index 26270f1ff5..5e10ae7946 100644 --- a/libpod/sqlite_state.go +++ b/libpod/sqlite_state.go @@ -1299,7 +1299,7 @@ func (s *SQLiteState) LookupPod(idOrName string) (*Pod, error) { foundResult = true } if !foundResult { - return nil, define.ErrNoSuchPod + return nil, fmt.Errorf("no pod with name or ID %s found: %w", idOrName, define.ErrNoSuchPod) } return s.createPod(rawJSON) From 495314a16a1b03edcbcffd28804706ccb5f62fa5 Mon Sep 17 00:00:00 2001 From: Valentin Rothberg Date: Thu, 23 Feb 2023 13:47:32 +0100 Subject: [PATCH 13/14] sqlite: fix container lookups with partial IDs Requires the trailing `%` to work correctly, see https://www.sqlitetutorial.net/sqlite-like/ Signed-off-by: Valentin Rothberg --- libpod/sqlite_state.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/libpod/sqlite_state.go b/libpod/sqlite_state.go index 5e10ae7946..b3ef023a39 100644 --- a/libpod/sqlite_state.go +++ b/libpod/sqlite_state.go @@ -466,7 +466,7 @@ func (s *SQLiteState) LookupContainerID(idOrName string) (string, error) { return "", define.ErrDBClosed } - rows, err := s.conn.Query("SELECT ID FROM ContainerConfig WHERE ContainerConfig.Name=? OR (ContainerConfig.ID LIKE ?);", idOrName, idOrName) + rows, err := s.conn.Query("SELECT ID FROM ContainerConfig WHERE ContainerConfig.Name=? OR (ContainerConfig.ID LIKE ?);", idOrName, idOrName+"%") if err != nil { return "", fmt.Errorf("looking up container %q in database: %w", idOrName, err) } @@ -502,7 +502,7 @@ func (s *SQLiteState) LookupContainer(idOrName string) (*Container, error) { return nil, define.ErrDBClosed } - rows, err := s.conn.Query("SELECT JSON FROM ContainerConfig WHERE ContainerConfig.Name=? OR (ContainerConfig.ID LIKE ?);", idOrName, idOrName) + rows, err := s.conn.Query("SELECT JSON FROM ContainerConfig WHERE ContainerConfig.Name=? OR (ContainerConfig.ID LIKE ?);", idOrName, idOrName+"%") if err != nil { return nil, fmt.Errorf("looking up container %q in database: %w", idOrName, err) } From 5d2d609be4867bee67dbbcf9a66d0a9ccfb877e7 Mon Sep 17 00:00:00 2001 From: Valentin Rothberg Date: Thu, 23 Feb 2023 13:56:58 +0100 Subject: [PATCH 14/14] sqlite: fix volume lookups with partial names Requires the trailing `%` to work correctly, see https://www.sqlitetutorial.net/sqlite-like/ Signed-off-by: Valentin Rothberg --- libpod/sqlite_state.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libpod/sqlite_state.go b/libpod/sqlite_state.go index b3ef023a39..e03d7ad49b 100644 --- a/libpod/sqlite_state.go +++ b/libpod/sqlite_state.go @@ -2033,7 +2033,7 @@ func (s *SQLiteState) LookupVolume(name string) (*Volume, error) { return nil, define.ErrDBClosed } - rows, err := s.conn.Query("SELECT JSON FROM VolumeConfig WHERE Name LIKE ?;", name) + rows, err := s.conn.Query("SELECT JSON FROM VolumeConfig WHERE Name LIKE ?;", name+"%") if err != nil { return nil, fmt.Errorf("querying database for volume %s: %w", name, err) }