From b0eaa5ed697ac191f15af6208174c49a375f09ce Mon Sep 17 00:00:00 2001 From: Russell Centanni Date: Mon, 14 Nov 2022 23:39:01 -0500 Subject: [PATCH 1/3] fix: resolve context path for windows --- pkg/devspace/build/builder/kaniko/kaniko.go | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/pkg/devspace/build/builder/kaniko/kaniko.go b/pkg/devspace/build/builder/kaniko/kaniko.go index 7abbdf6721..0259dd5f00 100644 --- a/pkg/devspace/build/builder/kaniko/kaniko.go +++ b/pkg/devspace/build/builder/kaniko/kaniko.go @@ -2,12 +2,13 @@ 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" @@ -15,7 +16,6 @@ import ( "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" @@ -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 { From 2e49bcc9d257529bdafab0cb2e74ac314e65b84e Mon Sep 17 00:00:00 2001 From: Russell Centanni Date: Tue, 15 Nov 2022 01:03:56 -0500 Subject: [PATCH 2/3] fix: skip local registry for loft vclusters contexts on local kubernetes clusters --- pkg/devspace/build/registry/util.go | 2 +- pkg/devspace/build/registry/util_test.go | 51 ++++++++++++++++++++++++ pkg/devspace/kubectl/util.go | 2 +- 3 files changed, 53 insertions(+), 2 deletions(-) diff --git a/pkg/devspace/build/registry/util.go b/pkg/devspace/build/registry/util.go index 1d5d52dc95..c548272924 100644 --- a/pkg/devspace/build/registry/util.go +++ b/pkg/devspace/build/registry/util.go @@ -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) diff --git a/pkg/devspace/build/registry/util_test.go b/pkg/devspace/build/registry/util_test.go index 88c8b237f8..83d4e80905 100644 --- a/pkg/devspace/build/registry/util_test.go +++ b/pkg/devspace/build/registry/util_test.go @@ -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{ @@ -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{ @@ -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{ @@ -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{ @@ -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, diff --git a/pkg/devspace/kubectl/util.go b/pkg/devspace/kubectl/util.go index f55e4e9dfc..b307158573 100644 --- a/pkg/devspace/kubectl/util.go +++ b/pkg/devspace/kubectl/util.go @@ -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) || From b42f6efc806670c284942e21f7c94c6bb0f26f33 Mon Sep 17 00:00:00 2001 From: Russell Centanni Date: Tue, 15 Nov 2022 02:54:15 -0500 Subject: [PATCH 3/3] fix: show warning instead of error when local registry is required but configured builder does not support it --- e2e/tests/localregistry/localregistry.go | 5 +- pkg/devspace/build/build.go | 108 +++++++++++++---------- 2 files changed, 65 insertions(+), 48 deletions(-) diff --git a/e2e/tests/localregistry/localregistry.go b/e2e/tests/localregistry/localregistry.go index 2082c0a8db..24ed002686 100644 --- a/e2e/tests/localregistry/localregistry.go +++ b/e2e/tests/localregistry/localregistry.go @@ -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) @@ -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() { diff --git a/pkg/devspace/build/build.go b/pkg/devspace/build/build.go index 2a6121690e..73693dbf71 100644 --- a/pkg/devspace/build/build.go +++ b/pkg/devspace/build/build.go @@ -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" ) @@ -87,26 +87,50 @@ 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(). @@ -114,23 +138,39 @@ func (c *controller) Build(ctx devspacecontext.Context, images []string, options 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 @@ -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{}{