Repository: code-ready/crc
Pull request #2300 (2021-05-03 07:43:25)

Cherry pick from 2296 PR

openshift-ci-robot: [APPROVALNOTIFIER] This PR is **APPROVED**

This pull-request has been approved by: *<a href="https://github.com/code-ready/crc/pull/2300#" title="Author self-approved">praveenkumar</a>*

The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=code-ready%2Fcrc).

The pull request process is described [here](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process)

<details >
Needs approval from an approver in each of these files:

- ~~[OWNERS](https://github.com/code-ready/crc/blob/master/OWNERS)~~ [praveenkumar]

Approvers can indicate their approval by writing `/approve` in a comment
Approvers can cancel approval by writing `/approve cancel` in a comment
</details>
<!-- META={"approvers":[]} -->

Patch:
diff --git a/pkg/crc/machine/bundle/metadata.go b/pkg/crc/machine/bundle/metadata.go
index f10c638fa..1567fecb0 100644
--- a/pkg/crc/machine/bundle/metadata.go
+++ b/pkg/crc/machine/bundle/metadata.go
@@ -5,6 +5,7 @@ import (
 	"io/ioutil"
 	"os"
 	"path/filepath"
+	"regexp"
 	"strconv"
 	"strings"
 	"time"
@@ -219,10 +220,8 @@ func GetBundleNameWithoutExtension(bundleName string) string {
 	return strings.TrimSuffix(bundleName, bundleExtension)
 }
 
-func GetCustomBundleNameWithoutExtension(bundleName string) string {
-	return fmt.Sprintf("%s_%d", GetBundleNameWithoutExtension(bundleName), time.Now().Unix())
-}
-
-func GetCustomBundle(bundleNameWithoutExtension string) string {
-	return fmt.Sprintf("%s%s", bundleNameWithoutExtension, bundleExtension)
+func GetCustomBundleName(bundleFilename string) string {
+	re := regexp.MustCompile(`(?:_[0-9]+)*.crcbundle$`)
+	baseName := re.ReplaceAllLiteralString(bundleFilename, "")
+	return fmt.Sprintf("%s_%d%s", baseName, time.Now().Unix(), bundleExtension)
 }
diff --git a/pkg/crc/machine/generate_bundle.go b/pkg/crc/machine/generate_bundle.go
index 78e97414a..5942a25f9 100644
--- a/pkg/crc/machine/generate_bundle.go
+++ b/pkg/crc/machine/generate_bundle.go
@@ -37,14 +37,15 @@ func (client *client) GenerateBundle() error {
 		return fmt.Errorf("VM is not stopped, current state is %s", currentState.String())
 	}
 
-	tmpBaseDir, err := ioutil.TempDir("", "crc_custom_bundle")
+	tmpBaseDir, err := ioutil.TempDir(constants.MachineCacheDir, "crc_custom_bundle")
 	if err != nil {
 		return err
 	}
 	defer os.RemoveAll(tmpBaseDir)
 
 	// Create the custom bundle directory which is used as top level directory for tarball during compression
-	customBundleNameWithoutExtension := bundle.GetCustomBundleNameWithoutExtension(bundleMetadata.GetBundleName())
+	customBundleName := bundle.GetCustomBundleName(bundleMetadata.GetBundleName())
+	customBundleNameWithoutExtension := bundle.GetBundleNameWithoutExtension(customBundleName)
 
 	copier, err := bundle.NewCopier(bundleMetadata, tmpBaseDir, customBundleNameWithoutExtension)
 	if err != nil {
@@ -76,7 +77,7 @@ func (client *client) GenerateBundle() error {
 	// Copy disk image
 	logging.Infof("Copying the disk image to %s", customBundleNameWithoutExtension)
 	logging.Debugf("Absolute path of custom bundle directory: %s", customBundleDir)
-	diskPath, diskFormat, err := copyDiskImage(customBundleDir, bundleMetadata.GetDiskImagePath())
+	diskPath, diskFormat, err := copyDiskImage(customBundleDir)
 	if err != nil {
 		return err
 	}
@@ -92,8 +93,8 @@ func (client *client) GenerateBundle() error {
 	if err != nil {
 		return err
 	}
-	logging.Infof("Bundle is generated in %s", filepath.Join(cwd, bundle.GetCustomBundle(customBundleNameWithoutExtension)))
-	logging.Infof("You need to perform 'crc delete' and 'crc start -b %s' to use this bundle", filepath.Join(cwd, bundle.GetCustomBundle(customBundleNameWithoutExtension)))
+	logging.Infof("Bundle is generated in %s", filepath.Join(cwd, customBundleName))
+	logging.Infof("You need to perform 'crc delete' and 'crc start -b %s' to use this bundle", filepath.Join(cwd, customBundleName))
 	return nil
 }
 
diff --git a/pkg/crc/machine/generate_bundle_linux.go b/pkg/crc/machine/generate_bundle_linux.go
index 1a35bed19..811800d32 100644
--- a/pkg/crc/machine/generate_bundle_linux.go
+++ b/pkg/crc/machine/generate_bundle_linux.go
@@ -1,50 +1,25 @@
 package machine
 
 import (
-	"os"
+	"fmt"
 	"path/filepath"
 
 	"github.com/code-ready/crc/pkg/crc/constants"
 	crcos "github.com/code-ready/crc/pkg/os"
 )
 
-func copyDiskImage(destDir string, srcDir string) (string, string, error) {
-	tmpDir := filepath.Join(constants.MachineCacheDir, "image-rebase")
-	_ = os.RemoveAll(tmpDir) // clean up before using it
-	if err := os.Mkdir(tmpDir, 0700); err != nil {
-		return "", "", err
-	}
-	defer func() {
-		_ = os.RemoveAll(tmpDir) // clean up after using it
-	}()
+func copyDiskImage(destDir string) (string, string, error) {
+	const destFormat = "qcow2"
 
-	// We copy the overlay file because before doing the commit, we need to adjust the backing file.
-	baseImageName := filepath.Base(srcDir)
-	srcOverlayDiskPath := filepath.Join(constants.MachineInstanceDir, constants.DefaultName, baseImageName)
-	destOverlayDiskPath := filepath.Join(tmpDir, "overlayImage.qcow2")
-	if err := crcos.CopyFileContents(srcOverlayDiskPath, destOverlayDiskPath, 0644); err != nil {
-		return "", "", err
-	}
+	imageName := fmt.Sprintf("%s.qcow2", constants.DefaultName)
 
-	// We copy the base image because this is where we are going to commit the changes which were made to the VM
-	if err := crcos.CopyFileContents(srcDir,
-		filepath.Join(tmpDir, baseImageName), 0644); err != nil {
-		return "", "", err
-	}
+	srcPath := filepath.Join(constants.MachineInstanceDir, constants.DefaultName, imageName)
+	destPath := filepath.Join(destDir, imageName)
 
-	// Use qemu-img commands to rebase and commit it.
-	format := "qcow2"
-	if _, _, err := crcos.RunWithDefaultLocale("qemu-img", "rebase", "-F", format, "-b",
-		filepath.Join(tmpDir, baseImageName), destOverlayDiskPath); err != nil {
-		return "", "", err
-	}
-	if _, _, err := crcos.RunWithDefaultLocale("qemu-img", "commit", destOverlayDiskPath); err != nil {
+	_, _, err := crcos.RunWithDefaultLocale("qemu-img", "convert", "-f", "qcow2", "-O", destFormat, srcPath, destPath)
+	if err != nil {
 		return "", "", err
 	}
 
-	// Copy the final image to destination dir
-	if err := crcos.CopyFileContents(filepath.Join(tmpDir, baseImageName), filepath.Join(destDir, baseImageName), 0644); err != nil {
-		return "", "", err
-	}
-	return filepath.Join(destDir, baseImageName), format, nil
+	return destPath, destFormat, nil
 }
diff --git a/pkg/crc/machine/generate_bundle_nonlinux.go b/pkg/crc/machine/generate_bundle_nonlinux.go
index f723a309a..39eb64682 100644
--- a/pkg/crc/machine/generate_bundle_nonlinux.go
+++ b/pkg/crc/machine/generate_bundle_nonlinux.go
@@ -7,6 +7,6 @@ import (
 	"runtime"
 )
 
-func copyDiskImage(dirName string, srcDir string) (string, string, error) {
+func copyDiskImage(dirName string) (string, string, error) {
 	return "", "", fmt.Errorf("Not implemented for %s", runtime.GOOS)
 }


Test patch:
diff --git a/pkg/crc/machine/bundle/metadata_test.go b/pkg/crc/machine/bundle/metadata_test.go
index 68bc3efde..6c4a6a80b 100644
--- a/pkg/crc/machine/bundle/metadata_test.go
+++ b/pkg/crc/machine/bundle/metadata_test.go
@@ -2,9 +2,15 @@ package bundle
 
 import (
 	"encoding/json"
+	"strings"
 	"testing"
+	"unicode"
+
+	"github.com/code-ready/crc/pkg/crc/constants"
+	"github.com/code-ready/crc/pkg/crc/logging"
 
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
 const reference = `{
@@ -118,3 +124,29 @@ func TestUnmarshalMarshal(t *testing.T) {
 	assert.NoError(t, err)
 	assert.JSONEq(t, string(bin), reference)
 }
+
+// check that the bundle name has the form "crc_libvirt_4.7.8.crcbundle" or "crc_libvirt_4.7.8_123456.crcbundle"
+func checkBundleName(t *testing.T, bundleName string) {
+	logging.Debugf("Checking bundle '%s", bundleName)
+	baseName := GetBundleNameWithoutExtension(constants.GetDefaultBundle())
+	require.True(t, strings.HasPrefix(bundleName, baseName), "%s should start with %s", bundleName, baseName)
+	bundleName = bundleName[len(baseName):]
+	require.True(t, strings.HasSuffix(bundleName, ".crcbundle"), "%s should have a '.crcbundle' extension", bundleName)
+	bundleName = bundleName[:len(bundleName)-len(".crcbundle")]
+	if bundleName == "" {
+		return
+	}
+	require.GreaterOrEqual(t, len(bundleName), 2)
+	require.Equal(t, rune(bundleName[0]), '_')
+	for _, char := range bundleName[1:] {
+		require.True(t, unicode.IsDigit(char), "%c should be a digit", char)
+	}
+}
+
+func TestCustomBundleName(t *testing.T) {
+	checkBundleName(t, constants.GetDefaultBundle())
+	customBundleName := GetCustomBundleName(constants.GetDefaultBundle())
+	checkBundleName(t, customBundleName)
+	customBundleName = GetCustomBundleName(customBundleName)
+	checkBundleName(t, customBundleName)
+}