From c10eff8252489061da9e9d52008a9a3a7671a7b5 Mon Sep 17 00:00:00 2001
From: Yan Song <imeoer@linux.alibaba.com>
Date: Thu, 21 Apr 2022 09:02:51 +0000
Subject: [PATCH] converter: fix incomplete unpack

The current `Convert` implementation has the following interface:

```
Convert(ctx context.Context, dest io.Writer, opt ConvertOption) (src io.WriteCloser, err error)
```

The caller writes the OCI tar stream to src and Convert converts
src to a nydus tar stream and writes it to dest.

The current implementation must wait for the OCI tar stream to be
unpacked into a directory before calling the builder (nydus-image)
to start the build, which takes advantage of `pr, pw := io.Pipe()`.

Before this commit, the `unpackOciTar` method did not fully read
the OCI tarball when `pw.Close()` was called, which means that the
build is started before the unpacking is complete, which may result
in some files being missing from the nydus image.

Fixup: use a channel to wait for the unpack to complete.

Signed-off-by: Yan Song <imeoer@linux.alibaba.com>
---
 pkg/converter/convert.go | 7 +++++++
 pkg/converter/utils.go   | 9 ++++++---
 2 files changed, 13 insertions(+), 3 deletions(-)

diff --git a/pkg/converter/convert.go b/pkg/converter/convert.go
index 571f5f3040..b03b721d2b 100644
--- a/pkg/converter/convert.go
+++ b/pkg/converter/convert.go
@@ -221,11 +221,13 @@ func Convert(ctx context.Context, dest io.Writer, opt ConvertOption) (io.WriteCl
 
 	pr, pw := io.Pipe()
 
+	unpackDone := make(chan bool, 1)
 	go func() {
 		if err := unpackOciTar(ctx, sourceDir, pr); err != nil {
 			pr.CloseWithError(errors.Wrapf(err, "unpack to %s", sourceDir))
 			return
 		}
+		unpackDone <- true
 	}()
 
 	wc := newWriteCloser(pw, func() error {
@@ -233,6 +235,11 @@ func Convert(ctx context.Context, dest io.Writer, opt ConvertOption) (io.WriteCl
 			os.RemoveAll(workDir)
 		}()
 
+		// Because PipeWriter#Close is called does not mean that the PipeReader
+		// has finished reading all the data, and unpack may not be complete yet,
+		// so we need to wait for that here.
+		<-unpackDone
+
 		bootstrapPath := filepath.Join(workDir, "bootstrap")
 		blobPath := filepath.Join(workDir, "blob")
 
diff --git a/pkg/converter/utils.go b/pkg/converter/utils.go
index a571b4a311..8fba62945f 100644
--- a/pkg/converter/utils.go
+++ b/pkg/converter/utils.go
@@ -26,13 +26,16 @@ func (c *writeCloser) Close() error {
 	if c.closed {
 		return nil
 	}
-	if err := c.action(); err != nil {
-		return err
-	}
+
 	if err := c.WriteCloser.Close(); err != nil {
 		return err
 	}
 	c.closed = true
+
+	if err := c.action(); err != nil {
+		return err
+	}
+
 	return nil
 }