Esrey/userspace convertor/unit tests

cmd/convertor/builder/ToTest.md

@@ -0,0 +1,46 @@
+This PR is meant to add some unit testing for some of the core portions of the userspace conversion. I have added some notes below on the added tests and notes on the remaining tests.
+
+## builder_engine
+ - uploadManifestAndConfig -> Added test
+ - getBuilderEngineBase -> Added test
+ - isGzipLayer -> Added Test
+
+## builder_utils.go
+- fetch - Covered by fetchManifest and fetchConfig
+- fetchManifest -> Added test
+- fetchConfig -> Added test
+- fetchManifestAndConfig -> Seemed unecessary
+- downloadLayer -> Added test
+- writeConfig -> Added test
+- getFileDesc -> Added test
+- uploadBlob -> Added test
+- uploadBytes -> Added test
+- buildArchiveFromFiles -> Left For later
+- addFileToArchive -> Left For later
+
+## builder.go
+- build -> Added test
+
+## overlaybd_builder.go
+- uploadBaseLayer -> Added test
+- checkForConvertedLayer -> Added test
+- storeConvertedLayer -> Added test
+I am not currenly sure how best to add the remaining functions due to their use of the overlaybd binaries. In the near term these should be covered by unit tests but leaving as a future work item.
+
+## fastoci_builder.go
+Similar problem to above. Leaving for later.
+
+## End to End tests
+- The existing CI seems like the best place to add end to end tests. TBD.
+
+# Introduced Tools
+Tests for the userspace convertor are not particularly simple to make, if only because they require a lot of setup and work on both the filesystem and remote sources. To help with this I have introduced a few tools to help with testing.
+
+## local Remotes
+This is an abstraction to interact with a local registry. The registry itself supports fetching, resolving, and pushes. See the local_registry.go file for more info. Along with this there is a mocks/registry folder which allows us to load stored images into the local registry for testing for now I've kept the added images small and restricted to hello-world but more can be added easily.
+
+# Filesystem interactivity
+test_utils.go introduces RunTestWithTempDir which is a helper emulating the snapshotter tests that allows us to run a test with a temporary directory. This is useful for testing the filesystem interactions of several of the functions.
+
+# Other
+Theres also a small bugfix to builder to remove a small contention issue found while testing.

cmd/convertor/builder/builder.go

@@ -115,7 +115,11 @@ func (b *overlaybdBuilder) Build(ctx context.Context) error {
 				// in the event of failure fallback to regular process
 				return nil
 			}
-			alreadyConverted[idx] <- &desc
+			select {
+			case <-rctx.Done():
+			case alreadyConverted[idx] <- &desc:
+			}
+
 			return nil
 		})
 
@@ -133,7 +137,8 @@ func (b *overlaybdBuilder) Build(ctx context.Context) error {
 				err := b.engine.DownloadConvertedLayer(rctx, idx, *cachedLayer)
 				if err == nil {
 					logrus.Infof("downloaded cached layer %d", idx)
-					return err
+					sendToChannel(rctx, downloaded[idx], nil)
+					return nil
 				}
 				logrus.Infof("failed to download cached layer %d falling back to conversion : %s", idx, err)
 			}

cmd/convertor/builder/builder_engine_test.go

@@ -0,0 +1,158 @@
+/*
+   Copyright The Accelerated Container Image Authors
+
+   Licensed under the Apache License, Version 2.0 (the "License");
+   you may not use this file except in compliance with the License.
+   You may obtain a copy of the License at
+
+       http://www.apache.org/licenses/LICENSE-2.0
+
+   Unless required by applicable law or agreed to in writing, software
+   distributed under the License is distributed on an "AS IS" BASIS,
+   WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+   See the License for the specific language governing permissions and
+   limitations under the License.
+*/
+
+package builder
+
+import (
+	"context"
+	"encoding/json"
+	"fmt"
+	"testing"
+
+	testingresources "github.com/containerd/accelerated-container-image/cmd/convertor/testingresources"
+	_ "github.com/containerd/containerd/pkg/testutil" // Handle custom root flag
+	"github.com/containerd/containerd/remotes"
+	"github.com/opencontainers/go-digest"
+	specs "github.com/opencontainers/image-spec/specs-go/v1"
+	v1 "github.com/opencontainers/image-spec/specs-go/v1"
+)
+
+func Test_builderEngineBase_isGzipLayer(t *testing.T) {
+	ctx := context.Background()
+	resolver := testingresources.GetTestResolver(t, ctx)
+
+	type fields struct {
+		fetcher  remotes.Fetcher
+		manifest specs.Manifest
+	}
+	getFields := func(ctx context.Context, ref string) fields {
+		_, desc, err := resolver.Resolve(ctx, ref)
+		if err != nil {
+			t.Error(err)
+		}
+
+		fetcher, err := resolver.Fetcher(ctx, ref)
+		if err != nil {
+			t.Error(err)
+		}
+
+		manifestStream, err := fetcher.Fetch(ctx, desc)
+		if err != nil {
+			t.Error(err)
+		}
+
+		if err != nil {
+			t.Error(err)
+		}
+
+		parsedManifest := v1.Manifest{}
+		decoder := json.NewDecoder(manifestStream)
+		if err = decoder.Decode(&parsedManifest); err != nil {
+			t.Error(err)
+		}
+
+		return fields{
+			fetcher:  fetcher,
+			manifest: parsedManifest,
+		}
+	}
+
+	type args struct {
+		ctx context.Context
+		idx int
+	}
+	tests := []struct {
+		name    string
+		fields  fields
+		args    args
+		want    bool
+		wantErr bool
+	}{
+		// TODO Add more layers types for validation
+		// Unknown Layer Type
+		// Uncompressed Layer Type
+		{
+			name:   "Valid Gzip Layer",
+			fields: getFields(ctx, testingresources.DockerV2_Manifest_Simple_Ref),
+			args: args{
+				ctx: ctx,
+				idx: 0,
+			},
+			want:    true,
+			wantErr: false,
+		},
+		{
+			name: "Layer Not Found",
+			fields: func() fields {
+				fields := getFields(ctx, testingresources.DockerV2_Manifest_Simple_Ref)
+				fields.manifest.Layers[0].Digest = digest.FromString("sample")
+				return fields
+			}(),
+			args: args{
+				ctx: ctx,
+				idx: 0,
+			},
+			want:    false,
+			wantErr: true,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			e := &builderEngineBase{
+				fetcher:  tt.fields.fetcher,
+				manifest: tt.fields.manifest,
+			}
+			got, err := e.isGzipLayer(tt.args.ctx, tt.args.idx)
+			if (err != nil) != tt.wantErr {
+				t.Errorf("builderEngineBase.isGzipLayer() error = %v, wantErr %v", err, tt.wantErr)
+				return
+			}
+			if got != tt.want {
+				t.Errorf("builderEngineBase.isGzipLayer() = %v, want %v", got, tt.want)
+			}
+		})
+	}
+}
+
+func Test_getBuilderEngineBase(t *testing.T) {
+	resolver := testingresources.GetTestResolver(t, context.Background())
+	engine, err := getBuilderEngineBase(context.TODO(),
+		resolver,
+		testingresources.DockerV2_Manifest_Simple_Ref,
+		fmt.Sprintf("%s-obd", testingresources.DockerV2_Manifest_Simple_Ref),
+	)
+	if err != nil {
+		t.Error(err)
+	}
+
+	testingresources.Assert(t, engine.fetcher != nil, "Fetcher is nil")
+	testingresources.Assert(t, engine.pusher != nil, "Pusher is nil")
+	testingresources.Assert(t,
+		engine.manifest.Config.Digest == testingresources.DockerV2_Manifest_Simple_Config_Digest,
+		fmt.Sprintf("Config Digest is not equal to %s", testingresources.DockerV2_Manifest_Simple_Config_Digest))
+
+	content, err := testingresources.ConsistentManifestMarshal(&engine.manifest)
+	if err != nil {
+		t.Errorf("Could not parse obtained manifest, got: %v", err)
+	}
+
+	testingresources.Assert(t,
+		digest.FromBytes(content) == testingresources.DockerV2_Manifest_Simple_Digest,
+		fmt.Sprintf("Manifest Digest is not equal to %s", testingresources.DockerV2_Manifest_Simple_Digest))
+}
+
+func Test_uploadManifestAndConfig(t *testing.T) {
+}

cmd/convertor/builder/builder_test.go

@@ -0,0 +1,129 @@
+/*
+   Copyright The Accelerated Container Image Authors
+
+   Licensed under the Apache License, Version 2.0 (the "License");
+   you may not use this file except in compliance with the License.
+   You may obtain a copy of the License at
+
+       http://www.apache.org/licenses/LICENSE-2.0
+
+   Unless required by applicable law or agreed to in writing, software
+   distributed under the License is distributed on an "AS IS" BASIS,
+   WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+   See the License for the specific language governing permissions and
+   limitations under the License.
+*/
+
+package builder
+
+import (
+	"context"
+	"fmt"
+	"math/rand"
+	"testing"
+	"time"
+
+	_ "github.com/containerd/containerd/pkg/testutil" // Handle custom root flag
+	specs "github.com/opencontainers/image-spec/specs-go/v1"
+)
+
+// Test_builder_Err_Fuzz_Build This test is for the arguably complex error handling and potential go routine
+// locking that can happen for the builder component. It works by testing multiple potential error patterns
+// across all stages of the process (Through consistent pseudo random generation, for reproducibility and
+// ease of adjustment). The test is designed to run in parallel to maximize the chance of a contention.
+func Test_builder_Build_Contention(t *testing.T) {
+	// If timeout of 1 second is exceeded with the mock fuz builder engine
+	// then there is a high likelihood of a present contention error
+	contentionTimeout := time.Second * 1
+	patternCount := int64(500) // Try out 500 different seeds
+
+	var i int64
+	for i = 0; i < patternCount; i++ {
+		seed := i
+		t.Run(fmt.Sprintf("Test_builder_Err_Lock Contention Seed %d", i), func(t *testing.T) {
+			t.Parallel()
+			fixedRand := rand.New(rand.NewSource(seed))
+			builderEngine := newMockBuilderEngine(fixedRand)
+			b := &overlaybdBuilder{
+				engine: builderEngine,
+				layers: 25,
+			}
+			ctx, cancel := context.WithTimeout(context.Background(), contentionTimeout)
+			defer cancel()
+
+			b.Build(ctx)
+			// Build will typically return an error but completes successfully for some seeds as well
+			if ctx.Err() != nil {
+				if ctx.Err() == context.DeadlineExceeded {
+					t.Errorf("Context deadline was exceeded, likely contention error")
+				}
+			}
+		})
+	}
+}
+
+const (
+	failRate = 0.05 // 5% of the time, fail any given operation
+)
+
+type mockFuzzBuilderEngine struct {
+	fixedRand *rand.Rand
+}
+
+func newMockBuilderEngine(fixedRand *rand.Rand) builderEngine {
+	return &mockFuzzBuilderEngine{
+		fixedRand: fixedRand,
+	}
+}
+
+func (e *mockFuzzBuilderEngine) DownloadLayer(ctx context.Context, idx int) error {
+	if e.fixedRand.Float64() < failRate {
+		return fmt.Errorf("random error on download")
+	}
+	return nil
+}
+
+func (e *mockFuzzBuilderEngine) BuildLayer(ctx context.Context, idx int) error {
+	if e.fixedRand.Float64() < failRate {
+		return fmt.Errorf("random error on BuildLayer")
+	}
+	return nil
+}
+
+func (e *mockFuzzBuilderEngine) UploadLayer(ctx context.Context, idx int) error {
+	if e.fixedRand.Float64() < failRate {
+		return fmt.Errorf("random error on UploadLayer")
+	}
+	return nil
+}
+
+func (e *mockFuzzBuilderEngine) UploadImage(ctx context.Context) error {
+	if e.fixedRand.Float64() < failRate {
+		return fmt.Errorf("random error on UploadImage")
+	}
+	return nil
+}
+
+func (e *mockFuzzBuilderEngine) CheckForConvertedLayer(ctx context.Context, idx int) (specs.Descriptor, error) {
+	if e.fixedRand.Float64() < failRate {
+		return specs.Descriptor{}, fmt.Errorf("random error on CheckForConvertedLayer")
+	}
+	return specs.Descriptor{}, nil
+}
+
+func (e *mockFuzzBuilderEngine) StoreConvertedLayerDetails(ctx context.Context, idx int) error {
+	if e.fixedRand.Float64() < failRate {
+		return fmt.Errorf("random error on StoreConvertedLayerDetails")
+	}
+	return nil
+}
+
+func (e *mockFuzzBuilderEngine) DownloadConvertedLayer(ctx context.Context, idx int, desc specs.Descriptor) error {
+	if e.fixedRand.Float64() < failRate {
+		return fmt.Errorf("random error on DownloadConvertedLayer")
+	}
+	return nil
+}
+
+func (e *mockFuzzBuilderEngine) Cleanup() {
+}