Skip to content

Remove single namespace functionality#2474

Merged
siggy merged 3 commits intomasterfrom
siggy/remove-single-namespace-yesss
Mar 12, 2019
Merged

Remove single namespace functionality#2474
siggy merged 3 commits intomasterfrom
siggy/remove-single-namespace-yesss

Conversation

@siggy
Copy link
Member

@siggy siggy commented Mar 8, 2019

#1721 introduced a --single-namespace install flag,
enabling the control-plane to function within a single namespace. With
the introduction of ServiceProfiles, and upcoming identity changes, this
single namespace mode of operation is becoming less viable.

This change removes the --single-namespace install flag, and all
underlying support. The control-plane must have cluster-wide access to
operate.

A few related changes:

  • Remove --single-namespace from linkerd check, this motivates
    combining some check categories, as we can always assume cluster-wide
    requirements.
  • Simplify the k8s.ResourceAuthz API, as callers no longer need to
    make a decision based on cluster-wide vs. namespace-wide access.
    Components either have access, or they error out.
  • Modify the web dashboard to always assume ServiceProfiles are enabled.

Reverts #1721
Part of #2337

Signed-off-by: Andrew Seigner [email protected]

#1721 introduced a `--single-namespace` install flag,
enabling the control-plane to function within a single namespace. With
the introduction of ServiceProfiles, and upcoming identity changes, this
single namespace mode of operation is becoming less viable.

This change removes the `--single-namespace` install flag, and all
underlying support. The control-plane must have cluster-wide access to
operate.

A few related changes:
- Remove `--single-namespace` from `linkerd check`, this motivates
  combining some check categories, as we can always assume cluster-wide
  requirements.
- Simplify the `k8s.ResourceAuthz` API, as callers no longer need to
  make a decision based on cluster-wide vs. namespace-wide access.
  Components either have access, or they error out.
- Modify the web dashboard to always assume ServiceProfiles are enabled.

Reverts #1721
Part of #2337

Signed-off-by: Andrew Seigner <[email protected]>
@siggy siggy force-pushed the siggy/remove-single-namespace-yesss branch from a54d45a to 2367033 Compare March 8, 2019 17:41
Copy link
Contributor

@klingerf klingerf left a comment

Choose a reason for hiding this comment

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

⭐️ Yay! Such a huge cleanup. 🔪

Found one more single-namespace reference hanging around:

./controller/api/destination/k8s_resolver.go:72:	// In single namespace mode, we'll close the stream immediately and the proxy

And we can fix it with:

diff --git a/controller/api/destination/k8s_resolver.go b/controller/api/destination/k8s_resolver.go
index 0938dd82..b82f52be 100644
--- a/controller/api/destination/k8s_resolver.go
+++ b/controller/api/destination/k8s_resolver.go
@@ -69,15 +69,6 @@ func (k *k8sResolver) streamResolution(host string, port int, listener endpointU
 }
 
 func (k *k8sResolver) streamProfiles(host string, clientNs string, listener profileUpdateListener) error {
-	// In single namespace mode, we'll close the stream immediately and the proxy
-	// will reissue the request after 3 seconds. If we wanted to be more
-	// sophisticated about this in the future, we could leave the stream open
-	// indefinitely, or we could update the API to support a ProfilesDisabled
-	// message. For now, however, this works.
-	if k.profileWatcher == nil {
-		return nil
-	}
-
 	subscriptions := map[profileID]profileUpdateListener{}
 
 	primaryListener, secondaryListener := newFallbackProfileListener(listener)
@@ -131,9 +122,7 @@ func (k *k8sResolver) getState() servicePorts {
 
 func (k *k8sResolver) stop() {
 	k.endpointsWatcher.stop()
-	if k.profileWatcher != nil {
-		k.profileWatcher.stop()
-	}
+	k.profileWatcher.stop()
 }
 
 func (k *k8sResolver) resolveKubernetesService(id *serviceID, port int, listener endpointUpdateListener) error {

@siggy siggy merged commit e5d2460 into master Mar 12, 2019
@siggy siggy deleted the siggy/remove-single-namespace-yesss branch March 12, 2019 07:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants