Fix panic on rootless build without push

Some recent UX refinements broke the rootless creation flow.
Also a miswired error case resulted in a nil error
when there should have been an error which led to the panic.
Enabled and refined the integration tests for rootless coverage.

Makefile

@@ -3,6 +3,7 @@
 
 VERSION?=$(shell git describe --tags --first-parent --abbrev=7 --long --dirty --always)
 TEST_KUBECONFIG?=$(HOME)/.kube/config
+TEST_TIMEOUT=300s
 
 # Verify Go in PATH
 ifeq (, $(shell which go))
@@ -36,7 +37,7 @@ help:
 
 .PHONY: clean
 clean:
-	-rm -rf $(BIN_DIR) cover*.out cover.html
+	-rm -rf $(BIN_DIR) cover*.out cover*.html
 
 .PHONY: build
 build: $(BIN_DIR)/$(NATIVE_ARCH)/kubectl-buildkit $(BIN_DIR)/$(NATIVE_ARCH)/kubectl-build
@@ -70,7 +71,7 @@ test:
 integration:
 	@echo "Running integration tests with $(TEST_KUBECONFIG)"
 	@kubectl config get-contexts
-	TEST_KUBECONFIG=$(TEST_KUBECONFIG) go test $(GO_FLAGS) $(EXTRA_GO_TEST_FLAGS)  \
+	TEST_KUBECONFIG=$(TEST_KUBECONFIG) go test -timeout $(TEST_TIMEOUT) $(GO_FLAGS) $(EXTRA_GO_TEST_FLAGS)  \
 		$(GO_COVER_FLAGS) -coverprofile=./cover-int.out \
 		./integration/...
 

integration/common/basesuites.go

@@ -3,6 +3,10 @@
 package common
 
 import (
+	"fmt"
+	"path"
+	"strings"
+
 	"github.com/sirupsen/logrus"
 	"github.com/stretchr/testify/require"
 	"github.com/stretchr/testify/suite"
@@ -53,5 +57,43 @@ func (s *BaseSuite) TestSimpleBuild() {
 		dir,
 	)
 	err = RunBuild(args)
+	if isRootlessCreate(s.CreateFlags) {
+		require.Error(s.T(), err)
+		require.Contains(s.T(), err.Error(), "please specify")
+	} else {
+		require.NoError(s.T(), err, "build failed")
+	}
+}
+
+func isRootlessCreate(flags []string) bool {
+	for _, flag := range flags {
+		if strings.Contains(flag, "rootless") {
+			return true
+		}
+	}
+	return false
+}
+
+func (s *BaseSuite) TestLocalOutputTarBuild() {
+	logrus.Infof("%s: Local Output Tar Build", s.Name)
+
+	dir, cleanup, err := NewSimpleBuildContext()
+	defer cleanup()
+	require.NoError(s.T(), err, "Failed to set up temporary build context")
+	args := []string{}
+	if s.Name != "buildkit" { // TODO wire up the default name variable
+		args = append(
+			args,
+			"--builder", s.Name,
+		)
+	}
+	args = append(
+		args,
+		"--tag", s.Name+"replaceme:latest",
+		fmt.Sprintf("--output=type=tar,dest=%s", path.Join(dir, "out.tar")),
+		dir,
+	)
+	err = RunBuild(args)
 	require.NoError(s.T(), err, "build failed")
+	// TODO - consider inspecting the out.tar for validity...
 }

integration/suites/rootless_test.go

@@ -4,22 +4,24 @@ package suites
 
 import (
 	"testing"
-	//"github.com/stretchr/testify/suite"
-	//"github.com/vmware-tanzu/buildkit-cli-for-kubectl/integration/common"
+
+	"github.com/stretchr/testify/suite"
+	"github.com/vmware-tanzu/buildkit-cli-for-kubectl/integration/common"
 )
 
-//type rootlessSuite struct{ common.BaseSuite }
+type rootlessSuite struct{ common.BaseSuite }
 
 func TestRootlessSuite(t *testing.T) {
-	t.Skip("Skipping rootless due to bug! - should disable local storage mode automatically")
-	//common.Skipper(t)
+	common.Skipper(t)
 	//t.Parallel() // TODO - tests fail if run in parallel, may be actual race bug
-	/*
-		suite.Run(t, &rootlessSuite{
-			BaseSuite: common.BaseSuite{
-				Name:        "rootless",
-				CreateFlags: []string{"--rootless", "true"},
-			},
-		})
-	*/
+	suite.Run(t, &rootlessSuite{
+		BaseSuite: common.BaseSuite{
+			Name:        "rootless",
+			CreateFlags: []string{"--rootless", "true"},
+		},
+	})
 }
+
+// func (s *rootlessSuite) TestSimpleBuild() {
+// 	s.T().Skip("Rootless doesn't support loading to the runtime")
+// }

pkg/build/build.go

@@ -364,8 +364,8 @@ func toSolveOpt(ctx context.Context, d driver.Driver, multiDriver bool, opt Opti
 			} else if driverFeatures[driver.DockerExporter] {
 				opt.Exports[i].Type = "docker"
 			} else {
-				// TODO should we allow building without load or push?
-				return nil, nil, errors.Wrap(err, "unable to determine runtime, please specify --output=type=[docker || containerd] or --push")
+				// TODO should we allow building without load or push, perhaps a new "nil" or equivalent output type?
+				return nil, nil, errors.Errorf("loading image into cluster runtime not supported by this builder, please specify --push or a client local output: --output=type=local,dest=. --output=type=tar,dest=out.tar ")
 			}
 		}
 	}

pkg/cmd/create.go

@@ -128,7 +128,7 @@ Driver Specific Usage:
 	flags.StringVar(&options.configFile, "config", "", "BuildKit config file")
 	flags.StringArrayVar(&options.platform, "platform", []string{}, "Fixed platforms for current node")
 	flags.StringVar(&options.progress, "progress", "auto", "Set type of progress output [auto, plain, tty]. Use plain to show container output")
-	flags.StringVar(&options.image, "image", bkimage.DefaultImage, "Specify an alternate buildkit image")
+	flags.StringVar(&options.image, "image", "", fmt.Sprintf("Specify an alternate buildkit image (default: %s)", bkimage.DefaultImage))
 	flags.StringVar(&options.runtime, "runtime", "auto", "Container runtime used by cluster [auto, docker, containerd]")
 	flags.StringVar(&options.containerdSock, "containerd-sock", kubernetes.DefaultContainerdSockPath, "Path to the containerd.sock on the host")
 	flags.StringVar(&options.containerdNamespace, "containerd-namespace", kubernetes.DefaultContainerdNamespace, "Containerd namespace to build images in")

pkg/driver/kubernetes/factory.go

@@ -139,7 +139,9 @@ func (d *Driver) initDriverFromConfig() error {
 			if err != nil {
 				return err
 			}
-			deploymentOpt.Image = bkimage.DefaultRootlessImage
+			if deploymentOpt.Rootless {
+				deploymentOpt.Image = bkimage.DefaultRootlessImage
+			}
 		case "loadbalance":
 			switch v {
 			case LoadbalanceSticky:
@@ -232,7 +234,6 @@ func (d *Driver) initDriverFromConfig() error {
 		//        user tries to set properties that should be in the config file
 		d.configMap = manifest.NewConfigMap(deploymentOpt, data)
 	}
-
 	return nil
 }