danm icon indicating copy to clipboard operation
danm copied to clipboard

MatchExistingSvc() has logic problem

Open nokia-t1zhou opened this issue 4 years ago • 1 comments

Is this a BUG REPORT or FEATURE REQUEST?:

Uncomment only one, leave it on its own line:

bug

feature

What happened:

func MatchExistingSvc(de *danmv1.DanmEp, servicesList []*corev1.Service) []*corev1.Service {
	deNs := de.Namespace
	var svcList []*corev1.Service
	for _, svc := range servicesList {
		annotations := svc.GetAnnotations()
		selectorMap, svcNets, err := GetDanmSvcAnnotations(annotations)
		if err != nil {
			return svcList
		}
		if len(selectorMap) == 0 || !isDepSelectedBySvc(de, svcNets) || svc.GetNamespace() != deNs {
			continue
		}
		deMap := de.GetLabels()
		deFit := IsContain(deMap, selectorMap)
		if !deFit {
			continue
		}
		svcList = append(svcList, svc.DeepCopy())
	}
	return svcList
}

if GetDanmSvcAnnotations() return with error code, if the K8s service with wrong format of DANM service exist, then will cause some K8s endpoint can't be update/modified,
What you expected to happen: i think the code should be:

		annotations := svc.GetAnnotations()
		selectorMap, svcNets, err := GetDanmSvcAnnotations(annotations)
		if err != nil {
			continue
		}

by the way: my service YAML file with below Annotations: danm.k8s.io/selector: '{ "nwservice": "l1-nwmgmtservice" }' svcwatcher will ouput these error log: utils.go:44] utils: json error: invalid character 'f' after object key:value pair and DANM can't update K8s endpoint with selected IP. but if use below Annotations: danm.k8s.io/selector: '{"nwservice":"l1-nwmgmtservice"}' everything is OK.

nokia-t1zhou avatar May 14 '20 02:05 nokia-t1zhou

yeah, changing the return statement to continue makes sense

Levovar avatar May 14 '20 09:05 Levovar