Skip multicluster-gateways-endpoints for links with no gateways (#1…

…1447) The multicluster extension has always allowed the extension to be installed without a gateway; the idea being that users would provide their own. With p2p, we extended this to allow links that do not specify a gateway at all, but in the process we missed changing a key check -- `multicluster-gateways-endpoints` -- that asserts all links have a probe service. Without a gateway on the other end, a link will not have a probe spec (or a gateway address) so it makes no sense to run this check, there will never be a probe service created in the source cluster. To fix this issue, we skip the check when the link misses either a gateway address or a probe spec. Fixes #11428 Signed-off-by: Matei David <[email protected]> Co-authored-by: Alejandro Pedraza <[email protected]>
linkerd · Oct 18, 2023 · 21046ab · 21046ab
1 parent cd2c88e
commit 21046ab
Show file tree

Hide file tree

Showing 5 changed files with 97 additions and 46 deletions.
diff --git a/multicluster/cmd/check.go b/multicluster/cmd/check.go
@@ -34,8 +34,8 @@ const (
 	// prior to stable-2.10.0 when the linkerd prefix was removed.
 	MulticlusterLegacyExtension = "linkerd-multicluster"
 
-	// linkerdMulticlusterExtensionCheck adds checks related to the multicluster extension
-	linkerdMulticlusterExtensionCheck healthcheck.CategoryID = "linkerd-multicluster"
+	// LinkerdMulticlusterExtensionCheck adds checks related to the multicluster extension
+	LinkerdMulticlusterExtensionCheck healthcheck.CategoryID = "linkerd-multicluster"
 
 	linkerdServiceMirrorServiceAccountName = "linkerd-service-mirror-%s"
 	linkerdServiceMirrorComponentName      = "service-mirror"
@@ -129,7 +129,7 @@ func configureAndRunChecks(wout io.Writer, werr io.Writer, options *checkOptions
 		return fmt.Errorf("Validation error when executing check command: %w", err)
 	}
 	checks := []healthcheck.CategoryID{
-		linkerdMulticlusterExtensionCheck,
+		LinkerdMulticlusterExtensionCheck,
 	}
 	linkerdHC := healthcheck.NewHealthChecker(checks, &healthcheck.Options{
 		ControlPlaneNamespace: controlPlaneNamespace,
@@ -290,7 +290,7 @@ func multiclusterCategory(hc *healthChecker, wait time.Duration) *healthcheck.Ca
 				return healthcheck.CheckIfProxyVersionsMatchWithCLI(pods)
 			}))
 
-	return healthcheck.NewCategory(linkerdMulticlusterExtensionCheck, checkers, true)
+	return healthcheck.NewCategory(LinkerdMulticlusterExtensionCheck, checkers, true)
 }
 
 func (hc *healthChecker) checkLinkCRD(ctx context.Context) error {
@@ -555,6 +555,13 @@ func (hc *healthChecker) checkIfGatewayMirrorsHaveEndpoints(ctx context.Context,
 	links := []string{}
 	errors := []error{}
 	for _, link := range hc.links {
+		// When linked against a cluster without a gateway, there will be no
+		// gateway address and no probe spec initialised. In such cases, skip
+		// the check
+		if link.GatewayAddress == "" || link.ProbeSpec.Path == "" {
+			continue
+		}
+
 		// Check that each gateway probe service has endpoints.
 		selector := metav1.ListOptions{LabelSelector: fmt.Sprintf("%s,%s=%s", k8s.MirroredGatewayLabel, k8s.RemoteClusterNameLabel, link.TargetClusterName)}
 		gatewayMirrors, err := hc.KubeAPIClient().CoreV1().Services(metav1.NamespaceAll).List(ctx, selector)

diff --git a/test/integration/multicluster/install_test.go b/test/integration/multicluster/install_test.go
@@ -14,6 +14,8 @@ import (
 	"testing"
 	"time"
 
+	mcHealthcheck "github.com/linkerd/linkerd2/multicluster/cmd"
+	"github.com/linkerd/linkerd2/pkg/healthcheck"
 	"github.com/linkerd/linkerd2/testutil"
 )
 
@@ -139,8 +141,16 @@ func TestInstall(t *testing.T) {
 }
 
 func TestInstallMulticluster(t *testing.T) {
-	for _, ctx := range contexts {
-		out, err := TestHelper.LinkerdRun("--context="+ctx, "multicluster", "install")
+	for k, ctx := range contexts {
+		var out string
+		var err error
+		// Source context should be installed without a gateway
+		if k == testutil.SourceContextKey {
+			out, err = TestHelper.LinkerdRun("--context="+ctx, "multicluster", "install", "--gateway=false")
+		} else {
+			out, err = TestHelper.LinkerdRun("--context="+ctx, "multicluster", "install")
+		}
+
 		if err != nil {
 			testutil.AnnotatedFatal(t, "'linkerd multicluster install' command failed", err)
 		}
@@ -152,10 +162,8 @@ func TestInstallMulticluster(t *testing.T) {
 		}
 	}
 
-	// Wait for gateways to come up
-	for _, ctx := range contexts {
-		TestHelper.WaitRolloutWithContext(t, testutil.MulticlusterDeployReplicas, ctx)
-	}
+	// Wait for gateways to come up in target cluster
+	TestHelper.WaitRolloutWithContext(t, testutil.MulticlusterDeployReplicas, contexts[testutil.TargetContextKey])
 
 	TestHelper.AddInstalledExtension("multicluster")
 }
@@ -165,63 +173,101 @@ func TestMulticlusterResourcesPostInstall(t *testing.T) {
 		{Namespace: "linkerd-multicluster", Name: "linkerd-gateway"},
 	}
 
-	for _, ctx := range contexts {
-		TestHelper.SwitchContext(ctx)
-		testutil.TestResourcesPostInstall(TestHelper.GetMulticlusterNamespace(), multiclusterSvcs, testutil.MulticlusterDeployReplicas, TestHelper, t)
-	}
+	TestHelper.SwitchContext(contexts[testutil.TargetContextKey])
+	testutil.TestResourcesPostInstall(TestHelper.GetMulticlusterNamespace(), multiclusterSvcs, testutil.MulticlusterDeployReplicas, TestHelper, t)
 }
 
 func TestLinkClusters(t *testing.T) {
-	linkName := "target"
-	// Get gateway IP from target cluster
+	// Get the control plane node IP, this is used to communicate with the
+	// API Server address.
+	// k3s runs an API server on the control plane node, the docker
+	// container IP suffices for a connection between containers to happen
+	// since they run on a shared network.
 	lbCmd := []string{
-		"get", "svc",
-		"-n", "kube-system", "traefik",
-		"-o", "go-template={{ (index .status.loadBalancer.ingress 0).ip }}",
+		"get", "node",
+		"-n", " -l=node-role.kubernetes.io/control-plane=true",
+		"-o", "go-template={{ (index (index .items 0).status.addresses 0).address }}",
 	}
-	lbIP, err := TestHelper.KubectlWithContext("", contexts[testutil.TargetContextKey], lbCmd...)
 
+	// Link target cluster to source
+	// * source cluster should support headless services
+	linkName := "target"
+	lbIP, err := TestHelper.KubectlWithContext("", contexts[testutil.TargetContextKey], lbCmd...)
 	if err != nil {
 		testutil.AnnotatedFatalf(t, "'kubectl get' command failed",
 			"'kubectl get' command failed\n%s", lbIP)
 	}
+
 	linkCmd := []string{
 		"--context=" + contexts[testutil.TargetContextKey],
+		"--cluster-name", linkName,
+		"--api-server-address", fmt.Sprintf("https://%s:6443", lbIP),
+		"--set", "enableHeadlessServices=true",
 		"multicluster", "link",
 		"--log-format", "json",
 		"--log-level", "debug",
-		"--api-server-address", fmt.Sprintf("https://%s:6443", lbIP),
-		"--cluster-name", linkName, "--set", "enableHeadlessServices=true",
 	}
 
-	// Create link in target context
 	out, err := TestHelper.LinkerdRun(linkCmd...)
 	if err != nil {
 		testutil.AnnotatedFatalf(t, "'linkerd multicluster link' command failed", "'linkerd multicluster link' command failed: %s\n%s", out, err)
 	}
 
-	// Apply Link in source
 	out, err = TestHelper.KubectlApplyWithContext(out, contexts[testutil.SourceContextKey], "-f", "-")
 	if err != nil {
 		testutil.AnnotatedFatalf(t, "'kubectl apply' command failed",
 			"'kubectl apply' command failed\n%s", out)
 	}
 
+	// Link source cluster to target
+	// * source cluster does not have a gateway, so the link will reflect that
+	linkName = "source"
+	lbIP, err = TestHelper.KubectlWithContext("", contexts[testutil.SourceContextKey], lbCmd...)
+	if err != nil {
+		testutil.AnnotatedFatalf(t, "'kubectl get' command failed",
+			"'kubectl get' command failed\n%s", lbIP)
+	}
+
+	linkCmd = []string{
+		"--context=" + contexts[testutil.SourceContextKey],
+		"--cluster-name", linkName, "--gateway=false",
+		"--api-server-address", fmt.Sprintf("https://%s:6443", lbIP),
+		"multicluster", "link",
+		"--log-format", "json",
+		"--log-level", "debug",
+	}
+
+	out, err = TestHelper.LinkerdRun(linkCmd...)
+	if err != nil {
+		testutil.AnnotatedFatalf(t, "'linkerd multicluster link' command failed", "'linkerd multicluster link' command failed: %s\n%s", out, err)
+	}
+
+	out, err = TestHelper.KubectlApplyWithContext(out, contexts[testutil.TargetContextKey], "-f", "-")
+	if err != nil {
+		testutil.AnnotatedFatalf(t, "'kubectl apply' command failed",
+			"'kubectl apply' command failed\n%s", out)
+	}
+
 }
 
 func TestCheckMulticluster(t *testing.T) {
-	// Check resources after link were created successfully in source cluster
-	ctx := contexts[testutil.SourceContextKey]
+	// Run `linkerd check` for both clusters, expect multicluster checks to be
+	// run and pass successfully
+	for _, ctx := range contexts {
+		// First, switch context to make sure we check pods in the cluster we're
+		// supposed to be checking them in. This will rebuild the clientset
+		if err := TestHelper.SwitchContext(ctx); err != nil {
+			testutil.AnnotatedFatalf(t, "failed to rebuild helper clientset with new context", "failed to rebuild helper clientset with new context [%s]: %v", ctx, err)
+		}
 
-	// First, switch context to make sure we check pods in the cluster we're
-	// supposed to be checking them in. This will rebuild the clientset
-	if err := TestHelper.SwitchContext(ctx); err != nil {
-		testutil.AnnotatedFatalf(t, "failed to rebuild helper clientset with new context", "failed to rebuild helper clientset with new context [%s]: %v", ctx, err)
-	}
-	if err := TestHelper.TestCheckMc("--context", ctx); err != nil {
-		t.Fatalf("'linkerd check' command failed: %s", err)
+		err := TestHelper.TestCheckWith([]healthcheck.CategoryID{mcHealthcheck.LinkerdMulticlusterExtensionCheck}, "--context", ctx)
+		if err != nil {
+			t.Fatalf("'linkerd check' command failed: %s", err)
+		}
 	}
 
+	// Check resources after link were created successfully in source cluster (e.g.
+	// secrets)
 	t.Run("Outputs resources that allow service-mirror controllers to connect to target cluster", func(t *testing.T) {
 		if err := TestHelper.SwitchContext(contexts[testutil.TargetContextKey]); err != nil {
 			testutil.AnnotatedFatalf(t,

diff --git a/test/integration/multicluster/multicluster-traffic/mc_traffic_test.go b/test/integration/multicluster/multicluster-traffic/mc_traffic_test.go
@@ -9,6 +9,8 @@ import (
 	"testing"
 	"time"
 
+	mcHealthcheck "github.com/linkerd/linkerd2/multicluster/cmd"
+	"github.com/linkerd/linkerd2/pkg/healthcheck"
 	"github.com/linkerd/linkerd2/pkg/k8s"
 	"github.com/linkerd/linkerd2/testutil"
 	"github.com/linkerd/linkerd2/testutil/prommatch"
@@ -135,7 +137,8 @@ func TestCheckGatewayAfterRepairEndpoints(t *testing.T) {
 			contexts[testutil.SourceContextKey], err)
 	}
 	time.Sleep(time.Minute + 5*time.Second)
-	if err := TestHelper.TestCheckMc("--context", contexts[testutil.SourceContextKey]); err != nil {
+	err := TestHelper.TestCheckWith([]healthcheck.CategoryID{mcHealthcheck.LinkerdMulticlusterExtensionCheck}, "--context", contexts[testutil.SourceContextKey])
+	if err != nil {
 		t.Fatalf("'linkerd check' command failed: %s", err)
 	}
 }

diff --git a/testutil/test_helper.go b/testutil/test_helper.go
@@ -99,11 +99,10 @@ var MulticlusterDeployReplicas = map[string]DeploySpec{
 }
 
 // MulticlusterSourceReplicas is a map containing the number of replicas for the
-// Gateway and Service Mirror component; components that we'd only expect in the
+// Service Mirror component; component that we'd only expect in the
 // source cluster.
 var MulticlusterSourceReplicas = map[string]DeploySpec{
 	"linkerd-service-mirror-target": {Namespace: "linkerd-multicluster", Replicas: 1},
-	"linkerd-gateway":               {Namespace: "linkerd-multicluster", Replicas: 1},
 }
 
 // ExternalVizDeployReplicas has an external prometheus instance that's in a

diff --git a/testutil/test_helper_check.go b/testutil/test_helper_check.go
@@ -44,20 +44,16 @@ func (h *TestHelper) TestCheckPre() error {
 
 // TestCheck runs validates the output of `linkerd check`
 func (h *TestHelper) TestCheck(extraArgs ...string) error {
-	cmd := []string{"check", "--output", "json", "--wait", "5m"}
-	cmd = append(cmd, extraArgs...)
-	categories := append(coreCategories, healthcheck.LinkerdControlPlaneVersionChecks,
-		vizHealthcheck.LinkerdVizExtensionCheck)
-	return h.testCheck(cmd, categories)
+	return h.TestCheckWith([]healthcheck.CategoryID{healthcheck.LinkerdControlPlaneVersionChecks, vizHealthcheck.LinkerdVizExtensionCheck}, extraArgs...)
 }
 
-// TestCheckMc validates the output of `linkerd check` and `linkerd mc check`
-func (h *TestHelper) TestCheckMc(extraArgs ...string) error {
-	// TODO (matei): expose multicluster healthchecks in a public library so
-	// they can be consumed here, similar to what viz is doing.
+// TestCheckWith validates the output of `linkerd check`. It will validate the
+// core categories and any additional categories that the caller provides.
+func (h *TestHelper) TestCheckWith(additional []healthcheck.CategoryID, extraArgs ...string) error {
 	cmd := []string{"check", "--output", "json", "--wait", "5m"}
 	cmd = append(cmd, extraArgs...)
-	return h.testCheck(cmd, coreCategories)
+	categories := append(coreCategories, additional...)
+	return h.testCheck(cmd, categories)
 }
 
 // TestCheckProxy runs validates the output of `linkerd check --proxy`