Skip to content

Commit

Permalink
o/ifacestate: unify code into autoConnectChecker.addAutoConnections
Browse files Browse the repository at this point in the history 
We had variations of the same code twice for auto-connect and as well for
hotplug-connect, this unifies them in one implementation on
autoConnectChecker. There is not much of a line win but this code will
need to change to support slots-per-plugs: * and otherwise we would
have needed 3 set of changes.
  • Loading branch information
@pedronis
pedronis authored Oct 30, 2019
2 parents 8f3b353 + b34edcf commit 43c3ad9
Show file tree
Hide file tree
Showing 3 changed files with 161 additions and 135 deletions.
180 changes: 48 additions & 132 deletions overlord/ifacestate/handlers.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -461,7 +461,7 @@ func (m *InterfaceManager) doConnect(task *state.Task, _ *tomb.Tomb) error {
// policy "connection" rules, other auto-connections obey the
// "auto-connection" rules
if autoConnect && !byGadget {
autochecker, err := newAutoConnectChecker(st, deviceCtx)
autochecker, err := newAutoConnectChecker(st, task, m.repo, deviceCtx)
if err != nil {
return err
}
Expand Down Expand Up @@ -945,6 +945,17 @@ func batchConnectTasks(st *state.State, snapsup *snapstate.SnapSetup, conns map[
return ts, nil
}

func filterForSlot(slot *snap.SlotInfo) func(candSlots []*snap.SlotInfo) []*snap.SlotInfo {
return func(candSlots []*snap.SlotInfo) []*snap.SlotInfo {
for _, candSlot := range candSlots {
if candSlot.String() == slot.String() {
return []*snap.SlotInfo{slot}
}
}
return nil
}
}

// doAutoConnect creates task(s) to connect the given snap to viable candidates.
func (m *InterfaceManager) doAutoConnect(task *state.Task, _ *tomb.Tomb) error {
st := task.State()
Expand Down Expand Up @@ -976,7 +987,7 @@ func (m *InterfaceManager) doAutoConnect(task *state.Task, _ *tomb.Tomb) error {

snapName := snapsup.InstanceName()

autochecker, err := newAutoConnectChecker(st, deviceCtx)
autochecker, err := newAutoConnectChecker(st, task, m.repo, deviceCtx)
if err != nil {
return err
}
Expand Down Expand Up @@ -1011,58 +1022,20 @@ func (m *InterfaceManager) doAutoConnect(task *state.Task, _ *tomb.Tomb) error {
slots := m.repo.Slots(snapName)
newconns := make(map[string]*interfaces.ConnRef, len(plugs)+len(slots))

// Auto-connect all the plugs
for _, plug := range plugs {
candidates := m.repo.AutoConnectCandidateSlots(snapName, plug.Name, autochecker.check)
if len(candidates) == 0 {
continue
}
// If we are in a core transition we may have both the old ubuntu-core
// snap and the new core snap providing the same interface. In that
// situation we want to ignore any candidates in ubuntu-core and simply
// go with those from the new core snap.
if len(candidates) == 2 {
switch {
case candidates[0].Snap.InstanceName() == "ubuntu-core" && candidates[1].Snap.InstanceName() == "core":
candidates = candidates[1:2]
case candidates[1].Snap.InstanceName() == "ubuntu-core" && candidates[0].Snap.InstanceName() == "core":
candidates = candidates[0:1]
}
}
if len(candidates) != 1 {
crefs := make([]string, len(candidates))
for i, candidate := range candidates {
crefs[i] = candidate.String()
}
task.Logf("cannot auto-connect plug %s, candidates found: %s", plug, strings.Join(crefs, ", "))
continue
}
slot := candidates[0]
connRef := interfaces.NewConnRef(plug, slot)
key := connRef.ID()
if _, ok := conns[key]; ok {
// Suggested connection already exist (or has Undesired flag set) so don't clobber it.
// NOTE: we don't log anything here as this is a normal and common condition.
continue
}

ignore, err := findSymmetricAutoconnectTask(st, plug.Snap.InstanceName(), slot.Snap.InstanceName(), task)
if err != nil {
return err
}

if ignore {
continue
conflictError := func(retry *state.Retry, err error) error {
if retry != nil {
task.Logf("Waiting for conflicting change in progress: %s", retry.Reason)
return retry // will retry
}
return fmt.Errorf("auto-connect conflict check failed: %v", err)
}

if err := checkAutoconnectConflicts(st, task, plug.Snap.InstanceName(), slot.Snap.InstanceName()); err != nil {
if retry, ok := err.(*state.Retry); ok {
task.Logf("Waiting for conflicting change in progress: %s", retry.Reason)
return err // will retry
}
return fmt.Errorf("auto-connect conflict check failed: %s", err)
}
newconns[connRef.ID()] = connRef
// Auto-connect all the plugs
cannotAutoConnectLog := func(plug *snap.PlugInfo, candRefs []string) string {
return fmt.Sprintf("cannot auto-connect plug %s, candidates found: %s", plug, strings.Join(candRefs, ", "))
}
if err := autochecker.addAutoConnections(newconns, plugs, nil, conns, cannotAutoConnectLog, conflictError); err != nil {
return err
}
// Auto-connect all the slots
for _, slot := range slots {
Expand All @@ -1071,49 +1044,11 @@ func (m *InterfaceManager) doAutoConnect(task *state.Task, _ *tomb.Tomb) error {
continue
}

for _, plug := range candidates {
// make sure slot is the only viable
// connection for plug, same check as if we were
// considering auto-connections from plug
candSlots := m.repo.AutoConnectCandidateSlots(plug.Snap.InstanceName(), plug.Name, autochecker.check)

if len(candSlots) != 1 || candSlots[0].String() != slot.String() {
crefs := make([]string, len(candSlots))
for i, candidate := range candSlots {
crefs[i] = candidate.String()
}
task.Logf("cannot auto-connect slot %s to %s, candidates found: %s", slot, plug, strings.Join(crefs, ", "))
continue
}

connRef := interfaces.NewConnRef(plug, slot)
key := connRef.ID()
if _, ok := conns[key]; ok {
// Suggested connection already exist (or has Undesired flag set) so don't clobber it.
// NOTE: we don't log anything here as this is a normal and common condition.
continue
}
if _, ok := newconns[key]; ok {
continue
}

ignore, err := findSymmetricAutoconnectTask(st, plug.Snap.InstanceName(), slot.Snap.InstanceName(), task)
if err != nil {
return err
}

if ignore {
continue
}

if err := checkAutoconnectConflicts(st, task, plug.Snap.InstanceName(), slot.Snap.InstanceName()); err != nil {
if retry, ok := err.(*state.Retry); ok {
task.Logf("Waiting for conflicting change in progress: %s", retry.Reason)
return err // will retry
}
return fmt.Errorf("auto-connect conflict check failed: %s", err)
}
newconns[connRef.ID()] = connRef
cannotAutoConnectLog := func(plug *snap.PlugInfo, candRefs []string) string {
return fmt.Sprintf("cannot auto-connect slot %s to plug %s, candidates found: %s", slot, plug, strings.Join(candRefs, ", "))
}
if err := autochecker.addAutoConnections(newconns, candidates, filterForSlot(slot), conns, cannotAutoConnectLog, conflictError); err != nil {
return err
}
}

Expand Down Expand Up @@ -1381,6 +1316,14 @@ func (m *InterfaceManager) doHotplugConnect(task *state.Task, _ *tomb.Tomb) erro
// to recreate old connections that are only remembered in the state.
connsForDevice := findConnsForHotplugKey(conns, ifaceName, hotplugKey)

conflictError := func(retry *state.Retry, err error) error {
if retry != nil {
task.Logf("hotplug connect will be retried: %s", retry.Reason)
return retry // will retry
}
return fmt.Errorf("hotplug-connect conflict check failed: %v", err)
}

// find old connections to recreate
var recreate []*interfaces.ConnRef
for _, id := range connsForDevice {
Expand All @@ -1399,55 +1342,28 @@ func (m *InterfaceManager) doHotplugConnect(task *state.Task, _ *tomb.Tomb) erro
}

if err := checkAutoconnectConflicts(st, task, connRef.PlugRef.Snap, connRef.SlotRef.Snap); err != nil {
if retry, ok := err.(*state.Retry); ok {
task.Logf("hotplug connect will be retried: %s", retry.Reason)
return err // will retry
}
return fmt.Errorf("hotplug connect conflict check failed: %s", err)
retry, _ := err.(*state.Retry)
return conflictError(retry, err)
}
recreate = append(recreate, connRef)
}

// find new auto-connections
autochecker, err := newAutoConnectChecker(st, deviceCtx)
autochecker, err := newAutoConnectChecker(st, task, m.repo, deviceCtx)
if err != nil {
return err
}

instanceName := slot.Snap.InstanceName()
candidates := m.repo.AutoConnectCandidatePlugs(instanceName, slot.Name, autochecker.check)

var newconns []*interfaces.ConnRef
// Auto-connect the slots
for _, plug := range candidates {
// make sure slot is the only viable
// connection for plug, same check as if we were
// considering auto-connections from plug
candSlots := m.repo.AutoConnectCandidateSlots(plug.Snap.InstanceName(), plug.Name, autochecker.check)
if len(candSlots) != 1 || candSlots[0].String() != slot.String() {
crefs := make([]string, len(candSlots))
for i, candidate := range candSlots {
crefs[i] = candidate.String()
}
task.Logf("cannot auto-connect slot %s to %s, candidates found: %s", slot, plug, strings.Join(crefs, ", "))
continue
}

connRef := interfaces.NewConnRef(plug, slot)
key := connRef.ID()
if _, ok := conns[key]; ok {
// existing connection, already considered by connsForDevice loop
continue
}

if err := checkAutoconnectConflicts(st, task, plug.Snap.InstanceName(), slot.Snap.InstanceName()); err != nil {
if retry, ok := err.(*state.Retry); ok {
task.Logf("hotplug connect will be retried: %s", retry.Reason)
return err // will retry
}
return fmt.Errorf("hotplug connect conflict check failed: %s", err)
}
newconns = append(newconns, connRef)
newconns := make(map[string]*interfaces.ConnRef, len(candidates))
// Auto-connect the plugs
cannotAutoConnectLog := func(plug *snap.PlugInfo, candRefs []string) string {
return fmt.Sprintf("cannot auto-connect hotplug slot %s to plug %s, candidates found: %s", slot, plug, strings.Join(candRefs, ", "))
}
if err := autochecker.addAutoConnections(newconns, candidates, filterForSlot(slot), conns, cannotAutoConnectLog, conflictError); err != nil {
return err
}

if len(recreate) == 0 && len(newconns) == 0 {
Expand Down
114 changes: 112 additions & 2 deletions overlord/ifacestate/helpers.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -449,19 +449,24 @@ type connState struct {
}

type autoConnectChecker struct {
st *state.State
st *state.State
task *state.Task
repo *interfaces.Repository

deviceCtx snapstate.DeviceContext
cache map[string]*asserts.SnapDeclaration
baseDecl *asserts.BaseDeclaration
}

func newAutoConnectChecker(s *state.State, deviceCtx snapstate.DeviceContext) (*autoConnectChecker, error) {
func newAutoConnectChecker(s *state.State, task *state.Task, repo *interfaces.Repository, deviceCtx snapstate.DeviceContext) (*autoConnectChecker, error) {
baseDecl, err := assertstate.BaseDeclaration(s)
if err != nil {
return nil, fmt.Errorf("internal error: cannot find base declaration: %v", err)
}
return &autoConnectChecker{
st: s,
task: task,
repo: repo,
deviceCtx: deviceCtx,
cache: make(map[string]*asserts.SnapDeclaration),
baseDecl: baseDecl,
Expand Down Expand Up @@ -527,6 +532,111 @@ func (c *autoConnectChecker) check(plug *interfaces.ConnectedPlug, slot *interfa
return ic.CheckAutoConnect() == nil, nil
}

// filterUbuntuCoreSlots filters out any ubuntu-core slots,
// if there are both ubuntu-core and core slots. This would occur
// during a ubuntu-core -> core transition.
func filterUbuntuCoreSlots(candidates []*snap.SlotInfo) []*snap.SlotInfo {
hasCore := false
hasUbuntuCore := false
var withoutUbuntuCore []*snap.SlotInfo
for i, candSlot := range candidates {
switch candSlot.Snap.InstanceName() {
case "ubuntu-core":
if !hasUbuntuCore {
hasUbuntuCore = true
withoutUbuntuCore = append(withoutUbuntuCore, candidates[:i]...)
}
case "core":
hasCore = true
fallthrough
default:
if hasUbuntuCore {
withoutUbuntuCore = append(withoutUbuntuCore, candSlot)
}
}
}
if hasCore && hasUbuntuCore {
candidates = withoutUbuntuCore
}
return candidates
}

// addAutoConnections adds to newconns any applicable auto-connections
// from the given plugs to corresponding candidates slots after
// filtering them with optional filter and against preexisting
// conns. cannotAutoConnectLog is called to build a log message in
// case no applicable pair was found. conflictError is called
// to handle checkAutoconnectConflicts errors.
func (c *autoConnectChecker) addAutoConnections(newconns map[string]*interfaces.ConnRef, plugs []*snap.PlugInfo, filter func([]*snap.SlotInfo) []*snap.SlotInfo, conns map[string]*connState, cannotAutoConnectLog func(plug *snap.PlugInfo, candRefs []string) string, conflictError func(*state.Retry, error) error) error {
for _, plug := range plugs {
candSlots := c.repo.AutoConnectCandidateSlots(plug.Snap.InstanceName(), plug.Name, c.check)

if len(candSlots) == 0 {
continue
}

// If we are in a core transition we may have both the
// old ubuntu-core snap and the new core snap
// providing the same interface. In that situation we
// want to ignore any candidates in ubuntu-core and
// simply go with those from the new core snap.
candSlots = filterUbuntuCoreSlots(candSlots)

applicable := candSlots
// candidate arity check
if len(candSlots) != 1 {
applicable = nil
}

if filter != nil {
applicable = filter(applicable)
}

if len(applicable) == 0 {
crefs := make([]string, len(candSlots))
for i, candidate := range candSlots {
crefs[i] = candidate.String()
}
c.task.Logf(cannotAutoConnectLog(plug, crefs))
continue
}

for _, slot := range applicable {
connRef := interfaces.NewConnRef(plug, slot)
key := connRef.ID()
if _, ok := conns[key]; ok {
// Suggested connection already exist (or has
// Undesired flag set) so don't clobber it.
// NOTE: we don't log anything here as this is
// a normal and common condition.
continue
}
if _, ok := newconns[key]; ok {
continue
}

if c.task.Kind() == "auto-connect" {
ignore, err := findSymmetricAutoconnectTask(c.st, plug.Snap.InstanceName(), slot.Snap.InstanceName(), c.task)
if err != nil {
return err
}

if ignore {
continue
}
}

if err := checkAutoconnectConflicts(c.st, c.task, plug.Snap.InstanceName(), slot.Snap.InstanceName()); err != nil {
retry, _ := err.(*state.Retry)
return conflictError(retry, err)
}
newconns[key] = connRef
}
}

return nil
}

type connectChecker struct {
st *state.State
deviceCtx snapstate.DeviceContext
Expand Down
2 changes: 1 addition & 1 deletion overlord/ifacestate/ifacestate_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4492,7 +4492,7 @@ func (s *interfaceManagerSuite) TestAutoConnectDuringCoreTransition(c *C) {

// Add a sample snap with a "network" plug which should be auto-connected.
// Normally it would not be auto connected because there are multiple
// provides but we have special support for this case so the old
// providers but we have special support for this case so the old
// ubuntu-core snap is ignored and we pick the new core snap.
snapInfo := s.mockSnap(c, sampleSnapYaml)

Expand Down

0 comments on commit 43c3ad9

Please sign in to comment.