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

MatchExistingSvc() has logic problem #219

Open
nokia-t1zhou opened this issue May 14, 2020 · 1 comment
Open

MatchExistingSvc() has logic problem #219

nokia-t1zhou opened this issue May 14, 2020 · 1 comment
Labels
bug Something isn't working

Comments

@nokia-t1zhou
Copy link

nokia-t1zhou commented May 14, 2020

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.

@Levovar
Copy link
Collaborator

Levovar commented May 14, 2020

yeah, changing the return statement to continue makes sense

@Levovar Levovar added the bug Something isn't working label May 18, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants