Merge pull request #159 from canonical/meta-gadget-yaml

FR-5759 - Use `meta/gadget.yaml` for the gadget yaml path for classic

debian/changelog

@@ -34,6 +34,7 @@ ubuntu-image (3.0+23.04ubuntu1) UNRELEASED; urgency=medium
   * Support the keep-enabled parameter in ubuntu-image extra-ppas
   * Clean image build leftovers
   * Make sure the fstab file is cleanly overridden (LP: #2031889)
+  * Always look for gadget.yaml file in meta directory
   * Add a make-dirs manual customization option to create directories in the rootfs
 
   [ Alfonso Sanchez-Beato ]

internal/commands/pack.go

@@ -3,7 +3,7 @@ package commands
 // PackOpts holds all flags that are specific to the pack command
 type PackOpts struct {
 	ArtifactType string `long:"artifact-type" description:"Type of the resulting disk image file." required:"true" default:"raw" choice:"raw"`
-	GadgetDir    string `long:"gadget-dir" description:"Directory containing the gadget.yaml and the gadget tree." required:"true"`
+	GadgetDir    string `long:"gadget-dir" description:"Directory containing the gadget tree. The gadget.yaml file is expected to be in a meta subdirectory." required:"true"`
 	RootfsDir    string `long:"rootfs-dir" description:"Directory containing the rootfs" required:"true"`
 }
 

internal/statemachine/classic_states.go

@@ -430,14 +430,12 @@ func (stateMachine *StateMachine) buildGadgetTree() error {
 		return fmt.Errorf("Error creating scratch/gadget directory: %s", err.Error())
 	}
 
-	var sourceDir string
 	switch classicStateMachine.ImageDef.Gadget.GadgetType {
 	case "git":
 		err := cloneGitRepo(classicStateMachine.ImageDef, gadgetDir)
 		if err != nil {
 			return fmt.Errorf("Error cloning gadget repository: \"%s\"", err.Error())
 		}
-		sourceDir = gadgetDir
 	case "directory":
 		gadgetTreePath := strings.TrimPrefix(classicStateMachine.ImageDef.Gadget.GadgetURL, "file://")
 		if !filepath.IsAbs(gadgetTreePath) {
@@ -455,8 +453,6 @@ func (stateMachine *StateMachine) buildGadgetTree() error {
 				return fmt.Errorf("Error copying gadget source: %s", err.Error())
 			}
 		}
-
-		sourceDir = filepath.Join(gadgetDir)
 	}
 
 	// now run "make" to build the gadget tree
@@ -474,7 +470,7 @@ func (stateMachine *StateMachine) buildGadgetTree() error {
 	}...)
 	// add the current ENV to the command
 	makeCmd.Env = append(makeCmd.Env, os.Environ()...)
-	makeCmd.Dir = sourceDir
+	makeCmd.Dir = gadgetDir
 
 	makeOutput := helper.SetCommandOutput(makeCmd, classicStateMachine.commonFlags.Debug)
 
@@ -503,20 +499,20 @@ func (stateMachine *StateMachine) prepareGadgetTree() error {
 			gadgetTree, _ = filepath.Abs(gadgetTree)
 		}
 	} else {
-		gadgetTree = filepath.Join(classicStateMachine.tempDirs.scratch, "gadget")
+		gadgetTree = filepath.Join(classicStateMachine.tempDirs.scratch, "gadget", "install")
 	}
-	files, err := osReadDir(gadgetTree)
+	entries, err := osReadDir(gadgetTree)
 	if err != nil {
 		return fmt.Errorf("Error reading gadget tree: %s", err.Error())
 	}
-	for _, gadgetFile := range files {
-		srcFile := filepath.Join(gadgetTree, gadgetFile.Name())
+	for _, gadgetEntry := range entries {
+		srcFile := filepath.Join(gadgetTree, gadgetEntry.Name())
 		if err := osutilCopySpecialFile(srcFile, gadgetDir); err != nil {
-			return fmt.Errorf("Error copying gadget tree: %s", err.Error())
+			return fmt.Errorf("Error copying gadget tree entry: %s", err.Error())
 		}
 	}
 
-	classicStateMachine.YamlFilePath = filepath.Join(gadgetDir, "gadget.yaml")
+	classicStateMachine.YamlFilePath = filepath.Join(gadgetDir, gadgetYamlPathInTree)
 
 	return nil
 }

internal/statemachine/classic_test.go

@@ -430,10 +430,13 @@ func TestPrepareGadgetTree(t *testing.T) {
 		err := stateMachine.makeTemporaryDirectories()
 		asserter.AssertErrNil(err, true)
 
-		// place a test gadget tree in the  scratch directory so we don't have to build one
+		// place a test gadget tree in the scratch directory so we don't have to build one
+		gadgetDir := filepath.Join(stateMachine.tempDirs.scratch, "gadget")
+		err = os.MkdirAll(gadgetDir, 0755)
+		asserter.AssertErrNil(err, true)
+
 		gadgetSource := filepath.Join("testdata", "gadget_tree")
-		gadgetDest := filepath.Join(stateMachine.tempDirs.scratch, "gadget")
-		err = osutil.CopySpecialFile(gadgetSource, gadgetDest)
+		err = osutil.CopySpecialFile(gadgetSource, filepath.Join(gadgetDir, "install"))
 		asserter.AssertErrNil(err, true)
 
 		err = stateMachine.prepareGadgetTree()
@@ -505,9 +508,12 @@ func TestFailedPrepareGadgetTree(t *testing.T) {
 		asserter.AssertErrNil(err, true)
 
 		// place a test gadget tree in the  scratch directory so we don't have to build one
+		gadgetDir := filepath.Join(stateMachine.tempDirs.scratch, "gadget")
+		err = os.MkdirAll(gadgetDir, 0755)
+		asserter.AssertErrNil(err, true)
+
 		gadgetSource := filepath.Join("testdata", "gadget_tree")
-		gadgetDest := filepath.Join(stateMachine.tempDirs.scratch, "gadget")
-		err = osutil.CopySpecialFile(gadgetSource, gadgetDest)
+		err = osutil.CopySpecialFile(gadgetSource, filepath.Join(gadgetDir, "install"))
 		asserter.AssertErrNil(err, true)
 
 		// mock os.Mkdir
@@ -2895,8 +2901,8 @@ func TestBuildGadgetTreeGit(t *testing.T) {
 func TestBuildGadgetTreeDirectory(t *testing.T) {
 	t.Run("test_build_gadget_tree_directory", func(t *testing.T) {
 		asserter := helper.Asserter{T: t}
-		saveCWD := helper.SaveCWD()
-		defer saveCWD()
+		restoreCWD := helper.SaveCWD()
+		defer restoreCWD()
 
 		var stateMachine ClassicStateMachine
 		stateMachine.commonFlags, stateMachine.stateMachineFlags = helper.InitCommonOpts()
@@ -2905,11 +2911,12 @@ func TestBuildGadgetTreeDirectory(t *testing.T) {
 		// need workdir set up for this
 		err := stateMachine.makeTemporaryDirectories()
 		asserter.AssertErrNil(err, true)
+		t.Cleanup(func() { os.RemoveAll(stateMachine.stateMachineFlags.WorkDir) })
 
 		// git clone the gadget into a /tmp dir
 		gadgetDir, err := os.MkdirTemp("", "pc-gadget-")
 		asserter.AssertErrNil(err, true)
-		defer os.RemoveAll(gadgetDir)
+		t.Cleanup(func() { os.RemoveAll(gadgetDir) })
 		gitCloneCommand := *exec.Command(
 			"git",
 			"clone",
@@ -2924,7 +2931,7 @@ func TestBuildGadgetTreeDirectory(t *testing.T) {
 		asserter.AssertErrNil(err, true)
 
 		// now set up the image definition to build from this directory
-		imageDef := imagedefinition.ImageDefinition{
+		stateMachine.ImageDef = imagedefinition.ImageDefinition{
 			Architecture: getHostArch(),
 			Series:       getHostSuite(),
 			Gadget: &imagedefinition.Gadget{
@@ -2933,8 +2940,6 @@ func TestBuildGadgetTreeDirectory(t *testing.T) {
 			},
 		}
 
-		stateMachine.ImageDef = imageDef
-
 		err = stateMachine.buildGadgetTree()
 		asserter.AssertErrNil(err, true)
 
@@ -2944,9 +2949,6 @@ func TestBuildGadgetTreeDirectory(t *testing.T) {
 		asserter.AssertErrNil(err, true)
 		err = stateMachine.loadGadgetYaml()
 		asserter.AssertErrNil(err, true)
-
-		os.RemoveAll(gadgetDir)
-		os.RemoveAll(stateMachine.stateMachineFlags.WorkDir)
 	})
 }
 
@@ -4242,8 +4244,12 @@ func TestFailedUpdateBootloader(t *testing.T) {
 
 		// place a test gadget tree in the scratch directory so we don't
 		// have to build one
+		gadgetDir := filepath.Join(stateMachine.tempDirs.scratch, "gadget")
+		err = os.MkdirAll(gadgetDir, 0755)
+		asserter.AssertErrNil(err, true)
+
 		gadgetSource := filepath.Join("testdata", "gadget_tree")
-		gadgetDest := filepath.Join(stateMachine.tempDirs.scratch, "gadget")
+		gadgetDest := filepath.Join(gadgetDir, "install")
 		err = osutil.CopySpecialFile(gadgetSource, gadgetDest)
 		asserter.AssertErrNil(err, true)
 		// also copy gadget.yaml to the root of the scratch/gadget dir
@@ -4301,11 +4307,15 @@ func TestUnsupportedBootloader(t *testing.T) {
 
 		// place a test gadget tree in the scratch directory so we don't
 		// have to build one
+		gadgetDir := filepath.Join(stateMachine.tempDirs.scratch, "gadget")
+		err = os.MkdirAll(gadgetDir, 0755)
+		asserter.AssertErrNil(err, true)
+
 		gadgetSource := filepath.Join("testdata", "gadget_tree")
-		gadgetDest := filepath.Join(stateMachine.tempDirs.scratch, "gadget")
+		gadgetDest := filepath.Join(gadgetDir, "install")
 		err = osutil.CopySpecialFile(gadgetSource, gadgetDest)
 		asserter.AssertErrNil(err, true)
-		// also copy gadget.yaml to the root of the scratc/gadget dir
+		// also copy gadget.yaml to the root of the scratch/gadget dir
 		err = osutil.CopyFile(
 			filepath.Join(gadgetDest, "meta", "gadget.yaml"),
 			filepath.Join(gadgetDest, "gadget.yaml"),

internal/statemachine/common_states.go

@@ -83,7 +83,9 @@ func (stateMachine *StateMachine) loadGadgetYaml() error {
 	gadgetYamlDst := filepath.Join(stateMachine.stateMachineFlags.WorkDir, "gadget.yaml")
 	if err := osutilCopyFile(stateMachine.YamlFilePath,
 		gadgetYamlDst, osutil.CopyFlagOverwrite); err != nil {
-		return fmt.Errorf("Error copying gadget.yaml to %s: %s", gadgetYamlDst, err.Error())
+		return fmt.Errorf(`Error copying gadget.yaml to %s: %s
+The gadget.yaml file is expected to be located in a "meta" subdirectory of the provided built gadget directory.
+`, gadgetYamlDst, err.Error())
 	}
 
 	// read in the gadget.yaml as bytes, because snapd expects it that way

internal/statemachine/common_test.go

@@ -209,6 +209,7 @@ func TestFailedLoadGadgetYaml(t *testing.T) {
 		}()
 		err = stateMachine.loadGadgetYaml()
 		asserter.AssertErrContains(err, "Error copying gadget.yaml")
+		asserter.AssertErrContains(err, "\nThe gadget.yaml file is expected to be located in a \"meta\" subdirectory of the provided built gadget directory.\n")
 		osutilCopyFile = osutil.CopyFile
 
 		// mock osReadFile

internal/statemachine/pack_states.go

@@ -11,7 +11,7 @@ import (
 func (stateMachine *StateMachine) preparePack() error {
 	packStateMachine := stateMachine.parent.(*PackStateMachine)
 
-	packStateMachine.YamlFilePath = filepath.Join(packStateMachine.Opts.GadgetDir, "gadget.yaml")
+	packStateMachine.YamlFilePath = filepath.Join(packStateMachine.Opts.GadgetDir, gadgetYamlPathInTree)
 
 	return nil
 }

internal/statemachine/snap_states.go

@@ -77,7 +77,7 @@ func (stateMachine *StateMachine) prepareImage() error {
 	}
 
 	// set the gadget yaml location
-	snapStateMachine.YamlFilePath = filepath.Join(stateMachine.tempDirs.unpack, "gadget", "meta", "gadget.yaml")
+	snapStateMachine.YamlFilePath = filepath.Join(stateMachine.tempDirs.unpack, "gadget", gadgetYamlPathInTree)
 
 	return nil
 }

internal/statemachine/state_machine.go

@@ -33,6 +33,8 @@ const (
 	metadataStateFile = "ubuntu-image.json"
 )
 
+var gadgetYamlPathInTree = filepath.Join("meta", "gadget.yaml")
+
 // define some functions that can be mocked by test cases
 var gadgetLayoutVolume = gadget.LayoutVolume
 var gadgetNewMountedFilesystemWriter = gadget.NewMountedFilesystemWriter