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: consider modes when checking for deps availability #9710

Merged
merged 3 commits into from Nov 30, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
8 changes: 8 additions & 0 deletions seed/seedwriter/helpers.go
Expand Up @@ -21,6 +21,7 @@ package seedwriter

import (
"fmt"
"strings"

"github.com/snapcore/snapd/asserts"
"github.com/snapcore/snapd/asserts/snapasserts"
Expand Down Expand Up @@ -120,6 +121,13 @@ func checkType(sn *SeedSnap, model *asserts.Model) error {
return nil
}

func errorMsgForModesSuffix(modes []string) string {
if len(modes) == 1 && modes[0] == "run" {
return ""
}
return fmt.Sprintf(" for all relevant modes (%s)", strings.Join(modes, ", "))
}

type seedSnapsByType []*SeedSnap

func (s seedSnapsByType) Len() int { return len(s) }
Expand Down
17 changes: 13 additions & 4 deletions seed/seedwriter/seed16.go
Expand Up @@ -86,7 +86,8 @@ func (pol *policy16) extraSnapDefaultChannel() string {
return "stable"
}

func (pol *policy16) checkBase(info *snap.Info, availableSnaps *naming.SnapSet) error {
func (pol *policy16) checkBase(info *snap.Info, modes []string, availableByMode map[string]*naming.SnapSet) error {
availableSnaps := availableByMode["run"]
// snap needs no base (or it simply needs core which is never listed explicitly): nothing to do
if info.Base == "" {
if info.Type() == snap.TypeGadget || info.Type() == snap.TypeApp {
Expand All @@ -109,7 +110,13 @@ func (pol *policy16) checkBase(info *snap.Info, availableSnaps *naming.SnapSet)
return fmt.Errorf("cannot add snap %q without also adding its base %q explicitly", info.SnapName(), info.Base)
}

func (pol *policy16) needsImplicitSnaps(availableSnaps *naming.SnapSet) (bool, error) {
func (pol *policy16) checkAvailable(snapRef naming.SnapRef, modes []string, availableByMode map[string]*naming.SnapSet) bool {
availableSnaps := availableByMode["run"]
return availableSnaps.Contains(snapRef)
}

func (pol *policy16) needsImplicitSnaps(availableByMode map[string]*naming.SnapSet) (bool, error) {
availableSnaps := availableByMode["run"]
// do we need to add implicitly either snapd (or core)
hasCore := availableSnaps.Contains(naming.Snap("core"))
if len(pol.needsCore) != 0 && !hasCore {
Expand All @@ -131,7 +138,8 @@ func (pol *policy16) needsImplicitSnaps(availableSnaps *naming.SnapSet) (bool, e
return false, nil
}

func (pol *policy16) implicitSnaps(availableSnaps *naming.SnapSet) []*asserts.ModelSnap {
func (pol *policy16) implicitSnaps(availableByMode map[string]*naming.SnapSet) []*asserts.ModelSnap {
availableSnaps := availableByMode["run"]
if len(pol.needsCore) != 0 && !availableSnaps.Contains(naming.Snap("core")) {
return []*asserts.ModelSnap{makeSystemSnap("core")}
}
Expand All @@ -141,7 +149,8 @@ func (pol *policy16) implicitSnaps(availableSnaps *naming.SnapSet) []*asserts.Mo
return nil
}

func (pol *policy16) implicitExtraSnaps(availableSnaps *naming.SnapSet) []*OptionsSnap {
func (pol *policy16) implicitExtraSnaps(availableByMode map[string]*naming.SnapSet) []*OptionsSnap {
availableSnaps := availableByMode["run"]
if len(pol.needsCore) != 0 && !availableSnaps.Contains(naming.Snap("core")) {
return []*OptionsSnap{{Name: "core"}}
}
Expand Down
39 changes: 32 additions & 7 deletions seed/seedwriter/seed20.go
Expand Up @@ -81,7 +81,7 @@ func (pol *policy20) extraSnapDefaultChannel() string {
return "latest/stable"
}

func (pol *policy20) checkBase(info *snap.Info, availableSnaps *naming.SnapSet) error {
func (pol *policy20) checkBase(info *snap.Info, modes []string, availableByMode map[string]*naming.SnapSet) error {
base := info.Base
if base == "" {
if info.Type() != snap.TypeGadget && info.Type() != snap.TypeApp {
Expand All @@ -90,32 +90,57 @@ func (pol *policy20) checkBase(info *snap.Info, availableSnaps *naming.SnapSet)
base = "core"
}

if availableSnaps.Contains(naming.Snap(base)) {
if pol.checkAvailable(naming.Snap(base), modes, availableByMode) {
return nil
}

whichBase := fmt.Sprintf("its base %q", base)
if base == "core16" {
if availableSnaps.Contains(naming.Snap("core")) {
if pol.checkAvailable(naming.Snap("core"), modes, availableByMode) {
return nil
}
whichBase += ` (or "core")`
}

return fmt.Errorf("cannot add snap %q without also adding %s explicitly", info.SnapName(), whichBase)
return fmt.Errorf("cannot add snap %q without also adding %s explicitly%s", info.SnapName(), whichBase, errorMsgForModesSuffix(modes))
}

func (pol *policy20) needsImplicitSnaps(*naming.SnapSet) (bool, error) {
func (pol *policy20) checkAvailable(snapRef naming.SnapRef, modes []string, availableByMode map[string]*naming.SnapSet) bool {
// checks that snapRef is available in all modes
for _, mode := range modes {
byMode := availableByMode[mode]
if !byMode.Contains(snapRef) {
if mode == "run" || mode == "ephemeral" {
// no additional fallback for these
// cases:
// * run is not ephemeral,
// is covered only by run
// * ephemeral is only covered by ephemeral
return false
Copy link
Collaborator

Choose a reason for hiding this comment

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

// no additional fallback check for well known modes

}
// all non-run modes (e.g. recover) are
// considered ephemeral, as a fallback check
// if the snap is listed under the ephemeral mode label
ephem := availableByMode["ephemeral"]
if ephem == nil || !ephem.Contains(snapRef) {
return false
}
}
}
return true
}

func (pol *policy20) needsImplicitSnaps(map[string]*naming.SnapSet) (bool, error) {
// no implicit snaps with Core 20
// TODO: unless we want to support them for extra snaps
return false, nil
}

func (pol *policy20) implicitSnaps(*naming.SnapSet) []*asserts.ModelSnap {
func (pol *policy20) implicitSnaps(map[string]*naming.SnapSet) []*asserts.ModelSnap {
return nil
}

func (pol *policy20) implicitExtraSnaps(*naming.SnapSet) []*OptionsSnap {
func (pol *policy20) implicitExtraSnaps(map[string]*naming.SnapSet) []*OptionsSnap {
return nil
}

Expand Down
52 changes: 38 additions & 14 deletions seed/seedwriter/writer.go
Expand Up @@ -90,6 +90,15 @@ type SeedSnap struct {
optionSnap *OptionsSnap
}

func (sn *SeedSnap) modes() []string {
if sn.modelSnap == nil {
// run is the assumed mode for extra snaps not listed
// in the model
return []string{"run"}
Copy link
Collaborator

Choose a reason for hiding this comment

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

// default mode for extra snaps not listed in the model

}
return sn.modelSnap.Modes
}

var _ naming.SnapRef = (*SeedSnap)(nil)

/* Writer writes Core 16/18 and Core 20 seeds.
Expand Down Expand Up @@ -172,7 +181,8 @@ type Writer struct {
localSnaps map[*OptionsSnap]*SeedSnap
byRefLocalSnaps *naming.SnapSet

availableSnaps *naming.SnapSet
availableSnaps *naming.SnapSet
availableByMode map[string]*naming.SnapSet

// toDownload tracks which set of snaps SnapsToDownload should compute
// next
Expand All @@ -194,11 +204,13 @@ type policy interface {
modelSnapDefaultChannel() string
extraSnapDefaultChannel() string

checkBase(*snap.Info, *naming.SnapSet) error
checkBase(s *snap.Info, modes []string, availableByMode map[string]*naming.SnapSet) error

checkAvailable(snpRef naming.SnapRef, modes []string, availableByMode map[string]*naming.SnapSet) bool

needsImplicitSnaps(*naming.SnapSet) (bool, error)
implicitSnaps(*naming.SnapSet) []*asserts.ModelSnap
implicitExtraSnaps(*naming.SnapSet) []*OptionsSnap
needsImplicitSnaps(availableByMode map[string]*naming.SnapSet) (bool, error)
implicitSnaps(availableByMode map[string]*naming.SnapSet) []*asserts.ModelSnap
implicitExtraSnaps(availableByMode map[string]*naming.SnapSet) []*OptionsSnap
}

type tree interface {
Expand Down Expand Up @@ -769,11 +781,11 @@ func (w *Writer) SnapsToDownload() (snaps []*SeedSnap, err error) {
}
return toDownload, nil
case toDownloadImplicit:
return w.modelSnapsToDownload(w.policy.implicitSnaps(w.availableSnaps))
return w.modelSnapsToDownload(w.policy.implicitSnaps(w.availableByMode))
case toDownloadExtra:
return w.extraSnapsToDownload(w.optExtraSnaps())
case toDownloadExtraImplicit:
return w.extraSnapsToDownload(w.policy.implicitExtraSnaps(w.availableSnaps))
return w.extraSnapsToDownload(w.policy.implicitExtraSnaps(w.availableByMode))
default:
panic(fmt.Sprintf("unknown to-download set: %d", w.toDownload))
}
Expand Down Expand Up @@ -820,7 +832,7 @@ func (w *Writer) resolveChannel(whichSnap string, modSnap *asserts.ModelSnap, op
return resChannel, nil
}

func (w *Writer) checkBase(info *snap.Info) error {
func (w *Writer) checkBase(info *snap.Info, modes []string) error {
// Sanity check, note that we could support this case
// if we have a use-case but it requires changes in the
// devicestate/firstboot.go ordering code.
Expand All @@ -833,19 +845,29 @@ func (w *Writer) checkBase(info *snap.Info) error {
return nil
}

return w.policy.checkBase(info, w.availableSnaps)
return w.policy.checkBase(info, modes, w.availableByMode)
}

func (w *Writer) downloaded(seedSnaps []*SeedSnap) error {
if w.availableSnaps == nil {
w.availableSnaps = naming.NewSnapSet(nil)
w.availableByMode = make(map[string]*naming.SnapSet)
w.availableByMode["run"] = naming.NewSnapSet(nil)
}

for _, sn := range seedSnaps {
if sn.Info == nil {
return fmt.Errorf("internal error: before seedwriter.Writer.Downloaded snap %q Info should have been set", sn.SnapName())
}
w.availableSnaps.Add(sn)
for _, mode := range sn.modes() {
byMode := w.availableByMode[mode]
if byMode == nil {
byMode = naming.NewSnapSet(nil)
w.availableByMode[mode] = byMode
}
byMode.Add(sn)
}
}

for _, sn := range seedSnaps {
Expand Down Expand Up @@ -873,14 +895,16 @@ func (w *Writer) downloaded(seedSnaps []*SeedSnap) error {
return fmt.Errorf("cannot use classic snap %q in a core system", info.SnapName())
}

if err := w.checkBase(info); err != nil {
modes := sn.modes()

if err := w.checkBase(info, modes); err != nil {
return err
}
// error about missing default providers
for dp := range snap.NeededDefaultProviders(info) {
if !w.availableSnaps.Contains(naming.Snap(dp)) {
if !w.policy.checkAvailable(naming.Snap(dp), modes, w.availableByMode) {
// TODO: have a way to ignore this issue on a snap by snap basis?
return fmt.Errorf("cannot use snap %q without its default content provider %q being added explicitly", info.SnapName(), dp)
return fmt.Errorf("cannot use snap %q without its default content provider %q being added explicitly%s", info.SnapName(), dp, errorMsgForModesSuffix(modes))
}
}

Expand Down Expand Up @@ -925,7 +949,7 @@ func (w *Writer) Downloaded() (complete bool, err error) {

switch w.toDownload {
case toDownloadModel:
implicitNeeded, err := w.policy.needsImplicitSnaps(w.availableSnaps)
implicitNeeded, err := w.policy.needsImplicitSnaps(w.availableByMode)
if err != nil {
return false, err
}
Expand All @@ -942,7 +966,7 @@ func (w *Writer) Downloaded() (complete bool, err error) {
return false, nil
}
case toDownloadExtra:
implicitNeeded, err := w.policy.needsImplicitSnaps(w.availableSnaps)
implicitNeeded, err := w.policy.needsImplicitSnaps(w.availableByMode)
if err != nil {
return false, err
}
Expand Down