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.

Sign up for GitHub

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

Jump to bottom

federation service controller has trouble deciding when there is no delta #28135

Closed
mml opened this issue Jun 27, 2016 · 4 comments
Closed

federation service controller has trouble deciding when there is no delta #28135

mml opened this issue Jun 27, 2016 · 4 comments
Labels
priority/backlog Higher priority than priority/awaiting-more-evidence.
Milestone
next-candidate

Comments

@mml
Copy link
Contributor

mml commented Jun 27, 2016

E.g.,

I0627 22:26:46.221856       1 dns.go:215] Existing recordset                <(clouddns) "federated-service.e2e-tests-service-cuza5.federation.svc.us-central1-c.us-central1.kube.5yetis.net." type=CNAME rrdatas=["federated-service.e2e-tests-service-cuza5.federation.svc.us-central1.kube.5yetis.net."] ttl=180>
I0627 22:26:46.221885       1 dns.go:216] ... not equal to needed recordset <(clouddns) "federated-service.e2e-tests-service-cuza5.federation.svc.us-central1-c.us-central1.kube.5yetis.net." type=CNAME rrdatas=["federated-service.e2e-tests-service-cuza5.federation.svc.us-central1.kube.5yetis.net."] ttl=180>
I0627 22:26:46.221919       1 dns.go:217] ... removing existing and adding needed.

See also #28134
@mml
Copy link
Contributor Author

mml commented Jun 27, 2016

@kubernetes/sig-cluster-federation

@mml mml changed the title federation service manager has trouble deciding when there is no delta federation service controller has trouble deciding when there is no delta Jun 27, 2016
@ghost
Copy link

ghost commented Jun 27, 2016

Yes, I actually anticipated that problem. There is a fairly simple fix. This should not present a significant issue in the real world, but worth fixing.

https://github.com/kubernetes/kubernetes/blob/master/federation/pkg/federation-controller/service/dns.go#L208

@ghost ghost self-assigned this Jun 27, 2016
@ghost ghost added team/control-plane priority/backlog Higher priority than priority/awaiting-more-evidence. labels Jun 27, 2016
@ghost ghost added this to the v1.3 milestone Jun 27, 2016
@ghost
Copy link

ghost commented Jun 27, 2016

@mml This is simply because these are two different instances of ResourceRecordSet being compared, each with different internal field values, but identical external field values. I will replace the == with an equivalent() function, which only compares external fields.

@goltermann
Copy link
Contributor

@mml @quinton-hoole we're probably building 1.3.0 today, so this one's likely out unless it's on a cherrypick list in an hour.

@ghost ghost added the cherrypick-candidate label Jun 29, 2016
@goltermann goltermann modified the milestones: next-candidate, v1.3 Jul 1, 2016
k8s-github-robot pushed a commit that referenced this issue Jul 22, 2016
Merge pull request #28182 from quinton-hoole/2016-06-28-compare-rrset…
72a697a 
…s-properly

Automatic merge from submit-queue

Federation: Use equivalent() instead of == to compare DNS ResourceRecordSets. Fixes #28135

Without this we sometimes incorrectly assume that DNS records hosted on cloud DNS servers need to be replaced when they don't need to be.  This results in unnecessary churn on DNS records.

cc: @nikhiljindal @madhusudancs @mfanjie FYI
zefciu pushed a commit to zefciu/kubernetes that referenced this issue Jul 28, 2016
@zefciu
Use ResourceRecordSetsEquivalent() instead of == to compare rrsets. F…
793eadd 
…ixes kubernetes#28135
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
priority/backlog Higher priority than priority/awaiting-more-evidence.
Projects
None yet
Milestone
next-candidate
Development

No branches or pull requests
2 participants
@mml @goltermann