Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

seed/seedwriter: fail early when system seed directory exists #10220

Merged
17 changes: 7 additions & 10 deletions overlord/devicestate/systems_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -460,18 +460,15 @@ func (s *createSystemSuite) TestCreateSystemWithSomeSnapsAlreadyExisting(c *C) {
filepath.Join(unassertedSnapsDir, "other-unasserted_1.0.snap"), 0)
c.Assert(err, IsNil)

// when a given snap in asserted snaps directory already exists, it is
// not copied over
// the unasserted snap goes into the snaps directory under the system
// directory, which triggers the error in creating the directory by
// seed writer
newFiles, dir, err = devicestate.CreateSystemForModelFromValidatedSnaps(modelWithUnasserted, "1234unasserted", s.db, infoGetter)

c.Assert(err, ErrorMatches, "unable to create .*/other-unasserted_1.0.snap: file exists")
c.Check(newFiles, DeepEquals, []string{
// returned as new file so that cleanup can be properly
// performed even if the file was partially written
filepath.Join(unassertedSnapsDir, "other-unasserted_1.0.snap"),
})
c.Check(dir, Equals, filepath.Join(boot.InitramfsUbuntuSeedDir, "systems/1234unasserted"))
c.Check(osutil.IsDirectory(filepath.Join(boot.InitramfsUbuntuSeedDir, "systems/1234unasserted")), Equals, true)
c.Assert(err, ErrorMatches, "mkdir .*/ubuntu-seed/systems/1234unasserted: file exists")
// we failed early
c.Check(newFiles, HasLen, 0)
c.Check(dir, Equals, "")
}

func (s *createSystemSuite) TestCreateSystemInfoAndAssertsChecks(c *C) {
Expand Down
5 changes: 4 additions & 1 deletion seed/seedwriter/seed20.go
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,10 @@ func (tr *tree20) mkFixedDirs() error {
return err
}

return os.MkdirAll(tr.systemDir, 0755)
if err := os.MkdirAll(filepath.Dir(tr.systemDir), 0755); err != nil {
return err
}
return os.Mkdir(tr.systemDir, 0755)
bboozzoo marked this conversation as resolved.
Show resolved Hide resolved
}

func (tr *tree20) ensureSystemSnapsDir() (string, error) {
Expand Down
42 changes: 38 additions & 4 deletions seed/seedwriter/writer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2156,8 +2156,9 @@ func (s *writerSuite) TestDownloadedCore20CheckBaseCoreXX(c *C) {
{[]interface{}{requiredBaseCore16Ent}, `cannot add snap "required-base-core16" without also adding its base "core16" \(or "core"\) explicitly`},
}

s.opts.Label = "20191003"
for _, t := range tests {
baseLabel := "20191003"
for idx, t := range tests {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we could also remove the dir instead? I don't have a very strong preference but wanted to mention it

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

using the index as a suffix seems reasonable enough to me

s.opts.Label = fmt.Sprintf("%s%d", baseLabel, idx)
snaps := []interface{}{
map[string]interface{}{
"name": "pc-kernel",
Expand Down Expand Up @@ -2326,7 +2327,7 @@ func (s *writerSuite) TestCore20NonDangerousDisallowedOptionsSnaps(c *C) {

pcFn := s.makeLocalSnap(c, "pc")

s.opts.Label = "20191107"
baseLabel := "20191107"

tests := []struct {
optSnap *seedwriter.OptionsSnap
Expand All @@ -2338,7 +2339,8 @@ func (s *writerSuite) TestCore20NonDangerousDisallowedOptionsSnaps(c *C) {

const expectedErr = `cannot override channels, add devmode snaps, local snaps, or extra snaps with a model of grade higher than dangerous`

for _, t := range tests {
for idx, t := range tests {
s.opts.Label = fmt.Sprintf("%s%d", baseLabel, idx)
w, err := seedwriter.New(model, s.opts)
c.Assert(err, IsNil)

Expand Down Expand Up @@ -3201,3 +3203,35 @@ func (s *writerSuite) TestSeedSnapsWriteMetaCore20SignedLocalAssertedSnaps(c *C)
// no options file was created
c.Check(filepath.Join(systemDir, "options.yaml"), testutil.FileAbsent)
}

func (s *writerSuite) TestSeedSnapsWriteCore20ErrWhenDirExists(c *C) {
model := s.Brands.Model("my-brand", "my-model", map[string]interface{}{
"display-name": "my model",
"architecture": "amd64",
"base": "core20",
"grade": "signed",
"snaps": []interface{}{
map[string]interface{}{
"name": "pc-kernel",
"id": s.AssertedSnapID("pc-kernel"),
"type": "kernel",
"default-channel": "20",
},
map[string]interface{}{
"name": "pc",
"id": s.AssertedSnapID("pc"),
"type": "gadget",
"default-channel": "20",
}},
})

err := os.MkdirAll(filepath.Join(s.opts.SeedDir, "systems", "1234"), 0755)
c.Assert(err, IsNil)
s.opts.Label = "1234"
w, err := seedwriter.New(model, s.opts)
c.Assert(err, IsNil)
c.Assert(w, NotNil)

_, err = w.Start(s.db, s.newFetcher)
c.Assert(err, ErrorMatches, "mkdir .*/seed/systems/1234: file exists")
}