Skip to content

Commit

Permalink
Improve error handling with vlanpool
Browse files Browse the repository at this point in the history 
Fix an issue with pool delete
Don't pick up vlan when not in pool for NAD

Signed-off-by: Kiran Shastri <shastrinator@gmail.com>
  • Loading branch information
@shastrinator
shastrinator committed Sep 16, 2023
1 parent eab1ab6 commit ddedab1
Show file tree
Hide file tree
Showing 3 changed files with 40 additions and 35 deletions.
2 changes: 1 addition & 1 deletion pkg/controller/fabricvlanpools.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,7 @@ func (cont *AciController) handleFabricVlanPoolUpdate(obj interface{}) bool {

func (cont *AciController) handleFabricVlanPoolDelete(key string) bool {
cont.log.Infof("fabricvlanpool delete: %s", key)
fabricVlanPoolElems := strings.Split("/", key)
fabricVlanPoolElems := strings.Split(key, "/")
cont.indexMutex.Lock()
defer cont.indexMutex.Unlock()
nsVlanPoolMap, ok := cont.fabricVlanPoolMap[fabricVlanPoolElems[0]]
Expand Down
7 changes: 4 additions & 3 deletions pkg/controller/nodefabricnetworkattachments.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,14 +46,12 @@ func (cont *AciController) updateGlobalConfig(encapStr string, progMap map[strin
cont.log.Errorf("Cannot set globalscope objects in per-port vlan mode")
return
}
if cont.globalVlanConfig.SharedPhysDom == nil {
return
}
_, encapBlks, err := cont.parseNodeFabNetAttVlanList(encapStr)
if err != nil {
cont.log.Errorf("Error updating GlobalScopeVlanConfig: %v", err)
return
}
cont.log.Infof("Setting globalvlanpool: %s", encapStr)
apicSlice = cont.updateNodeFabNetAttDom(encapBlks, globalScopeVlanDomPrefix)
labelKey := cont.aciNameForKey("nfna", globalScopeVlanDomPrefix)
progMap[labelKey] = apicSlice
Expand Down Expand Up @@ -282,6 +280,9 @@ func (cont *AciController) populateFabricPaths(epg apicapi.ApicObject, encap int
}

func (cont *AciController) parseNodeFabNetAttVlanList(vlan string) (vlans []int, vlanBlks []string, err error) {
if vlan == "" {
return vlans, vlanBlks, err
}
listContents := vlan
_, after, found := strings.Cut(vlan, "[")
if found {
Expand Down
66 changes: 35 additions & 31 deletions pkg/hostagent/netattachdef.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ import (
"github.com/k8snetworkplumbingwg/sriovnet"
fabattv1 "github.com/noironetworks/aci-containers/pkg/fabricattachment/apis/aci.fabricattachment/v1"
md "github.com/noironetworks/aci-containers/pkg/metadata"
"github.com/noironetworks/aci-containers/pkg/util"
apierrors "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
Expand Down Expand Up @@ -82,7 +83,6 @@ type NetworkAttachmentData struct {
EncapKey string
PluginVlan string
Programmed bool
vlanUnspecified bool
}

type ClientInfo struct {
Expand Down Expand Up @@ -287,9 +287,6 @@ func (agent *HostAgent) LoadCniNetworks() error {
for _, localIface := range netAttData.Ifaces {
agent.netattdefifacemap[localIface] = &netAttData
}
if netAttData.vlanUnspecified {
agent.orphanNadMap[netAttKey] = &netAttData
}
} else {
agent.log.Errorf("Failed to read additional network file %s:%v", path, err2)
return nil
Expand Down Expand Up @@ -408,19 +405,14 @@ func (agent *HostAgent) updateNodeFabricNetworkAttachmentForEncapChangeLocked(na
return
}
if !isDelete {
netAttData.vlanUnspecified = false
vlanList = agent.getAllowedVlansLocked(vlanList, netAttData.Namespace)
netAttData.EncapVlan = vlanList
netAttData.EncapKey = encapKey
agent.log.Debugf("Using nadVlanMap: %s", vlanList)
agent.log.Debugf("Using nadVlanMap allowed list : %s", vlanList)
} else {
vlan, _ := strconv.Atoi(netAttData.PluginVlan)
agent.handlePluginVlan(netAttData, vlan)
}
if netAttData.vlanUnspecified {
agent.orphanNadMap[nadKey] = netAttData
} else {
delete(agent.orphanNadMap, nadKey)
}
fabNetAttName := agent.config.NodeName + "-" + netAttData.Namespace + "-" + netAttData.Name
fabNetAtt, err := agent.fabAttClient.NodeFabricNetworkAttachments(fabNetAttDefNamespace).Get(context.TODO(), fabNetAttName, metav1.GetOptions{})
if err == nil {
Expand All @@ -438,13 +430,9 @@ func (agent *HostAgent) updateNodeFabricNetworkAttachmentForEncapChangeLocked(na
}

func (agent *HostAgent) updateNodeFabricNetworkAttachmentForFabricVlanPoolLocked() {
for _, netAttData := range agent.orphanNadMap {
vlanStr := agent.getFabricVlanPool(netAttData.Namespace, true)
if vlanStr != "" {
netAttData.EncapVlan = vlanStr
} else {
netAttData.EncapVlan = "0"
}
for _, netAttData := range agent.netattdefmap {
vlan, _ := strconv.Atoi(netAttData.PluginVlan)
agent.handlePluginVlan(netAttData, vlan)
fabNetAttName := agent.config.NodeName + "-" + netAttData.Namespace + "-" + netAttData.Name
fabNetAtt, err := agent.fabAttClient.NodeFabricNetworkAttachments(fabNetAttDefNamespace).Get(context.TODO(), fabNetAttName, metav1.GetOptions{})
if err == nil {
Expand Down Expand Up @@ -595,13 +583,39 @@ func validateVlanAnnotation(vlanStr string) (string, error) {
return resultStr, nil
}

func (agent *HostAgent) getAllowedVlansLocked(encapStr string, namespace string) (allowedStr string) {
poolStr := agent.getFabricVlanPool(namespace, true)
poolVlans, _, _, err := util.ParseVlanList([]string{poolStr})
if err != nil {
return ""
}
vlans, _, _, err := util.ParseVlanList([]string{encapStr})
if err != nil {
return ""
}
vlanMap := make(map[int]bool)
for _, elem := range vlans {
vlanMap[elem] = false
}
for _, elem := range poolVlans {
if _, ok := vlanMap[elem]; ok {
if allowedStr == "" {
allowedStr = fmt.Sprintf("%d", elem)
continue
}
allowedStr += fmt.Sprintf(",%d", elem)
}
}
return allowedStr
}

func (agent *HostAgent) handlePluginVlan(netAttData *NetworkAttachmentData, vlan int) {
netAttData.vlanUnspecified = false
nadKey := netAttData.Namespace + "/" + netAttData.Name
netAttData.PluginVlan = fmt.Sprintf("%d", vlan)
if vlan == 0 {
if val, ok := netAttData.KnownAnnots[vlanAnnot]; ok {
if vlanStr, err := validateVlanAnnotation(val); err == nil {
vlanStr = agent.getAllowedVlansLocked(vlanStr, netAttData.Namespace)
agent.log.Debugf("Using annotation: %s", vlanStr)
netAttData.EncapVlan = vlanStr
netAttData.EncapKey = ""
Expand All @@ -612,19 +626,15 @@ func (agent *HostAgent) handlePluginVlan(netAttData *NetworkAttachmentData, vlan
}
matchingKey, vlanList, match := agent.getNadVlanMapMatchLocked(nadKey)
if match {
vlanList = agent.getAllowedVlansLocked(vlanList, netAttData.Namespace)
agent.log.Debugf("Using nadVlanMap: %s", vlanList)
netAttData.EncapVlan = vlanList
netAttData.EncapKey = matchingKey
return
}
encap := "0"
netAttData.vlanUnspecified = true
vlanStr := agent.getFabricVlanPool(netAttData.Namespace, true)
agent.log.Debugf("Using vlan pool: %s", vlanStr)
if vlanStr != "" {
encap = vlanStr
}
netAttData.EncapVlan = encap
netAttData.EncapVlan = vlanStr
netAttData.EncapKey = ""
return
}
Expand Down Expand Up @@ -774,11 +784,6 @@ func (agent *HostAgent) networkAttDefChanged(ntd *netpolicy.NetworkAttachmentDef
}
}
}
if netattdata.vlanUnspecified {
agent.orphanNadMap[netAttDefKey] = &netattdata
} else {
delete(agent.orphanNadMap, netAttDefKey)
}
if err := agent.updateNodeFabricNetworkAttachmentLocked(&netattdata); err != nil {
agent.log.Errorf("Failed to create/update nodefabricnetworkattachment %s :%v", nodeFabNetAttName, err)
}
Expand Down Expand Up @@ -849,7 +854,6 @@ func (agent *HostAgent) networkAttDefDeleted(obj interface{}) {
delete(agent.netattdefifacemap, iface)
}
}
delete(agent.orphanNadMap, netAttDefKey)
agent.DeleteNetworkMetadata(netattDef)
}
delete(agent.netattdefmap, netAttDefKey)
Expand Down

0 comments on commit ddedab1

Please sign in to comment.