Don't Panic in getComposeStatus and skip invalid jobs in fsjobqueue New

This handles corrupt job json files by skipping them. They still exist, and errors are logged, but the system keeps working. If one or more of the json files in /var/lib/osbuild-composer/jobs/ becomes corrupt they can stop the osbuild-composer service from starting, or stop commands like 'composer-cli compose status' from working because they quit on the first error and miss any job that aren't broken.
osbuild · Nov 15, 2023 · 5ed1bc2 · 5ed1bc2
1 parent 6649e7b
commit 5ed1bc2
Show file tree

Hide file tree

Showing 6 changed files with 291 additions and 18 deletions.
diff --git a/internal/jobqueue/fsjobqueue/fsjobqueue.go b/internal/jobqueue/fsjobqueue/fsjobqueue.go
@@ -106,7 +106,8 @@ func New(dir string) (*fsJobQueue, error) {
 		}
 		j, err := q.readJob(jobId)
 		if err != nil {
-			return nil, err
+			// Skip invalid jobs, leaving them in place for later examination
+			continue
 		}
 
 		// If a job is running, and not cancelled, track the token

diff --git a/internal/jobqueue/fsjobqueue/fsjobqueue_test.go b/internal/jobqueue/fsjobqueue/fsjobqueue_test.go
@@ -1,6 +1,8 @@
 package fsjobqueue_test
 
 import (
+	"os"
+	"path"
 	"testing"
 
 	"github.com/osbuild/osbuild-composer/pkg/jobqueue"
@@ -28,3 +30,15 @@ func TestNonExistant(t *testing.T) {
 	require.Error(t, err)
 	require.Nil(t, q)
 }
+
+func TestJobQueueBadJSON(t *testing.T) {
+	dir := t.TempDir()
+
+	// Write a purposfully invalid JSON file into the queue
+	err := os.WriteFile(path.Join(dir, "/4f1cf5f8-525d-46b7-aef4-33c6a919c038.json"), []byte("{invalid json content"), 0600)
+	require.Nil(t, err)
+
+	q, err := fsjobqueue.New(dir)
+	require.Nil(t, err)
+	require.NotNil(t, q)
+}
diff --git a/internal/mocks/rpmmd/fixtures.go b/internal/mocks/rpmmd/fixtures.go
@@ -1,6 +1,9 @@
 package rpmmd_mock
 
 import (
+	"os"
+	"path"
+
 	"github.com/osbuild/osbuild-composer/internal/jobqueue/fsjobqueue"
 	dnfjson_mock "github.com/osbuild/osbuild-composer/internal/mocks/dnfjson"
 	"github.com/osbuild/osbuild-composer/internal/store"
@@ -64,3 +67,20 @@ func OldChangesFixture(tmpdir string) Fixture {
 		dnfjson_mock.Base,
 	}
 }
+
+func BadJobJSONFixture(tmpdir string) Fixture {
+	err := os.Mkdir(path.Join(tmpdir, "/jobs"), 0755)
+	if err != nil {
+		panic(err)
+	}
+	err = os.WriteFile(path.Join(tmpdir, "/jobs/30000000-0000-0000-0000-000000000005.json"), []byte("{invalid json content"), 0600)
+	if err != nil {
+		panic(err)
+	}
+
+	return Fixture{
+		store.FixtureJobs(),
+		createBaseWorkersFixture(path.Join(tmpdir, "/jobs")),
+		dnfjson_mock.Base,
+	}
+}
diff --git a/internal/store/fixtures.go b/internal/store/fixtures.go
@@ -354,3 +354,152 @@ func FixtureOldChanges() *Store {
 
 	return s
 }
+
+// Fixture to use for checking job queue files
+func FixtureJobs() *Store {
+	var bName = "test"
+	var b = blueprint.Blueprint{
+		Name:           bName,
+		Version:        "0.0.0",
+		Packages:       []blueprint.Package{},
+		Modules:        []blueprint.Package{},
+		Groups:         []blueprint.Group{},
+		Customizations: nil,
+	}
+
+	var date = time.Date(2019, 11, 27, 13, 19, 0, 0, time.FixedZone("UTC+1", 60*60))
+
+	var awsTarget = &target.Target{
+		Uuid:      uuid.MustParse("10000000-0000-0000-0000-000000000000"),
+		Name:      target.TargetNameAWS,
+		ImageName: "awsimage",
+		Created:   date,
+		Status:    common.IBWaiting,
+		Options: &target.AWSTargetOptions{
+			Region:          "frankfurt",
+			AccessKeyID:     "accesskey",
+			SecretAccessKey: "secretkey",
+			Bucket:          "clay",
+			Key:             "imagekey",
+		},
+	}
+
+	dr := test_distro.NewRegistry()
+	d := dr.FromHost()
+	arch, err := d.GetArch(test_distro.TestArchName)
+	if err != nil {
+		panic(fmt.Sprintf("failed to get architecture %s for a test distro: %v", test_distro.TestArchName, err))
+	}
+	imgType, err := arch.GetImageType(test_distro.TestImageTypeName)
+	if err != nil {
+		panic(fmt.Sprintf("failed to get image type %s for a test distro architecture: %v", test_distro.TestImageTypeName, err))
+	}
+	manifest, _, err := imgType.Manifest(nil, distro.ImageOptions{}, nil, 0)
+	if err != nil {
+		panic(fmt.Sprintf("failed to create a manifest: %v", err))
+	}
+
+	mf, err := manifest.Serialize(nil, nil, nil)
+	if err != nil {
+		panic(fmt.Sprintf("failed to create a manifest: %v", err))
+	}
+
+	s := New(nil, dr, nil)
+
+	pkgs := []rpmmd.PackageSpec{
+		{
+			Name:    "test1",
+			Epoch:   0,
+			Version: "2.11.2",
+			Release: "1.fc35",
+			Arch:    test_distro.TestArchName,
+		}, {
+			Name:    "test2",
+			Epoch:   3,
+			Version: "4.2.2",
+			Release: "1.fc35",
+			Arch:    test_distro.TestArchName,
+		}}
+
+	s.blueprints[bName] = b
+	s.composes = map[uuid.UUID]Compose{
+		uuid.MustParse("30000000-0000-0000-0000-000000000000"): {
+			Blueprint: &b,
+			ImageBuild: ImageBuild{
+				QueueStatus: common.IBWaiting,
+				ImageType:   imgType,
+				Manifest:    mf,
+				Targets:     []*target.Target{awsTarget},
+				JobCreated:  date,
+			},
+			Packages: []rpmmd.PackageSpec{},
+		},
+		uuid.MustParse("30000000-0000-0000-0000-000000000001"): {
+			Blueprint: &b,
+			ImageBuild: ImageBuild{
+				QueueStatus: common.IBRunning,
+				ImageType:   imgType,
+				Manifest:    mf,
+				Targets:     []*target.Target{},
+				JobCreated:  date,
+				JobStarted:  date,
+			},
+			Packages: []rpmmd.PackageSpec{},
+		},
+		uuid.MustParse("30000000-0000-0000-0000-000000000002"): {
+			Blueprint: &b,
+			ImageBuild: ImageBuild{
+				QueueStatus: common.IBFinished,
+				ImageType:   imgType,
+				Manifest:    mf,
+				Targets:     []*target.Target{awsTarget},
+				JobCreated:  date,
+				JobStarted:  date,
+				JobFinished: date,
+			},
+			Packages: []rpmmd.PackageSpec{},
+		},
+		uuid.MustParse("30000000-0000-0000-0000-000000000003"): {
+			Blueprint: &b,
+			ImageBuild: ImageBuild{
+				QueueStatus: common.IBFailed,
+				ImageType:   imgType,
+				Manifest:    mf,
+				Targets:     []*target.Target{awsTarget},
+				JobCreated:  date,
+				JobStarted:  date,
+				JobFinished: date,
+			},
+			Packages: []rpmmd.PackageSpec{},
+		},
+		uuid.MustParse("30000000-0000-0000-0000-000000000004"): {
+			Blueprint: &b,
+			ImageBuild: ImageBuild{
+				QueueStatus: common.IBFinished,
+				ImageType:   imgType,
+				Manifest:    mf,
+				Targets:     []*target.Target{awsTarget},
+				JobCreated:  date,
+				JobStarted:  date,
+				JobFinished: date,
+			},
+			Packages: pkgs,
+		},
+		uuid.MustParse("30000000-0000-0000-0000-000000000005"): {
+			Blueprint: &b,
+			ImageBuild: ImageBuild{
+				QueueStatus: common.IBFinished,
+				ImageType:   imgType,
+				Manifest:    mf,
+				Targets:     []*target.Target{awsTarget},
+				JobCreated:  date,
+				JobStarted:  date,
+				JobFinished: date,
+				JobID:       uuid.MustParse("30000000-0000-0000-0000-000000000005"),
+			},
+			Packages: pkgs,
+		},
+	}
+
+	return s
+}