From 84b5c6c713b614fa84274eafa21f0aed7c57aaab Mon Sep 17 00:00:00 2001
From: Valentin Rothberg <vrothberg@redhat.com>
Date: Mon, 27 Feb 2023 13:06:52 +0100
Subject: [PATCH 1/9] sqlite: RewritePodConfig: update error message
Use the same error message as the boltdb backend.
[NO NEW TESTS NEEDED]
Signed-off-by: Valentin Rothberg <vrothberg@redhat.com>
---
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 e03d7ad49b..a543523923 100644
--- a/libpod/sqlite_state.go
+++ b/libpod/sqlite_state.go
@@ -1186,7 +1186,7 @@ func (s *SQLiteState) RewritePodConfig(pod *Pod, newCfg *PodConfig) (defErr erro
}
if rows == 0 {
pod.valid = false
- return define.ErrNoSuchPod
+ return fmt.Errorf("no pod with ID %s found in DB: %w", pod.ID(), define.ErrNoSuchPod)
}
if err := tx.Commit(); err != nil {
From e87014e444085a64411559f46ea38877938de8c4 Mon Sep 17 00:00:00 2001
From: Valentin Rothberg <vrothberg@redhat.com>
Date: Mon, 27 Feb 2023 16:54:13 +0100
Subject: [PATCH 2/9] sqlite: return correct error on pod-name conflict
I wasn't able to find a way to get error-checks working with the sqlite3
library with the time at hand.
[NO NEW TESTS NEEDED]
Signed-off-by: Valentin Rothberg <vrothberg@redhat.com>
---
libpod/sqlite_state.go | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/libpod/sqlite_state.go b/libpod/sqlite_state.go
index a543523923..b86b648996 100644
--- a/libpod/sqlite_state.go
+++ b/libpod/sqlite_state.go
@@ -1470,6 +1470,19 @@ func (s *SQLiteState) AddPod(pod *Pod) (defErr error) {
}
}()
+ // TODO: explore whether there's a more idiomatic way to do error checks for the name.
+ // There is a sqlite3.ErrConstraintUnique error but I (vrothberg) couldn't find a way
+ // to work with the returned errors yet.
+ var check int
+ row := tx.QueryRow("SELECT 1 FROM PodConfig WHERE Name=?;", pod.Name())
+ if err := row.Scan(&check); err != nil {
+ if !errors.Is(err, sql.ErrNoRows) {
+ return fmt.Errorf("checking if pod name %s exists in database: %w", pod.ID(), err)
+ }
+ } else if check != 0 {
+ return fmt.Errorf("name \"%s\" is in use: %w", pod.Name(), define.ErrPodExists)
+ }
+
if _, err := tx.Exec("INSERT INTO IDNamespace VALUES (?);", pod.ID()); err != nil {
return fmt.Errorf("adding pod id to database: %w", err)
}
From 69ff04f7360f2a76a3255291c8bc00da62aea1dc Mon Sep 17 00:00:00 2001
From: Valentin Rothberg <vrothberg@redhat.com>
Date: Mon, 27 Feb 2023 16:55:18 +0100
Subject: [PATCH 3/9] sqlite: fix type rewriting container config
It's `UPDATE $NAME` not `UPDATE TABLE $NAME`.
[NO NEW TESTS NEEDED]
Signed-off-by: Valentin Rothberg <vrothberg@redhat.com>
---
libpod/sqlite_state_internal.go | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/libpod/sqlite_state_internal.go b/libpod/sqlite_state_internal.go
index 1128d01c77..358a15ec8c 100644
--- a/libpod/sqlite_state_internal.go
+++ b/libpod/sqlite_state_internal.go
@@ -318,7 +318,7 @@ func (s *SQLiteState) rewriteContainerConfig(ctr *Container, newCfg *ContainerCo
}
}()
- results, err := tx.Exec("UPDATE TABLE ContainerConfig SET Name=?, JSON=? WHERE ID=?;", newCfg.Name, json, ctr.ID())
+ results, err := tx.Exec("UPDATE ContainerConfig SET Name=?, JSON=? WHERE ID=?;", newCfg.Name, json, ctr.ID())
if err != nil {
return fmt.Errorf("updating container config table with new configuration for container %s: %w", ctr.ID(), err)
}
From 01359457c44a468f2dbaa3613e9a8158b46dac4f Mon Sep 17 00:00:00 2001
From: Valentin Rothberg <vrothberg@redhat.com>
Date: Tue, 28 Feb 2023 09:55:11 +0100
Subject: [PATCH 4/9] sqlite: LookupVolume: wrap error
Wrap the error with the message expexted by the system tests.
[NO NEW TESTS NEEDED]
Signed-off-by: Valentin Rothberg <vrothberg@redhat.com>
---
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 b86b648996..2ae010c08d 100644
--- a/libpod/sqlite_state.go
+++ b/libpod/sqlite_state.go
@@ -1477,7 +1477,7 @@ func (s *SQLiteState) AddPod(pod *Pod) (defErr error) {
row := tx.QueryRow("SELECT 1 FROM PodConfig WHERE Name=?;", pod.Name())
if err := row.Scan(&check); err != nil {
if !errors.Is(err, sql.ErrNoRows) {
- return fmt.Errorf("checking if pod name %s exists in database: %w", pod.ID(), err)
+ return fmt.Errorf("checking if pod name %s exists in database: %w", pod.Name(), err)
}
} else if check != 0 {
return fmt.Errorf("name \"%s\" is in use: %w", pod.Name(), define.ErrPodExists)
@@ -2064,7 +2064,7 @@ func (s *SQLiteState) LookupVolume(name string) (*Volume, error) {
foundResult = true
}
if !foundResult {
- return nil, define.ErrNoSuchVolume
+ return nil, fmt.Errorf("no volume with name %q found: %w", name, define.ErrNoSuchVolume)
}
vol := new(Volume)
From df88f546b69a8c2c524b73d86f51ec65bb3390f4 Mon Sep 17 00:00:00 2001
From: Valentin Rothberg <vrothberg@redhat.com>
Date: Tue, 28 Feb 2023 10:43:37 +0100
Subject: [PATCH 5/9] sqlite: LookupVolume: fix partial name match
A partial name match is tricky as we want it to be fast but also make
sure there's only one partial match iff there's no full one.
[NO NEW TESTS NEEDED] as it fixes a system test.
Signed-off-by: Valentin Rothberg <vrothberg@redhat.com>
---
libpod/sqlite_state.go | 15 ++++++++-------
1 file changed, 8 insertions(+), 7 deletions(-)
diff --git a/libpod/sqlite_state.go b/libpod/sqlite_state.go
index 2ae010c08d..499d1b966f 100644
--- a/libpod/sqlite_state.go
+++ b/libpod/sqlite_state.go
@@ -2046,24 +2046,25 @@ 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 Name, JSON FROM VolumeConfig WHERE Name LIKE ? ORDER BY LENGTH(Name) ASC;", name+"%")
if err != nil {
return nil, fmt.Errorf("querying database for volume %s: %w", name, err)
}
defer rows.Close()
- var configJSON string
- foundResult := false
+ var foundName, configJSON string
for rows.Next() {
- if foundResult {
+ if foundName != "" {
return nil, fmt.Errorf("more than one result for volume name %s: %w", name, define.ErrVolumeExists)
}
- if err := rows.Scan(&configJSON); err != nil {
+ if err := rows.Scan(&foundName, &configJSON); err != nil {
return nil, fmt.Errorf("retrieving volume %s config from database: %w", name, err)
}
- foundResult = true
+ if foundName == name {
+ break
+ }
}
- if !foundResult {
+ if foundName == "" {
return nil, fmt.Errorf("no volume with name %q found: %w", name, define.ErrNoSuchVolume)
}
From 86d12520e95e0b23b7fc272c6e1b66f4bd48ef92 Mon Sep 17 00:00:00 2001
From: Valentin Rothberg <vrothberg@redhat.com>
Date: Tue, 28 Feb 2023 11:03:52 +0100
Subject: [PATCH 6/9] sqlite: implement RewriteVolumeConfig
[NO NEW TESTS NEEDED]
Signed-off-by: Valentin Rothberg <vrothberg@redhat.com>
---
libpod/sqlite_state.go | 70 ++++++++++++++++++++++--------------------
1 file changed, 37 insertions(+), 33 deletions(-)
diff --git a/libpod/sqlite_state.go b/libpod/sqlite_state.go
index 499d1b966f..4dfc2415f0 100644
--- a/libpod/sqlite_state.go
+++ b/libpod/sqlite_state.go
@@ -1199,8 +1199,7 @@ func (s *SQLiteState) RewritePodConfig(pod *Pod, newCfg *PodConfig) (defErr erro
// RewriteVolumeConfig rewrites a volume'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) RewriteVolumeConfig(volume *Volume, newCfg *VolumeConfig) error {
+func (s *SQLiteState) RewriteVolumeConfig(volume *Volume, newCfg *VolumeConfig) (defErr error) {
if !s.valid {
return define.ErrDBClosed
}
@@ -1209,38 +1208,43 @@ func (s *SQLiteState) RewriteVolumeConfig(volume *Volume, newCfg *VolumeConfig)
return define.ErrVolumeRemoved
}
+ json, err := json.Marshal(newCfg)
+ if err != nil {
+ return fmt.Errorf("error marshalling volume %s new config JSON: %w", volume.Name(), err)
+ }
+
+ tx, err := s.conn.Begin()
+ if err != nil {
+ return fmt.Errorf("beginning transaction to rewrite volume %s config: %w", volume.Name(), err)
+ }
+ defer func() {
+ if defErr != nil {
+ if err := tx.Rollback(); err != nil {
+ logrus.Errorf("Rolling back transaction to rewrite volume %s config: %v", volume.Name(), err)
+ }
+ }
+ }()
+
+ results, err := tx.Exec("UPDATE VolumeConfig SET Name=?, JSON=? WHERE ID=?;", newCfg.Name, json, volume.Name())
+ if err != nil {
+ return fmt.Errorf("updating volume config table with new configuration for volume %s: %w", volume.Name(), err)
+ }
+ rows, err := results.RowsAffected()
+ if err != nil {
+ return fmt.Errorf("retrieving volume %s config rewrite rows affected: %w", volume.Name(), err)
+ }
+ if rows == 0 {
+ volume.valid = false
+ return fmt.Errorf("no volume with name %q found in DB: %w", volume.Name(), define.ErrNoSuchVolume)
+ }
+
+ if err := tx.Commit(); err != nil {
+ return fmt.Errorf("committing transaction to rewrite volume %s config: %w", volume.Name(), err)
+ }
+
+ return nil
+
return define.ErrNotImplemented
-
- // newCfgJSON, err := json.Marshal(newCfg)
- // if err != nil {
- // return fmt.Errorf("marshalling new configuration JSON for volume %q: %w", volume.Name(), err)
- // }
-
- // db, err := s.getDBCon()
- // if err != nil {
- // return err
- // }
- // defer s.deferredCloseDBCon(db)
-
- // err = db.Update(func(tx *bolt.Tx) error {
- // volBkt, err := getVolBucket(tx)
- // if err != nil {
- // return err
- // }
-
- // volDB := volBkt.Bucket([]byte(volume.Name()))
- // if volDB == nil {
- // volume.valid = false
- // return fmt.Errorf("no volume with name %q found in DB: %w", volume.Name(), define.ErrNoSuchVolume)
- // }
-
- // if err := volDB.Put(configKey, newCfgJSON); err != nil {
- // return fmt.Errorf("updating volume %q config JSON: %w", volume.Name(), err)
- // }
-
- // return nil
- // })
- // return err
}
// Pod retrieves a pod given its full ID
From 2342d1a314f3c930dd36dbb5ad628cd7bf42d2a0 Mon Sep 17 00:00:00 2001
From: Valentin Rothberg <vrothberg@redhat.com>
Date: Tue, 28 Feb 2023 13:21:06 +0100
Subject: [PATCH 7/9] sqlite: addContainer: add named volume only once
There's a unique constraint in the table, so we shouldn't add the same
volume more than once to the same container.
[NO NEW TESTS NEEDED] as it fixes an existing one.
Signed-off-by: Valentin Rothberg <vrothberg@redhat.com>
---
libpod/sqlite_state_internal.go | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/libpod/sqlite_state_internal.go b/libpod/sqlite_state_internal.go
index 358a15ec8c..b13cc8b62e 100644
--- a/libpod/sqlite_state_internal.go
+++ b/libpod/sqlite_state_internal.go
@@ -399,9 +399,13 @@ func (s *SQLiteState) addContainer(ctr *Container) (defErr error) {
return fmt.Errorf("adding container dependency %s to database: %w", dep, err)
}
}
+ volMap := make(map[string]bool)
for _, vol := range ctr.config.NamedVolumes {
- if _, err := tx.Exec("INSERT INTO ContainerVolume VALUES (?, ?);", ctr.ID(), vol.Name); err != nil {
- return fmt.Errorf("adding container volume %s to database: %w", vol.Name, err)
+ if _, ok := volMap[vol.Name]; !ok {
+ if _, err := tx.Exec("INSERT INTO ContainerVolume VALUES (?, ?);", ctr.ID(), vol.Name); err != nil {
+ return fmt.Errorf("adding container volume %s to database: %w", vol.Name, err)
+ }
+ volMap[vol.Name] = true
}
}
From 38acab832df2a4e5ea24461a64b6b16f5a4d056f Mon Sep 17 00:00:00 2001
From: Valentin Rothberg <vrothberg@redhat.com>
Date: Tue, 28 Feb 2023 16:24:53 +0100
Subject: [PATCH 8/9] sqlite: remove dead code
Found by golangci-lint.
Signed-off-by: Valentin Rothberg <vrothberg@redhat.com>
---
libpod/sqlite_state.go | 2 --
1 file changed, 2 deletions(-)
diff --git a/libpod/sqlite_state.go b/libpod/sqlite_state.go
index 4dfc2415f0..881731f828 100644
--- a/libpod/sqlite_state.go
+++ b/libpod/sqlite_state.go
@@ -1243,8 +1243,6 @@ func (s *SQLiteState) RewriteVolumeConfig(volume *Volume, newCfg *VolumeConfig)
}
return nil
-
- return define.ErrNotImplemented
}
// Pod retrieves a pod given its full ID
From 2c67ff5d401c7853448ee15383400e8e4b389559 Mon Sep 17 00:00:00 2001
From: Valentin Rothberg <vrothberg@redhat.com>
Date: Wed, 1 Mar 2023 11:30:20 +0100
Subject: [PATCH 9/9] sqlite: add container short ID to network aliases
Signed-off-by: Valentin Rothberg <vrothberg@redhat.com>
---
libpod/sqlite_state_internal.go | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/libpod/sqlite_state_internal.go b/libpod/sqlite_state_internal.go
index b13cc8b62e..bc8b966c9e 100644
--- a/libpod/sqlite_state_internal.go
+++ b/libpod/sqlite_state_internal.go
@@ -339,6 +339,12 @@ func (s *SQLiteState) rewriteContainerConfig(ctr *Container, newCfg *ContainerCo
}
func (s *SQLiteState) addContainer(ctr *Container) (defErr error) {
+ for net := range ctr.config.Networks {
+ opts := ctr.config.Networks[net]
+ opts.Aliases = append(opts.Aliases, ctr.config.ID[:12])
+ ctr.config.Networks[net] = opts
+ }
+
configJSON, err := json.Marshal(ctr.config)
if err != nil {
return fmt.Errorf("marshalling container config json: %w", err)
@@ -503,6 +509,9 @@ func (s *SQLiteState) networkModify(ctr *Container, network string, opts types.P
}
if !disconnect {
+ if newCfg.Networks == nil {
+ newCfg.Networks = make(map[string]types.PerNetworkOptions)
+ }
newCfg.Networks[network] = opts
} else {
delete(newCfg.Networks, network)