Move to json storage, address pblindex issue

This patch changes the way we persist the data on disk when
running Calico/VPP. Instead of using struc and binary format
we transition to json files. Size should not be an issue as number
of pods per node are typically low (~100). This will make
troubleshooting easier and errors clearer when parsing fails.

We thus remove the /bin/debug troubleshooting utility as the
data format is not human readable.

Doing this, we address an issue where PBL indexes were reused
upon dataplane restart, as they were stored in a list. We now
will use a map to retain the containerIP mapping.

We also split the configuration from runtime spec in LocalPodSpec
and add a step to clear it when corresponding VRFs are not found
in VPP.

Finally we address an issue where uRPF was not properly set up
for ipv6.

Signed-off-by: Nathan Skrzypczak <nathan.skrzypczak@gmail.com>

Author: sknat

calico-vpp-agent/Dockerfile

@@ -3,7 +3,6 @@ FROM ubuntu:22.04
 LABEL maintainer="aloaugus@cisco.com"
 
 ADD bin/gobgp /bin/gobgp
-ADD bin/debug /bin/debug
 ADD version /etc/calicovppversion
 ADD bin/felix-api-proxy /bin/felix-api-proxy
 ADD bin/calico-vpp-agent /bin/calico-vpp-agent

calico-vpp-agent/Makefile

@@ -23,7 +23,6 @@ felix-api-proxy: bin
 
 build: felix-api-proxy bin
 	${DOCKER_RUN} go build -o ./bin/calico-vpp-agent ./cmd
-	${DOCKER_RUN} go build -o ./bin/debug ./cmd/debug-state
 
 gobgp: bin
 	${DOCKER_RUN} go build -o ./bin/gobgp github.com/osrg/gobgp/v3/cmd/gobgp/

calico-vpp-agent/cmd/debug-state/debug-state.go

@@ -182,7 +182,7 @@ var _ = Describe("Pod-related functionality of CNI", func() {
 						Workload: &cniproto.WorkloadIDs{
 							Annotations: map[string]string{
 								// needed just for setting up steering of traffic to default Tun/Tap and to secondary Memif
-								cni.VppAnnotationPrefix + cni.MemifPortAnnotation: fmt.Sprintf("tcp:%d-%d,udp:%d-%d",
+								config.VppAnnotationPrefix + config.MemifPortAnnotation: fmt.Sprintf("tcp:%d-%d,udp:%d-%d",
 									memifTCPPortStart, memifTCPPortEnd, memifUDPPortStart, memifUDPPortEnd),
 							},
 						},
@@ -418,7 +418,7 @@ var _ = Describe("Pod-related functionality of CNI", func() {
 							Workload: &cniproto.WorkloadIDs{
 								Annotations: map[string]string{
 									// needed just for setting up steering of traffic to default Tun/Tap and to secondary Memif
-									cni.VppAnnotationPrefix + cni.MemifPortAnnotation: fmt.Sprintf("tcp:%d-%d,udp:%d-%d",
+									config.VppAnnotationPrefix + config.MemifPortAnnotation: fmt.Sprintf("tcp:%d-%d,udp:%d-%d",
 										memifTCPPortStart, memifTCPPortEnd, memifUDPPortStart, memifUDPPortEnd),
 								},
 							},

calico-vpp-agent/cni/cni_pod_test.go

@@ -72,19 +72,6 @@ func swIfIdxToIfName(idx uint32) string {
 	return fmt.Sprintf("vpp-tun-%d", idx)
 }
 
-func getHostEndpointProto(proto string) types.IPProto {
-	switch proto {
-	case "udp":
-		return types.UDP
-	case "sctp":
-		return types.SCTP
-	case "tcp":
-		return types.TCP
-	default:
-		return types.TCP
-	}
-}
-
 func (s *Server) SetFelixConfig(felixConfig *felixConfig.Config) {
 	s.tuntapDriver.SetFelixConfig(felixConfig)
 }
@@ -93,127 +80,20 @@ func (s *Server) SetOurBGPSpec(nodeBGPSpec *common.LocalNodeSpec) {
 	s.nodeBGPSpec = nodeBGPSpec
 }
 
-func (s *Server) newLocalPodSpecFromAdd(request *cniproto.AddRequest) (*storage.LocalPodSpec, error) {
-	podSpec := storage.LocalPodSpec{
-		InterfaceName:     request.GetInterfaceName(),
-		NetnsName:         request.GetNetns(),
-		AllowIpForwarding: request.GetSettings().GetAllowIpForwarding(),
-		Routes:            make([]storage.LocalIPNet, 0),
-		ContainerIps:      make([]storage.LocalIP, 0),
-		Mtu:               int(request.GetSettings().GetMtu()),
-
-		IfPortConfigs: make([]storage.LocalIfPortConfigs, 0),
-
-		OrchestratorID: request.Workload.Orchestrator,
-		WorkloadID:     request.Workload.Namespace + "/" + request.Workload.Pod,
-		EndpointID:     request.Workload.Endpoint,
-		HostPorts:      make([]storage.HostPortBinding, 0),
-
-		/* defaults */
-		IfSpec:       GetDefaultIfSpec(true /* isL3 */),
-		PBLMemifSpec: GetDefaultIfSpec(false /* isL3 */),
-
-		V4VrfId: vpplink.InvalidID,
-		V6VrfId: vpplink.InvalidID,
-
-		MemifSwIfIndex:  vpplink.InvalidID,
-		TunTapSwIfIndex: vpplink.InvalidID,
-
-		NetworkName: request.DataplaneOptions["network_name"],
-	}
-
-	if podSpec.NetworkName != "" {
-		if !*config.GetCalicoVppFeatureGates().MultinetEnabled {
-			return nil, fmt.Errorf("enable multinet in config for multiple networks")
-		}
-		if isMemif(podSpec.InterfaceName) {
-			if !*config.GetCalicoVppFeatureGates().MemifEnabled {
-				return nil, fmt.Errorf("enable memif in config for memif interfaces")
-			}
-			podSpec.EnableMemif = true
-			podSpec.DefaultIfType = storage.VppIfTypeMemif
-			podSpec.IfSpec = GetDefaultIfSpec(false)
-		}
-	}
-
-	for _, port := range request.Workload.Ports {
-		hostIP := net.ParseIP(port.HostIp)
-		hostPort := uint16(port.HostPort)
-		if hostPort != 0 && hostIP != nil && !hostIP.IsUnspecified() {
-			podSpec.HostPorts = append(podSpec.HostPorts, storage.HostPortBinding{
-				HostPort:      hostPort,
-				HostIP:        hostIP,
-				ContainerPort: uint16(port.Port),
-				Protocol:      getHostEndpointProto(port.Protocol),
-			})
-		} else if hostPort != 0 {
-			// default to node IP
-			podSpec.HostPorts = append(podSpec.HostPorts, storage.HostPortBinding{
-				HostPort:      hostPort,
-				HostIP:        net.ParseIP(s.nodeBGPSpec.IPv4Address.IP.String()),
-				ContainerPort: uint16(port.Port),
-				Protocol:      getHostEndpointProto(port.Protocol),
-			})
-		}
-	}
-	for _, routeStr := range request.GetContainerRoutes() {
-		_, route, err := net.ParseCIDR(routeStr)
-		if err != nil {
-			return nil, errors.Wrapf(err, "Cannot parse container route %s", routeStr)
-		}
-		podSpec.Routes = append(podSpec.Routes, storage.LocalIPNet{
-			IP:   route.IP,
-			Mask: route.Mask,
-		})
-	}
+func (s *Server) Add(ctx context.Context, request *cniproto.AddRequest) (*cniproto.AddReply, error) {
+	/* We don't support request.GetDesiredHostInterfaceName() */
+	podSpec, err := storage.NewLocalPodSpecFromAdd(request, s.nodeBGPSpec)
 	if podSpec.NetworkName != "" {
 		value, ok := s.networkDefinitions.Load(podSpec.NetworkName)
 		if !ok {
-			s.log.Errorf("trying to create a pod in an unexisting network %s", podSpec.NetworkName)
+			return nil, fmt.Errorf("trying to create a pod in an unexisting network %s", podSpec.NetworkName)
 		} else {
 			_, route, err := net.ParseCIDR(value.(*watchers.NetworkDefinition).Range)
 			if err == nil {
-				podSpec.Routes = append(podSpec.Routes, storage.LocalIPNet{
-					IP:   route.IP,
-					Mask: route.Mask,
-				})
+				podSpec.Routes = append(podSpec.Routes, *route)
 			}
 		}
 	}
-	for _, requestContainerIP := range request.GetContainerIps() {
-		containerIp, _, err := net.ParseCIDR(requestContainerIP.GetAddress())
-		if err != nil {
-			return nil, fmt.Errorf("Cannot parse address: %s", requestContainerIP.GetAddress())
-		}
-		// We ignore the prefix len set on the address,
-		// for a tun it doesn't make sense
-		podSpec.ContainerIps = append(podSpec.ContainerIps, storage.LocalIP{IP: containerIp})
-	}
-	workload := request.GetWorkload()
-	if workload != nil {
-		err := s.ParsePodAnnotations(&podSpec, workload.Annotations)
-		if err != nil {
-			return nil, errors.Wrapf(err, "Cannot parse pod Annotations")
-		}
-	}
-
-	if podSpec.DefaultIfType == storage.VppIfTypeUnknown {
-		podSpec.DefaultIfType = storage.VppIfTypeTunTap
-	}
-
-	return &podSpec, nil
-}
-
-func NewLocalPodSpecFromDel(request *cniproto.DelRequest) *storage.LocalPodSpec {
-	return &storage.LocalPodSpec{
-		InterfaceName: request.GetInterfaceName(),
-		NetnsName:     request.GetNetns(),
-	}
-}
-
-func (s *Server) Add(ctx context.Context, request *cniproto.AddRequest) (*cniproto.AddReply, error) {
-	/* We don't support request.GetDesiredHostInterfaceName() */
-	podSpec, err := s.newLocalPodSpecFromAdd(request)
 	if err != nil {
 		s.log.Errorf("Error parsing interface add request %v %v", request, err)
 		return &cniproto.AddReply{
@@ -248,7 +128,7 @@ func (s *Server) Add(ctx context.Context, request *cniproto.AddRequest) (*cnipro
 		}, nil
 	}
 	if len(config.GetCalicoVppInitialConfig().RedirectToHostRules) != 0 && podSpec.NetworkName == "" {
-		err := s.AddRedirectToHostToInterface(podSpec.TunTapSwIfIndex)
+		err := s.AddRedirectToHostToInterface(podSpec.Status.TunTapSwIfIndex)
 		if err != nil {
 			return nil, err
 		}
@@ -311,7 +191,7 @@ func (s *Server) rescanState() {
 	s.lock.Lock()
 	defer s.lock.Unlock()
 	for _, podSpec := range podSpecs {
-		/* copy podSpec as a pointer to it will be sent over the event chan */
+		// we copy podSpec as a pointer to it will be sent over the event chan
 		podSpecCopy := podSpec.Copy()
 		_, err := s.AddVppInterface(&podSpecCopy, false /* doHostSideConf */)
 		switch err.(type) {
@@ -324,7 +204,7 @@ func (s *Server) rescanState() {
 			s.log.Errorf("Interface add failed %s : %v", podSpecCopy.String(), err)
 		}
 		if len(config.GetCalicoVppInitialConfig().RedirectToHostRules) != 0 && podSpecCopy.NetworkName == "" {
-			err := s.AddRedirectToHostToInterface(podSpecCopy.TunTapSwIfIndex)
+			err := s.AddRedirectToHostToInterface(podSpecCopy.Status.TunTapSwIfIndex)
 			if err != nil {
 				s.log.Error(err)
 			}
@@ -355,9 +235,9 @@ func (s *Server) AddRedirectToHostToInterface(swIfIndex uint32) error {
 }
 
 func (s *Server) Del(ctx context.Context, request *cniproto.DelRequest) (*cniproto.DelReply, error) {
-	partialPodSpec := NewLocalPodSpecFromDel(request)
+	podSpecKey := storage.LocalPodSpecKey(request.GetNetns(), request.GetInterfaceName())
 	// Only try to delete the device if a namespace was passed in.
-	if partialPodSpec.NetnsName == "" {
+	if request.GetNetns() == "" {
 		s.log.Debugf("no netns passed, skipping")
 		return &cniproto.DelReply{
 			Successful: true,
@@ -366,17 +246,17 @@ func (s *Server) Del(ctx context.Context, request *cniproto.DelRequest) (*cnipro
 	s.lock.Lock()
 	defer s.lock.Unlock()
 
-	s.log.Infof("pod(del) key=%s", partialPodSpec.Key())
-	initialSpec, ok := s.podInterfaceMap[partialPodSpec.Key()]
+	s.log.Infof("pod(del) key=%s", podSpecKey)
+	initialSpec, ok := s.podInterfaceMap[podSpecKey]
 	if !ok {
-		s.log.Warnf("Unknown pod to delete key=%s", partialPodSpec.Key())
+		s.log.Warnf("Unknown pod to delete key=%s", podSpecKey)
 	} else {
 		s.log.Infof("pod(del) spec=%s", initialSpec.String())
 		s.DelVppInterface(&initialSpec)
 		s.log.Infof("pod(del) Done! spec=%s", initialSpec.String())
 	}
 
-	delete(s.podInterfaceMap, initialSpec.Key())
+	delete(s.podInterfaceMap, podSpecKey)
 	err := storage.PersistCniServerState(s.podInterfaceMap, config.CniServerStateFile+fmt.Sprint(storage.CniServerStateFileVersion))
 	if err != nil {
 		s.log.Errorf("CNI state persist errored %v", err)
@@ -451,16 +331,16 @@ forloop:
 				}
 
 				for _, podSpec := range s.podInterfaceMap {
-					NeededSnat := podSpec.NeedsSnat
+					NeededSnat := podSpec.Status.NeedsSnat
 					for _, containerIP := range podSpec.GetContainerIps() {
-						podSpec.NeedsSnat = podSpec.NeedsSnat || s.policyServerIpam.IPNetNeedsSNAT(containerIP)
+						podSpec.Status.NeedsSnat = podSpec.Status.NeedsSnat || s.policyServerIpam.IPNetNeedsSNAT(containerIP)
 					}
-					if NeededSnat != podSpec.NeedsSnat {
-						for _, swIfIndex := range []uint32{podSpec.LoopbackSwIfIndex, podSpec.TunTapSwIfIndex, podSpec.MemifSwIfIndex} {
+					if NeededSnat != podSpec.Status.NeedsSnat {
+						for _, swIfIndex := range []uint32{podSpec.Status.LoopbackSwIfIndex, podSpec.Status.TunTapSwIfIndex, podSpec.Status.MemifSwIfIndex} {
 							if swIfIndex != vpplink.InvalidID {
 								s.log.Infof("Enable/Disable interface[%d] SNAT", swIfIndex)
 								for _, ipFamily := range vpplink.IpFamilies {
-									err := s.vpp.EnableDisableCnatSNAT(swIfIndex, ipFamily.IsIp6, podSpec.NeedsSnat)
+									err := s.vpp.EnableDisableCnatSNAT(swIfIndex, ipFamily.IsIp6, podSpec.Status.NeedsSnat)
 									if err != nil {
 										return errors.Wrapf(err, "Error enabling/disabling %s snat", ipFamily.Str)
 									}