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

Fix service mirror controller warnings for remote-discover services #11251

Merged
merged 3 commits into from Aug 16, 2023
Merged

Fix service mirror controller warnings for remote-discover services #11251

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
b2bfd10
Fix service mirror controller warnings for remote-discover services
alpeb Aug 15, 2023
e8add91
Feedback
alpeb Aug 15, 2023
c99c550
Rebase and fix isRemoteDiscovery call
alpeb Aug 15, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
22 changes: 16 additions & 6 deletions multicluster/service-mirror/cluster_watcher.go
View file
Open in desktop
Expand Up @@ -235,7 +235,7 @@ func (rcsw *RemoteClusterServiceWatcher) getMirroredServiceLabels(remoteService
return labels
}

if rcsw.isRemoteDiscovery(remoteService) {
if rcsw.isRemoteDiscovery(remoteService.Labels) {
labels[consts.RemoteDiscoveryLabel] = rcsw.link.TargetClusterName
labels[consts.RemoteServiceLabel] = remoteService.GetName()
}
Expand Down Expand Up @@ -440,7 +440,7 @@ func (rcsw *RemoteClusterServiceWatcher) handleRemoteServiceDeleted(ctx context.
func (rcsw *RemoteClusterServiceWatcher) handleRemoteServiceUpdated(ctx context.Context, ev *RemoteServiceUpdated) error {
rcsw.log.Infof("Updating mirror service %s/%s", ev.localService.Namespace, ev.localService.Name)

if rcsw.isRemoteDiscovery(ev.remoteUpdate) {
if rcsw.isRemoteDiscovery(ev.remoteUpdate.Labels) {
// The service is mirrored in remote discovery mode and any local
// endpoints for it should be deleted if they exist.
if ev.localEndpoints != nil {
Expand Down Expand Up @@ -550,7 +550,7 @@ func (rcsw *RemoteClusterServiceWatcher) handleRemoteServiceCreated(ctx context.
}
}

if rcsw.isRemoteDiscovery(remoteService) {
if rcsw.isRemoteDiscovery(remoteService.Labels) {
// For remote discovery services, skip creating gateway endpoints.
return nil
}
Expand Down Expand Up @@ -678,7 +678,7 @@ func (rcsw *RemoteClusterServiceWatcher) createGatewayEndpoints(ctx context.Cont
func (rcsw *RemoteClusterServiceWatcher) createOrUpdateService(service *corev1.Service) error {
localName := rcsw.mirroredResourceName(service.Name)

if rcsw.isExported(service.Labels) || rcsw.isRemoteDiscovery(service) {
if rcsw.isExported(service.Labels) || rcsw.isRemoteDiscovery(service.Labels) {
localService, err := rcsw.localAPIClient.Svc().Lister().Services(service.Namespace).Get(localName)
if err != nil {
if kerrors.IsNotFound(err) {
Expand Down Expand Up @@ -894,6 +894,10 @@ func (rcsw *RemoteClusterServiceWatcher) Start(ctx context.Context) error {
rcsw.log.Debugf("skipped processing endpoints object %s/%s: missing %s label", epNew.Namespace, epNew.Name, consts.DefaultExportedServiceSelector)
return
}
if rcsw.isRemoteDiscovery(epNew.Labels) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do the endpoints of an exported service have the exported label on them? Typically, the user would only label the exported service. Does the endpoints controller copy this label to the endpoints or something like that?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yup, it appears labels are copied into the endpoints automatically:

$ k -n emojivoto expose pod web-8559b97f7c-pk6xq --port 80
service/web-8559b97f7c-pk6xq exposed

$ k -n emojivoto label svc/web-8559b97f7c-pk6xq mirror.linkerd.io/exported=remote-discovery

$ k -n emojivoto get ep web-8559b97f7c-pk6xq -ojson | jq .metadata.labels
{
  "app": "web-svc",
  "linkerd.io/control-plane-ns": "linkerd",
  "linkerd.io/proxy-deployment": "web",
  "linkerd.io/workload-ns": "emojivoto",
  "mirror.linkerd.io/exported": "remote-discovery",
  "pod-template-hash": "8559b97f7c",
  "version": "v11"
}

rcsw.log.Debugf("skipped processing endpoints object %s/%s (service labeled for remote-discovery mode)", epNew.Namespace, epNew.Name)
return
}
rcsw.eventsQueue.Add(&OnUpdateEndpointsCalled{epNew})
},
},
Expand Down Expand Up @@ -1026,6 +1030,11 @@ func (rcsw *RemoteClusterServiceWatcher) repairEndpoints(ctx context.Context) er
continue
}

if _, ok := svc.Labels[consts.RemoteDiscoveryLabel]; ok {
rcsw.log.Debugf("Skipped repairing endpoints for service in remote-discovery mode %s/%s", svc.Namespace, svc.Name)
continue
}

endpoints, err := rcsw.localAPIClient.Endpoint().Lister().Endpoints(svc.Namespace).Get(svc.Name)
if err != nil {
if !kerrors.IsNotFound(err) {
Expand Down Expand Up @@ -1237,7 +1246,7 @@ func (rcsw *RemoteClusterServiceWatcher) isExported(l map[string]string) bool {
return selector.Matches(labels.Set(l)) || remoteDiscoverySelector.Matches(labels.Set(l))
}

func (rcsw *RemoteClusterServiceWatcher) isRemoteDiscovery(svc *corev1.Service) bool {
func (rcsw *RemoteClusterServiceWatcher) isRemoteDiscovery(l map[string]string) bool {
alpeb marked this conversation as resolved.
Show resolved Hide resolved
// Treat an empty remoteDiscoverySelector as "Nothing" instead of
// "Everything" so that when the remoteDiscoverySelector field is unset, we
// don't export all Services.
Expand All @@ -1249,5 +1258,6 @@ func (rcsw *RemoteClusterServiceWatcher) isRemoteDiscovery(svc *corev1.Service)
rcsw.log.Errorf("Invalid selector: %s", err)
return false
}
return remoteDiscoverySelector.Matches(labels.Set(svc.Labels))

return remoteDiscoverySelector.Matches(labels.Set(l))
}