From 4242e27924e7fb3cfbebcfebd77fd832afc2618c Mon Sep 17 00:00:00 2001 From: Roman Sysoev Date: Fri, 20 Feb 2026 19:34:44 +0300 Subject: [PATCH 1/4] fix(dvcr): improve importer logging - Ensure upload layer errors are not missed. - Wrap "DVCR is out of space" error. Signed-off-by: Roman Sysoev --- images/dvcr-artifact/pkg/registry/registry.go | 37 +++++++++++++------ .../pkg/controller/service/stat_service.go | 35 ++++++++++++++++-- 2 files changed, 58 insertions(+), 14 deletions(-) diff --git a/images/dvcr-artifact/pkg/registry/registry.go b/images/dvcr-artifact/pkg/registry/registry.go index 20e613e3ce..529ae9ee69 100644 --- a/images/dvcr-artifact/pkg/registry/registry.go +++ b/images/dvcr-artifact/pkg/registry/registry.go @@ -24,6 +24,7 @@ import ( "crypto/tls" "encoding/hex" "encoding/json" + "errors" "fmt" "io" "net/http" @@ -35,12 +36,11 @@ import ( "github.com/google/go-containerregistry/pkg/authn" "github.com/google/go-containerregistry/pkg/name" - "github.com/google/go-containerregistry/pkg/v1" + v1 "github.com/google/go-containerregistry/pkg/v1" "github.com/google/go-containerregistry/pkg/v1/empty" "github.com/google/go-containerregistry/pkg/v1/mutate" "github.com/google/go-containerregistry/pkg/v1/remote" "github.com/google/go-containerregistry/pkg/v1/stream" - "github.com/pkg/errors" "golang.org/x/sync/errgroup" "k8s.io/klog/v2" "kubevirt.io/containerized-data-importer/pkg/importer" @@ -140,17 +140,32 @@ func (p DataProcessor) Process(ctx context.Context) (ImportRes, error) { informer := NewImageInformer() - errsGroup, ctx := errgroup.WithContext(ctx) - errsGroup.Go(func() error { - return p.inspectAndStreamSourceImage(ctx, sourceImageFilename, sourceImageSize, progressMeter, pipeWriter, informer) - }) - errsGroup.Go(func() error { + resultCh := make(chan error, 2) + + go func() { + resultCh <- p.inspectAndStreamSourceImage( + ctx, sourceImageFilename, sourceImageSize, + progressMeter, pipeWriter, informer, + ) + }() + + go func() { defer pipeReader.Close() - return p.uploadLayersAndImage(ctx, pipeReader, sourceImageSize, informer) - }) + resultCh <- p.uploadLayersAndImage( + ctx, pipeReader, sourceImageSize, informer, + ) + }() + + err1 := <-resultCh + if err1 != nil { + klog.Errorln(err1) + } + err2 := <-resultCh + if err2 != nil { + klog.Errorln(err2) + } - err = errsGroup.Wait() - if err != nil { + if err := errors.Join(err1, err2); err != nil { return ImportRes{}, err } diff --git a/images/virtualization-artifact/pkg/controller/service/stat_service.go b/images/virtualization-artifact/pkg/controller/service/stat_service.go index efa67b5dcf..7974be0ab2 100644 --- a/images/virtualization-artifact/pkg/controller/service/stat_service.go +++ b/images/virtualization-artifact/pkg/controller/service/stat_service.go @@ -19,7 +19,11 @@ package service import ( "errors" "fmt" + "net/http" + "regexp" "strconv" + "strings" + "syscall" "time" corev1 "k8s.io/api/core/v1" @@ -99,9 +103,12 @@ func (s StatService) GetSize(pod *corev1.Pod) v1alpha2.ImageStatusSize { } var ( - ErrNotInitialized = errors.New("not initialized") - ErrNotScheduled = errors.New("not scheduled") - ErrProvisioningFailed = errors.New("provisioning failed") + ErrNotInitialized = errors.New("not initialized") + ErrNotScheduled = errors.New("not scheduled") + ErrProvisioningFailed = errors.New("provisioning failed") + ErrDVCRNoSpaceImageError = errors.New("DVCR is out of space; please contact the cluster administrator") + // ErrDVCRNoSpaceDiskError is intended to avoid confusion by clarifying that DVCR is needed as an intermediary when creating a virtual disk. + ErrDVCRNoSpaceDiskError = errors.New("DVCR is out of space to create the virtual disk; please contact the cluster administrator") ) func (s StatService) CheckPod(pod *corev1.Pod) error { @@ -125,6 +132,12 @@ func (s StatService) CheckPod(pod *corev1.Pod) error { } if report != nil && report.ErrMessage != "" { + if s.isDVCRNoSpaceError(report.ErrMessage) { + if strings.HasPrefix(pod.Name, "d8v-vd-") { + return fmt.Errorf("%w: %w", ErrProvisioningFailed, ErrDVCRNoSpaceDiskError) + } + return fmt.Errorf("%w: %w", ErrProvisioningFailed, ErrDVCRNoSpaceImageError) + } return fmt.Errorf("%w: Pod %s/%s termination message: %s", ErrProvisioningFailed, pod.Namespace, pod.Name, report.ErrMessage) } @@ -135,6 +148,22 @@ func (s StatService) CheckPod(pod *corev1.Pod) error { return nil } +func (s StatService) isDVCRNoSpaceError(terminationMessage string) bool { + dvcrSvc := "dvcr.d8-virtualization.svc" + + noSpaceErrorPattern := fmt.Sprintf("Err:%d", syscall.ENOSPC) + noDigitPattern := `\D` + re := regexp.MustCompile(noSpaceErrorPattern + noDigitPattern) + + if strings.Contains(terminationMessage, dvcrSvc) && + strings.Contains(terminationMessage, http.MethodPost) && + re.MatchString(terminationMessage) { + return true + } + + return false +} + func (s StatService) GetDownloadSpeed(ownerUID types.UID, pod *corev1.Pod) *v1alpha2.StatusSpeed { report, err := monitoring.GetFinalReportFromPod(pod) if err != nil && !errors.Is(err, monitoring.ErrTerminationMessageNotFound) { From 83411fd1b6c7206a76c4346c438a7fb25c920edd Mon Sep 17 00:00:00 2001 From: Roman Sysoev Date: Wed, 25 Feb 2026 17:05:29 +0300 Subject: [PATCH 2/4] fix(dvcr): resolve review comments - fix ide import auto formatting - wrap chan errors - improve naming - use context cancel func Signed-off-by: Roman Sysoev --- images/dvcr-artifact/pkg/registry/registry.go | 31 ++++++++++++++----- 1 file changed, 24 insertions(+), 7 deletions(-) diff --git a/images/dvcr-artifact/pkg/registry/registry.go b/images/dvcr-artifact/pkg/registry/registry.go index 529ae9ee69..459429161d 100644 --- a/images/dvcr-artifact/pkg/registry/registry.go +++ b/images/dvcr-artifact/pkg/registry/registry.go @@ -36,7 +36,7 @@ import ( "github.com/google/go-containerregistry/pkg/authn" "github.com/google/go-containerregistry/pkg/name" - v1 "github.com/google/go-containerregistry/pkg/v1" + "github.com/google/go-containerregistry/pkg/v1" "github.com/google/go-containerregistry/pkg/v1/empty" "github.com/google/go-containerregistry/pkg/v1/mutate" "github.com/google/go-containerregistry/pkg/v1/remote" @@ -137,30 +137,47 @@ func (p DataProcessor) Process(ctx context.Context) (ImportRes, error) { defer progressMeter.Stop() pipeReader, pipeWriter := io.Pipe() + defer pipeReader.Close() informer := NewImageInformer() - resultCh := make(chan error, 2) + ctx, cancel := context.WithCancel(ctx) + defer cancel() + + errsCh := make(chan error, 2) go func() { - resultCh <- p.inspectAndStreamSourceImage( + err := p.inspectAndStreamSourceImage( ctx, sourceImageFilename, sourceImageSize, progressMeter, pipeWriter, informer, ) + if err != nil { + err = fmt.Errorf("stream source error: %w", err) + if !errors.Is(err, context.Canceled) { + cancel() + } + } + errsCh <- err }() go func() { - defer pipeReader.Close() - resultCh <- p.uploadLayersAndImage( + err := p.uploadLayersAndImage( ctx, pipeReader, sourceImageSize, informer, ) + if err != nil { + err = fmt.Errorf("upload layers error: %w", err) + if !errors.Is(err, context.Canceled) { + cancel() + } + } + errsCh <- err }() - err1 := <-resultCh + err1 := <-errsCh if err1 != nil { klog.Errorln(err1) } - err2 := <-resultCh + err2 := <-errsCh if err2 != nil { klog.Errorln(err2) } From 591d1596e02604d4b2e7f396b0deb97881265ca8 Mon Sep 17 00:00:00 2001 From: Roman Sysoev Date: Fri, 27 Feb 2026 12:49:31 +0300 Subject: [PATCH 3/4] fix(dvcr): ErrClosedPipe is just a signal Signed-off-by: Roman Sysoev --- images/dvcr-artifact/pkg/registry/registry.go | 26 ++++++++++--------- 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/images/dvcr-artifact/pkg/registry/registry.go b/images/dvcr-artifact/pkg/registry/registry.go index 459429161d..03fb693d9f 100644 --- a/images/dvcr-artifact/pkg/registry/registry.go +++ b/images/dvcr-artifact/pkg/registry/registry.go @@ -137,7 +137,6 @@ func (p DataProcessor) Process(ctx context.Context) (ImportRes, error) { defer progressMeter.Stop() pipeReader, pipeWriter := io.Pipe() - defer pipeReader.Close() informer := NewImageInformer() @@ -152,35 +151,38 @@ func (p DataProcessor) Process(ctx context.Context) (ImportRes, error) { progressMeter, pipeWriter, informer, ) if err != nil { - err = fmt.Errorf("stream source error: %w", err) - if !errors.Is(err, context.Canceled) { - cancel() + if errors.Is(err, io.ErrClosedPipe) { + klog.Infof("source streaming: pipe is closed by upload: %s", err.Error()) + err = nil + } else { + if ctx.Err() != context.Canceled { + cancel() + } + err = fmt.Errorf("source streaming error: %w", err) + klog.Errorln(err) } } errsCh <- err }() go func() { + defer pipeReader.Close() + err := p.uploadLayersAndImage( ctx, pipeReader, sourceImageSize, informer, ) if err != nil { - err = fmt.Errorf("upload layers error: %w", err) - if !errors.Is(err, context.Canceled) { + if ctx.Err() != context.Canceled { cancel() } + err = fmt.Errorf("layers uploading error: %w", err) + klog.Errorln(err) } errsCh <- err }() err1 := <-errsCh - if err1 != nil { - klog.Errorln(err1) - } err2 := <-errsCh - if err2 != nil { - klog.Errorln(err2) - } if err := errors.Join(err1, err2); err != nil { return ImportRes{}, err From ee7d2d8d4db2b0c4c0fdf4bd67c271305a54fde2 Mon Sep 17 00:00:00 2001 From: Roman Sysoev Date: Wed, 4 Mar 2026 11:27:57 +0300 Subject: [PATCH 4/4] fix(dvcr): use ctx cancel without checking Signed-off-by: Roman Sysoev --- images/dvcr-artifact/pkg/registry/registry.go | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/images/dvcr-artifact/pkg/registry/registry.go b/images/dvcr-artifact/pkg/registry/registry.go index 03fb693d9f..f037780db0 100644 --- a/images/dvcr-artifact/pkg/registry/registry.go +++ b/images/dvcr-artifact/pkg/registry/registry.go @@ -155,9 +155,7 @@ func (p DataProcessor) Process(ctx context.Context) (ImportRes, error) { klog.Infof("source streaming: pipe is closed by upload: %s", err.Error()) err = nil } else { - if ctx.Err() != context.Canceled { - cancel() - } + cancel() err = fmt.Errorf("source streaming error: %w", err) klog.Errorln(err) } @@ -172,9 +170,7 @@ func (p DataProcessor) Process(ctx context.Context) (ImportRes, error) { ctx, pipeReader, sourceImageSize, informer, ) if err != nil { - if ctx.Err() != context.Canceled { - cancel() - } + cancel() err = fmt.Errorf("layers uploading error: %w", err) klog.Errorln(err) }