Replace rqlite with modernc.org/sqlite

[s3rj1k]

Description

Fixes:

• Check if possible to sunset cgo sqlite3 dependency #3051

Type of change

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)
• Documentation update

How Has This Been Tested?

• Manual test
• Auto test added

Checklist

• My code follows the style guidelines of this project
• My commit messages are signed-off
• I have performed a self-review of my code
• I have commented my code, particularly in hard-to-understand areas
• I have made corresponding changes to the documentation
• My changes generate no new warnings
• I have added tests that prove my fix is effective or that my feature works
• New and existing unit tests pass locally with my changes
• Any dependent changes have been merged and published in downstream modules
• I have checked my code and corrected any misspellings

[twz123]

Thanks for working on this. Has been in the backlog since forever ^^

[twz123]

I've seen that modernc.org/sqlite has Go APIs for taking backups. Would those be beneficial? I'm a bit concerned about the "SQL injection" here. At the very least, I'd ask for proper path escaping or using a proper SQL query parameter via the driver.

[s3rj1k]

I does look like that modernc.org/sqlite is backup and not vacuum, https://sqlite.org/lang_vacuum.html

I think we want VACUUM and not Backup.

for SQL injection part we can add check of isAbsPath and name only contain [0-9A-Za-z]

WDYT?

[twz123]

I does look like that modernc.org/sqlite is backup and not vacuum, https://sqlite.org/lang_vacuum.html

I think we want VACUUM and not Backup.

Right. I think we might actually want backup, because I think this refers to SQLite's online backup functionality, whereas VACUUM might interfere with ongoing transactions and so on. We could, however, VACCUM the backed up database file offline, afterwards. I might be wrong here, though. Maybe @kke has an opinion on this?

for SQL injection part we can add check of isAbsPath and name only contain [0-9A-Za-z]

I wonder if this works:

Suggested change

	_, err := db.Exec(fmt.Sprintf("VACUUM INTO '%s'", path))
	_, err := db.Exec("VACUUM INTO ?", path)

I've checked, and AFAICT, there's no backup/restore integration test for kine, only for etcd. So we might want to add a proper inttest for this first.

[twz123]

I've quickly hacked together something for the backup inttest:

diff

diff --git a/inttest/backup/backup_test.go b/inttest/backup/backup_test.go
index afb4017de..856057fda 100644
--- a/inttest/backup/backup_test.go
+++ b/inttest/backup/backup_test.go
@@ -4,64 +4,69 @@
 package basic
 
 import (
-	"bytes"
-	"html/template"
 	"testing"
 	"time"
 
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/client-go/kubernetes"
 
+	"github.com/ghodss/yaml"
 	"github.com/stretchr/testify/suite"
 
 	"github.com/k0sproject/k0s/inttest/common"
+	"github.com/k0sproject/k0s/pkg/apis/k0s/v1beta1"
 	v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
 
-const configWithExternaladdress = `
-apiVersion: k0s.k0sproject.io/v1beta1
-kind: ClusterConfig
-metadata:
-  name: k0s
-spec:
-  api:
-    externalAddress: {{ .Address }}
-`
-
 type BackupSuite struct {
 	common.BootlooseSuite
 	backupFunc  func() error
 	restoreFunc func() error
+	useKine     bool
 }
 
 func (s *BackupSuite) getControllerConfig(ipAddress string) string {
-	data := struct {
-		Address string
-	}{
-		Address: ipAddress,
+	config := v1beta1.ClusterConfig{
+		Spec: &v1beta1.ClusterSpec{
+			API: &v1beta1.APISpec{
+				ExternalAddress: ipAddress,
+			},
+		},
 	}
-	content := bytes.NewBuffer([]byte{})
-	s.Require().NoError(template.Must(template.New("k0s.yaml").Parse(configWithExternaladdress)).Execute(content, data), "can't execute k0s.yaml template")
-	return content.String()
+
+	if s.useKine {
+		config.Spec.Storage = &v1beta1.StorageSpec{
+			Type: v1beta1.KineStorageType,
+		}
+	}
+
+	yaml, err := yaml.Marshal(&config)
+	s.Require().NoError(err)
+	return string(yaml)
 }
 
 func (s *BackupSuite) TestK0sGetsUp() {
 	ipAddress := s.GetControllerIPAddress(0)
 	s.T().Logf("ip address: %s", ipAddress)
 	config := s.getControllerConfig(ipAddress)
-	s.PutFile("controller0", "/tmp/k0s.yaml", config)
-	s.PutFile("controller1", "/tmp/k0s.yaml", config)
+	s.T().Log("Config:", config)
+	s.PutFile(s.ControllerNode(0), "/tmp/k0s.yaml", config)
 
 	s.Require().NoError(s.InitController(0, "--config=/tmp/k0s.yaml", "--enable-worker"))
 	s.Require().NoError(s.RunWorkers())
 
 	kc, err := s.KubeClient(s.ControllerNode(0))
 	s.Require().NoError(err)
-	s.Require().NoError(s.WaitJoinAPI(s.ControllerNode(0)))
-	token, err := s.GetJoinToken("controller")
-	s.Require().NoError(err)
-	s.Require().NoError(s.InitController(1, token, "--config=/tmp/k0s.yaml"))
-	s.Require().NoError(s.WaitJoinAPI(s.ControllerNode(1)))
+
+	var token string
+	if s.ControllerCount > 1 {
+		s.PutFile(s.ControllerNode(1), "/tmp/k0s.yaml", config)
+		s.Require().NoError(s.WaitJoinAPI(s.ControllerNode(1)))
+		token, err = s.GetJoinToken("controller")
+		s.Require().NoError(err)
+		s.Require().NoError(s.InitController(1, token, "--config=/tmp/k0s.yaml"))
+		s.Require().NoError(s.WaitJoinAPI(s.ControllerNode(1)))
+	}
 
 	s.Require().NoError(s.WaitForNodeReady(s.WorkerNode(0), kc))
 	s.Require().NoError(s.WaitForNodeReady(s.WorkerNode(1), kc))
@@ -76,17 +81,21 @@ func (s *BackupSuite) TestK0sGetsUp() {
 	snapshot := s.makeSnapshot(kc)
 
 	s.Require().NoError(s.StopController(s.ControllerNode(0)))
-	_ = s.StopController(s.ControllerNode(1)) // No error check as k0s might have actually exited since etcd is not really happy
-
 	s.reset(s.ControllerNode(0))
-	s.reset(s.ControllerNode(1))
+
+	if s.ControllerCount > 1 {
+		_ = s.StopController(s.ControllerNode(1)) // No error check as k0s might have actually exited since etcd is not really happy
+		s.reset(s.ControllerNode(1))
+	}
 
 	s.Require().NoError(s.restoreFunc())
 	s.Require().NoError(s.InitController(0, "--enable-worker"))
 	s.Require().NoError(s.WaitJoinAPI(s.ControllerNode(0)))
 
-	// Join the second controller as normally
-	s.Require().NoError(s.InitController(1, "--enable-worker", token))
+	if s.ControllerCount > 1 {
+		// Join the second controller as normally
+		s.Require().NoError(s.InitController(1, "--enable-worker", token))
+	}
 
 	s.Require().NoError(s.WaitForNodeReady(s.WorkerNode(0), kc))
 	s.Require().NoError(s.WaitForNodeReady(s.WorkerNode(1), kc))
@@ -232,26 +241,39 @@ func (s *BackupSuite) restoreBackupStdin() error {
 	return nil
 }
 
-func TestBackupSuite(t *testing.T) {
+// func TestBackupSuite(t *testing.T) {
+// 	s := BackupSuite{
+// 		BootlooseSuite: common.BootlooseSuite{
+// 			ControllerCount: 2,
+// 			WorkerCount:     2,
+// 		},
+// 	}
+// 	s.backupFunc = s.takeBackup
+// 	s.restoreFunc = s.restoreBackup
+// 	suite.Run(t, &s)
+// }
+
+func TestBackupSuiteKine(t *testing.T) {
 	s := BackupSuite{
 		BootlooseSuite: common.BootlooseSuite{
-			ControllerCount: 2,
+			ControllerCount: 1,
 			WorkerCount:     2,
 		},
+		useKine: true,
 	}
 	s.backupFunc = s.takeBackup
 	s.restoreFunc = s.restoreBackup
 	suite.Run(t, &s)
 }
 
-func TestBackupSuiteStream(t *testing.T) {
-	s := BackupSuite{
-		BootlooseSuite: common.BootlooseSuite{
-			ControllerCount: 2,
-			WorkerCount:     2,
-		},
-	}
-	s.backupFunc = s.takeBackupStdout
-	s.restoreFunc = s.restoreBackupStdin
-	suite.Run(t, &s)
-}
+// func TestBackupSuiteStream(t *testing.T) {
+// 	s := BackupSuite{
+// 		BootlooseSuite: common.BootlooseSuite{
+// 			ControllerCount: 2,
+// 			WorkerCount:     2,
+// 		},
+// 	}
+// 	s.backupFunc = s.takeBackupStdout
+// 	s.restoreFunc = s.restoreBackupStdin
+// 	suite.Run(t, &s)
+// }

The above is hanging even on the current main branch for non-obvious reasons. Need to debug this.

[s3rj1k]

FYI: It seems that VACUUM is a actually a modern way of doing sqlite backups, it just does everything you need to do vs using actual backup

The VACUUM command with an INTO clause is an alternative to the [backup API](https://sqlite.org/backup.html) for generating backup copies of a live database. The advantage of using VACUUM INTO is that the resulting backup database is minimal in size and hence the amount of filesystem I/O may be reduced. Also, all deleted content is purged from the backup, leaving behind no forensic traces. On the other hand, the [backup API](https://sqlite.org/backup.html) uses fewer CPU cycles and can be executed incrementally.

from https://sqlite.org/lang_vacuum.html

[s3rj1k]

I wonder if this works:
_, err := db.Exec("VACUUM INTO ?", path)

https://pkg.go.dev/database/sql#DB.Exec says yes, but it's a driver specific thing

[twz123]

The missing integration test is here: #6568

[twz123]

@s3rj1k #6568 has been merged. Would you be able to rebase this and address the few nits?

[s3rj1k]

@s3rj1k #6568 has been merged. Would you be able to rebase this and address the few nits?

Done, except that VACUUM change, lets test that a bit later