Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Building in Kaniko is broken from Windows in devspace 6+ #2423

Merged
merged 3 commits into from
Nov 15, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion e2e/tests/localregistry/localregistry.go
Original file line number Diff line number Diff line change
Expand Up @@ -419,7 +419,7 @@ var _ = DevSpaceDescribe("localregistry", func() {
framework.ExpectNoError(err)
})

ginkgo.It("should error when local registry is required and not supported by build type", func() {
ginkgo.It("should not use local registry when not supported by build type", func() {
tempDir, err := framework.CopyToTempDir("tests/localregistry/testdata/local-registry-kaniko")
framework.ExpectNoError(err)
defer framework.CleanupTempDir(initialDir, tempDir)
Expand All @@ -438,6 +438,9 @@ var _ = DevSpaceDescribe("localregistry", func() {
gomega.Expect(output.String()).To(
gomega.ContainSubstring("unable to push image my-docker-username/helloworld-kaniko and only docker and buildkit builds support using a local registry"),
)
gomega.Expect(output.String()).To(
gomega.ContainSubstring("UNAUTHORIZED: authentication required"),
)
})

ginkgo.It("should error when local registry is required and disabled by configuration", func() {
Expand Down
108 changes: 61 additions & 47 deletions pkg/devspace/build/build.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,11 @@ import (
"github.com/loft-sh/devspace/pkg/devspace/build/types"
"github.com/loft-sh/devspace/pkg/devspace/config/constants"
devspacecontext "github.com/loft-sh/devspace/pkg/devspace/context"
"github.com/loft-sh/devspace/pkg/util/randutil"
"github.com/loft-sh/devspace/pkg/util/stringutil"

"github.com/loft-sh/devspace/pkg/devspace/config/versions/latest"
"github.com/loft-sh/devspace/pkg/devspace/hook"
"github.com/loft-sh/devspace/pkg/util/randutil"
"github.com/pkg/errors"
)

Expand Down Expand Up @@ -87,50 +87,90 @@ func (c *controller) Build(ctx devspacecontext.Context, images []string, options
}

// Determine if we need to use the local registry to build any images.
kubeClient := ctx.KubeClient()
var localRegistry *registry.LocalRegistry
if registry.UseLocalRegistry(kubeClient, conf, options.SkipPush) {
ctx := ctx.WithLogger(ctx.Log().WithPrefix("local-registry: "))
for key, imageConf := range conf.Images {
imageName := imageConf.Image
imageConfigName := key
kubeClient := ctx.KubeClient()
builders := map[string]builder.Interface{}
tags := map[string][]string{}

// Update cache for non-local registry use by default
imageCache, _ := ctx.Config().LocalCache().GetImageCache(imageConfigName)
imageCache.LocalRegistryImageName = ""
for imageConfigName, imageConf := range conf.Images {
imageName := imageConf.Image

// Get image tags
imageTags := []string{}
if len(options.Tags) > 0 {
imageTags = append(imageTags, options.Tags...)
} else if len(imageConf.Tags) > 0 {
imageTags = append(imageTags, imageConf.Tags...)
} else {
imageTags = append(imageTags, randutil.GenerateRandomString(7))
}

// Determine whether the local registry is required / enabled
isLocalReqistryRequired := !registry.HasPushPermission(imageConf)
if isLocalReqistryRequired {
// Not able to deploy a local registry
// replace the # in the tags
for i := range imageTags {
for strings.Contains(imageTags[i], "#") {
imageTags[i] = strings.Replace(imageTags[i], "#", randutil.GenerateRandomString(1), 1)
}
}

// Create new builder
builder, err := c.createBuilder(ctx, imageConfigName, imageConf, imageTags, options)
if err != nil {
return errors.Wrap(err, "create builder")
}

// Update cache for non local registry use by default
imageCache, _ := ctx.Config().LocalCache().GetImageCache(imageConfigName)
imageCache.ImageName = imageName
imageCache.LocalRegistryImageName = ""

if registry.UseLocalRegistry(kubeClient, conf, options.SkipPush) && !registry.HasPushPermission(imageConf) {
if SupportsLocalRegistry(builder) {
// Not able to deploy a local registry without a valid kube context
if kubeClient == nil {
return fmt.Errorf("unable to push image %s and a valid kube context is not available", imageConf.Image)
}

// Create and start a local registry if one isn't already running
if localRegistry == nil {
localRegistry = registry.NewLocalRegistry(
registry.NewDefaultOptions().
WithNamespace(kubeClient.Namespace()).
WithLocalRegistryConfig(conf.LocalRegistry),
)

ctx := ctx.WithLogger(ctx.Log().WithPrefix("local-registry: "))
err := localRegistry.Start(ctx)
if err != nil {
return errors.Wrap(err, "start registry")
}
}

var err error
builtImageName, err := localRegistry.RewriteImage(imageName)
// Update cache for local registry use
imageCache.LocalRegistryImageName, err = localRegistry.RewriteImage(imageName)
if err != nil {
return errors.Wrap(err, "rewrite image")
}
ctx.Config().LocalCache().SetImageCache(imageConfigName, imageCache)

// Update cache for local registry use
imageCache.LocalRegistryImageName = builtImageName
// Reset the builder for local registry usage
// TODO: refactor so this isn't necessary!
builder, err = c.createBuilder(ctx, imageConfigName, imageConf, imageTags, options)
if err != nil {
return errors.Wrap(err, "create builder")
}
} else {
ctx.Log().Warnf("unable to push image %s and only docker and buildkit builds support using a local registry", imageConf.Image)
}
ctx.Config().LocalCache().SetImageCache(imageConfigName, imageCache)
}

// Save image cache
ctx.Config().LocalCache().SetImageCache(imageConfigName, imageCache)

// Save builder for later use
builders[imageConfigName] = builder

// Save image tags
tags[imageConfigName] = imageTags
}

// Execute before images build hook
Expand All @@ -152,34 +192,8 @@ func (c *controller) Build(ctx devspacecontext.Context, images []string, options
imageConfigName := key
imageCache, _ := ctx.Config().LocalCache().GetImageCache(imageConfigName)
resolvedImage := imageCache.ResolveImage()

// Get image tags
imageTags := []string{}
if len(options.Tags) > 0 {
imageTags = append(imageTags, options.Tags...)
} else if len(imageConf.Tags) > 0 {
imageTags = append(imageTags, imageConf.Tags...)
} else {
imageTags = append(imageTags, randutil.GenerateRandomString(7))
}

// replace the # in the tags
for i := range imageTags {
for strings.Contains(imageTags[i], "#") {
imageTags[i] = strings.Replace(imageTags[i], "#", randutil.GenerateRandomString(1), 1)
}
}

// Create new builder
builder, err := c.createBuilder(ctx, imageConfigName, &cImageConf, imageTags, options)
if err != nil {
return errors.Wrap(err, "create builder")
}

// Check compatibility with local registry
if imageCache.IsLocalRegistryImage() && !SupportsLocalRegistry(builder) {
return fmt.Errorf("unable to push image %s and only docker and buildkit builds support using a local registry", imageConf.Image)
}
imageTags := tags[imageConfigName]
builder := builders[imageConfigName]

// Execute before images build hook
pluginErr := hook.ExecuteHooks(ctx, map[string]interface{}{
Expand Down
11 changes: 8 additions & 3 deletions pkg/devspace/build/builder/kaniko/kaniko.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,20 +2,20 @@ package kaniko

import (
"fmt"
"io"
"strings"

devspacecontext "github.com/loft-sh/devspace/pkg/devspace/context"
"github.com/loft-sh/devspace/pkg/devspace/kubectl/selector"
"github.com/loft-sh/devspace/pkg/devspace/services/logs"
"github.com/sirupsen/logrus"
"io"
"strings"

"github.com/loft-sh/devspace/pkg/util/interrupt"

"github.com/docker/docker/pkg/archive"
"github.com/docker/docker/pkg/idtools"
kerrors "k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apimachinery/pkg/util/wait"

"k8s.io/client-go/util/exec"

"github.com/loft-sh/devspace/pkg/devspace/build/builder"
Expand Down Expand Up @@ -109,6 +109,11 @@ func (b *Builder) ShouldRebuild(ctx devspacecontext.Context, forceRebuild bool)
func (b *Builder) BuildImage(ctx devspacecontext.Context, contextPath, dockerfilePath string, entrypoint []string, cmd []string) error {
var err error

contextPath, err = build.ResolveAndValidateContextPath(contextPath)
if err != nil {
return errors.Wrap(err, "resolve context path")
}

// build options
options := &types.ImageBuildOptions{}
if b.helper.ImageConf.BuildArgs != nil {
Expand Down
2 changes: 1 addition & 1 deletion pkg/devspace/build/registry/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ func UseLocalRegistry(client kubectl.Client, config *latest.Config, skipPush boo
context := client.CurrentContext()

// Determine if this is a vcluster
isVClusterContext := strings.HasPrefix(context, "vcluster_")
isVClusterContext := strings.Contains(context, "vcluster_")

// Determine if this is a local kubernetes cluster
isLocalKubernetes := kubectl.IsLocalKubernetes(context)
Expand Down
51 changes: 51 additions & 0 deletions pkg/devspace/build/registry/util_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,16 @@ func TestUseLocalRegistry(t *testing.T) {
},
expected: false,
},
{
name: "Loft VCluster with KinD Cluster",
client: &kubectltesting.Client{
Context: "loft-vcluster_devspace-kind_vcluster-devspace-kind_kind-kind",
},
config: &latest.Config{
LocalRegistry: nil,
},
expected: false,
},
{
name: "Docker Desktop Cluster",
client: &kubectltesting.Client{
Expand Down Expand Up @@ -134,6 +144,16 @@ func TestUseLocalRegistry(t *testing.T) {
},
expected: false,
},
{
name: "Loft VCluster with Docker Desktop Cluster",
client: &kubectltesting.Client{
Context: "loft-vcluster_devspacehelper_deploy-example_docker-desktop",
},
config: &latest.Config{
LocalRegistry: nil,
},
expected: false,
},
{
name: "Minikube Cluster",
client: &kubectltesting.Client{
Expand Down Expand Up @@ -191,6 +211,16 @@ func TestUseLocalRegistry(t *testing.T) {
},
expected: false,
},
{
name: "Loft VCluster with Minikube Cluster",
client: &kubectltesting.Client{
Context: "loft-vcluster_devspace-minikube_vcluster-devspace-minikube_minikube",
},
config: &latest.Config{
LocalRegistry: nil,
},
expected: false,
},
{
name: "Remote Cluster",
client: &kubectltesting.Client{
Expand Down Expand Up @@ -234,6 +264,16 @@ func TestUseLocalRegistry(t *testing.T) {
},
expected: true,
},
{
name: "Loft VCluster with Remote Cluster",
client: &kubectltesting.Client{
Context: "loft-vcluster_vcluster-eks_vcluster-vcluster-eks_arn:aws:eks:us-west-2:1234567890:cluster/remote-eks",
},
config: &latest.Config{
LocalRegistry: nil,
},
expected: true,
},
{
name: "VCluster with Remote Cluster skip push",
client: &kubectltesting.Client{
Expand All @@ -245,6 +285,17 @@ func TestUseLocalRegistry(t *testing.T) {
skipPush: true,
expected: false,
},
{
name: "Loft VCluster with Remote Cluster skip push",
client: &kubectltesting.Client{
Context: "loft-vcluster_vcluster-eks_vcluster-vcluster-eks_arn:aws:eks:us-west-2:1234567890:cluster/remote-eks",
},
config: &latest.Config{
LocalRegistry: nil,
},
skipPush: true,
expected: false,
},
{
name: "Nil KubeClient",
client: nil,
Expand Down
2 changes: 1 addition & 1 deletion pkg/devspace/kubectl/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -209,7 +209,7 @@ func IsLocalKubernetes(context string) bool {
context == dockerDesktopContext ||
context == dockerForDesktopContext {
return true
} else if strings.HasPrefix(context, "vcluster_") &&
} else if strings.Contains(context, "vcluster_") &&
(strings.HasSuffix(context, minikubeContext) ||
strings.HasSuffix(context, dockerDesktopContext) ||
strings.HasSuffix(context, dockerForDesktopContext) ||
Expand Down