Repository: linkerd/linkerd2
Pull request #8423 (2022-05-04 15:04:56)

refactor: Split CRD & Control Plane upgrade logic
This change follows on 4f3c374, which split the install logic for CRDs
and the core control plane, by splitting the upgrade logic for the CRDs
and the core control plane.

Signed-off-by: Oliver Gould <ver@buoyant.io>

<!--  Thanks for sending a pull request!

If you already have a well-structured git commit message, chances are GitHub
set the title and description of this PR to the git commit message subject and
body, respectively. If so, you may delete these instructions and submit your PR.

If this is your first time, please read our contributor guide:
https://github.com/linkerd/linkerd2/blob/main/CONTRIBUTING.md

The title and description of your Pull Request should match the git commit
subject and body, respectively. Git commit messages are structured as follows:

```
Subject

Problem

Solution

Validation

Fixes #[GitHub issue ID]

DCO Sign off
```

Example git commit message:

```
Introduce Pull Request Template

GitHub's community guidelines recommend a pull request template, the repo was
lacking one.

Introduce a `PULL_REQUEST_TEMPLATE.md` file.

Once merged, the
[Community profile checklist](https://github.com/linkerd/linkerd2/community)
should indicate the repo now provides a pull request template.

Fixes #3321

Signed-off-by: Jane Smith <jane.smith@example.com>
```

Note the git commit message subject becomes the pull request title.

For more details around git commits, see the section on Committing in our
contributor guide:
https://github.com/linkerd/linkerd2/blob/main/CONTRIBUTING.md#committing
-->



Patch:
diff --git a/Cargo.lock b/Cargo.lock
index 2c93cd214..74638807e 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -1159,9 +1159,9 @@ dependencies = [
 
 [[package]]
 name = "num-traits"
-version = "0.2.15"
+version = "0.2.14"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "578ede34cf02f8924ab9447f50c28075b4d3e5b269972345e7e0372b38c6cdcd"
+checksum = "9a64b1ec5cda2586e284722486d802acf1f7dbdc623e2bfc57e65ca1cd099290"
 dependencies = [
  "autocfg",
 ]
@@ -1671,9 +1671,9 @@ dependencies = [
 
 [[package]]
 name = "serde_json"
-version = "1.0.81"
+version = "1.0.80"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "9b7ce2b32a1aed03c558dc61a5cd328f15aff2dbc17daad8fb8af04d2100e15c"
+checksum = "f972498cf015f7c0746cac89ebe1d6ef10c293b94175a243a2d9442c163d9944"
 dependencies = [
  "indexmap",
  "itoa",
@@ -1683,9 +1683,9 @@ dependencies = [
 
 [[package]]
 name = "serde_yaml"
-version = "0.8.24"
+version = "0.8.23"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "707d15895415db6628332b737c838b88c598522e4dc70647e59b72312924aebc"
+checksum = "a4a521f2940385c165a24ee286aa8599633d162077a54bdcae2a6fd5a7bfa7a0"
 dependencies = [
  "indexmap",
  "ryu",
diff --git a/cli/cmd/install.go b/cli/cmd/install.go
index 796869700..7ec0e4a30 100644
--- a/cli/cmd/install.go
+++ b/cli/cmd/install.go
@@ -97,11 +97,11 @@ func newCmdInstall() *cobra.Command {
 
 This command provides all Kubernetes configs necessary to install the Linkerd
 control plane.`,
-		Example: `  # Default install.
-  linkerd install | kubectl apply -f -
+		Example: `  # Install CRDs first.
+  linkerd install --crds | kubectl apply -f -
 
-  # Install Linkerd into a non-default namespace.
-  linkerd install -L linkerdtest | kubectl apply -f -
+  # Install the core control plane.
+  linkerd install | kubectl apply -f -
 
 The installation can be configured by using the --set, --values, --set-string and --set-file flags.
 A full list of configurable values can be found at https://www.github.com/linkerd/linkerd2/tree/main/charts/linkerd2/README.md`,
@@ -130,7 +130,9 @@ A full list of configurable values can be found at https://www.github.com/linker
 			}
 
 			if crds {
-				if err = installCRDs(cmd.Context(), k8sAPI, os.Stdout, values, options); err != nil {
+				// The CRD chart is not configurable.
+				// TODO(ver): Error if values have been configured?
+				if err = installCRDs(cmd.Context(), k8sAPI, os.Stdout); err != nil {
 					return err
 				}
 
@@ -175,18 +177,12 @@ func checkNoConfig(ctx context.Context, k8sAPI *k8s.KubernetesAPI) error {
 	return nil
 }
 
-func installCRDs(ctx context.Context, k8sAPI *k8s.KubernetesAPI, w io.Writer, values *l5dcharts.Values, options valuespkg.Options) error {
+func installCRDs(ctx context.Context, k8sAPI *k8s.KubernetesAPI, w io.Writer) error {
 	if err := checkNoConfig(ctx, k8sAPI); err != nil {
 		return err
 	}
 
-	// Create values override
-	valuesOverrides, err := options.MergeValues(nil)
-	if err != nil {
-		return err
-	}
-
-	return renderCRDs(w, values, valuesOverrides)
+	return renderCRDs(w)
 }
 
 func installControlPlane(ctx context.Context, k8sAPI *k8s.KubernetesAPI, w io.Writer, values *l5dcharts.Values, flags []flag.Flag, options valuespkg.Options) error {
@@ -303,7 +299,7 @@ func renderChartToBuffer(files []*loader.BufferedFile, values *l5dcharts.Values,
 	return &buf, vals, nil
 }
 
-func renderCRDs(w io.Writer, values *l5dcharts.Values, valuesOverrides map[string]interface{}) error {
+func renderCRDs(w io.Writer) error {
 	files := []*loader.BufferedFile{
 		{Name: chartutil.ChartfileName},
 	}
@@ -314,7 +310,12 @@ func renderCRDs(w io.Writer, values *l5dcharts.Values, valuesOverrides map[strin
 		return err
 	}
 
-	buf, _, err := renderChartToBuffer(files, values, valuesOverrides)
+	// The CRD chart does not take any value configuration.
+	values, err := l5dcharts.NewValues()
+	if err != nil {
+		return err
+	}
+	buf, _, err := renderChartToBuffer(files, values, map[string]interface{}{})
 	if err != nil {
 		return err
 	}
diff --git a/cli/cmd/upgrade.go b/cli/cmd/upgrade.go
index 639ba1fed..1fb014d18 100644
--- a/cli/cmd/upgrade.go
+++ b/cli/cmd/upgrade.go
@@ -91,12 +91,21 @@ A full list of configurable values can be found at https://www.github.com/linker
   --prune-whitelist=rbac.authorization.k8s.io/v1/clusterrolebinding \
   --prune-whitelist=apiregistration.k8s.io/v1/apiservice -f -`,
 		RunE: func(cmd *cobra.Command, args []string) error {
+			if crds {
+				// The CRD chart is not configurable.
+				// TODO(ver): Error if values have been configured?
+				if _, err := upgradeCRDs().WriteTo(os.Stdout); err != nil {
+					fmt.Fprintln(os.Stderr, err.Error())
+					os.Exit(1)
+				}
+				return nil
+			}
+
 			k, err := k8sClient(manifests)
 			if err != nil {
 				return err
 			}
-			err = upgradeRunE(cmd.Context(), k, flags, crds, options)
-			if err != nil {
+			if err = upgradeControlPlaneRunE(cmd.Context(), k, flags, options); err != nil {
 				fmt.Fprintln(os.Stderr, err.Error())
 				os.Exit(1)
 			}
@@ -154,9 +163,8 @@ func makeUpgradeFlags() *pflag.FlagSet {
 	return upgradeFlags
 }
 
-func upgradeRunE(ctx context.Context, k *k8s.KubernetesAPI, flags []flag.Flag, crds bool, options valuespkg.Options) error {
-
-	buf, err := upgrade(ctx, k, flags, crds, options)
+func upgradeControlPlaneRunE(ctx context.Context, k *k8s.KubernetesAPI, flags []flag.Flag, options valuespkg.Options) error {
+	buf, err := upgradeControlPlane(ctx, k, flags, options)
 	if err != nil {
 		return err
 	}
@@ -167,22 +175,29 @@ func upgradeRunE(ctx context.Context, k *k8s.KubernetesAPI, flags []flag.Flag, c
 		}
 	}
 
-	buf.WriteTo(os.Stdout)
+	_, err = buf.WriteTo(os.Stdout)
+	return err
+}
 
-	return nil
+func upgradeCRDs() *bytes.Buffer {
+	var buf bytes.Buffer
+	if err := renderCRDs(&buf); err != nil {
+		upgradeErrorf("Could not render upgrade configuration: %s", err)
+	}
+	return &buf
 }
 
-func upgrade(ctx context.Context, k *k8s.KubernetesAPI, flags []flag.Flag, crds bool, options valuespkg.Options) (bytes.Buffer, error) {
+func upgradeControlPlane(ctx context.Context, k *k8s.KubernetesAPI, flags []flag.Flag, options valuespkg.Options) (*bytes.Buffer, error) {
 	values, err := loadStoredValues(ctx, k)
 	if err != nil {
-		return bytes.Buffer{}, fmt.Errorf("failed to load stored values: %w", err)
+		return nil, fmt.Errorf("failed to load stored values: %w", err)
 	}
 
 	// If values is still nil, then the linkerd-config-overrides secret was not found.
 	// This means either means that Linkerd was installed with Helm or that the installation
 	// needs to be repaired.
 	if values == nil {
-		return bytes.Buffer{}, errors.New(
+		return nil, errors.New(
 			`Could not find the Linkerd config. If Linkerd was installed with Helm, please
 use Helm to perform upgrades. If Linkerd was not installed with Helm, please use
 the 'linkerd repair' command to repair the Linkerd config`)
@@ -190,54 +205,45 @@ the 'linkerd repair' command to repair the Linkerd config`)
 
 	err = flag.ApplySetFlags(values, flags)
 	if err != nil {
-		return bytes.Buffer{}, err
+		return nil, err
 	}
 
 	if values.Identity.Issuer.Scheme == string(corev1.SecretTypeTLS) {
 		for _, flag := range flags {
 			if (flag.Name() == "identity-issuer-certificate-file" || flag.Name() == "identity-issuer-key-file") && flag.IsSet() {
-				return bytes.Buffer{}, errors.New("cannot update issuer certificates if you are using external cert management solution")
+				return nil, errors.New("cannot update issuer certificates if you are using external cert management solution")
 			}
 		}
 	}
 
 	err = validateValues(ctx, k, values)
 	if err != nil {
-		return bytes.Buffer{}, err
+		return nil, err
 	}
 	if !force && values.Identity.Issuer.Scheme == k8s.IdentityIssuerSchemeLinkerd {
 		err = ensureIssuerCertWorksWithAllProxies(ctx, k, values)
 		if err != nil {
-			return bytes.Buffer{}, err
+			return nil, err
 		}
 	}
 
 	// Create values override
 	valuesOverrides, err := options.MergeValues(nil)
 	if err != nil {
-		return bytes.Buffer{}, err
+		return nil, err
 	}
 	if !isRunAsRoot(valuesOverrides) {
 		err = healthcheck.CheckNodesHaveNonDockerRuntime(ctx, k)
 		if err != nil {
-			return bytes.Buffer{}, err
+			return nil, err
 		}
 	}
 
-	// rendering to a buffer and printing full contents of buffer after
-	// render is complete, to ensure that okStatus prints separately
 	var buf bytes.Buffer
-	if crds {
-		if err = renderCRDs(&buf, values, valuesOverrides); err != nil {
-			upgradeErrorf("Could not render upgrade configuration: %s", err)
-		}
-		return buf, nil
-	}
-
 	if err = renderControlPlane(&buf, values, valuesOverrides); err != nil {
 		upgradeErrorf("Could not render upgrade configuration: %s", err)
 	}
-	return buf, nil
+	return &buf, nil
 }
 
 func loadStoredValues(ctx context.Context, k *k8s.KubernetesAPI) (*charts.Values, error) {


Test patch:
diff --git a/cli/cmd/install_test.go b/cli/cmd/install_test.go
index 3e4c4330b..65e23142b 100644
--- a/cli/cmd/install_test.go
+++ b/cli/cmd/install_test.go
@@ -265,7 +265,7 @@ func TestRenderCRDs(t *testing.T) {
 	addFakeTLSSecrets(defaultValues)
 
 	var buf bytes.Buffer
-	if err := renderCRDs(&buf, defaultValues, map[string]interface{}{}); err != nil {
+	if err := renderCRDs(&buf); err != nil {
 		t.Fatalf("Failed to render templates: %v", err)
 	}
 	testDataDiffer.DiffTestdata(t, "install_crds.golden", buf.String())
diff --git a/cli/cmd/upgrade_test.go b/cli/cmd/upgrade_test.go
index 9a5967098..c9c41872c 100644
--- a/cli/cmd/upgrade_test.go
+++ b/cli/cmd/upgrade_test.go
@@ -564,27 +564,37 @@ func pathMatch(path []string, template []string) bool {
 	return true
 }
 
-func renderInstall(t *testing.T, values *linkerd2.Values) bytes.Buffer {
-	var installBuf bytes.Buffer
-	if err := renderControlPlane(&installBuf, values, nil); err != nil {
+func renderInstall(t *testing.T, values *linkerd2.Values) *bytes.Buffer {
+	var buf bytes.Buffer
+	if err := renderCRDs(&buf); err != nil {
 		t.Fatalf("could not render install manifests: %s", err)
 	}
-	return installBuf
+	buf.WriteString("---\n")
+	if err := renderControlPlane(&buf, values, nil); err != nil {
+		t.Fatalf("could not render install manifests: %s", err)
+	}
+	return &buf
 }
 
-func renderUpgrade(installManifest string, upgradeOpts []flag.Flag, templateOpts valuespkg.Options) (bytes.Buffer, error) {
+func renderUpgrade(installManifest string, upgradeOpts []flag.Flag, templateOpts valuespkg.Options) (*bytes.Buffer, error) {
 	k, err := k8s.NewFakeAPIFromManifests([]io.Reader{strings.NewReader(installManifest)})
 	if err != nil {
-		return bytes.Buffer{}, fmt.Errorf("failed to initialize fake API: %w\n\n%s", err, installManifest)
+		return nil, fmt.Errorf("failed to initialize fake API: %w\n\n%s", err, installManifest)
 	}
-	return upgrade(context.Background(), k, upgradeOpts, false, templateOpts)
+	buf := upgradeCRDs()
+	buf.WriteString("---\n")
+	cpbuf, err := upgradeControlPlane(context.Background(), k, upgradeOpts, templateOpts)
+	if err != nil {
+		return nil, err
+	}
+	buf.Write(cpbuf.Bytes())
+	return buf, nil
 }
 
-func renderInstallAndUpgrade(t *testing.T, installOpts *charts.Values, upgradeOpts []flag.Flag, templateOpts valuespkg.Options) (bytes.Buffer, bytes.Buffer, error) {
-
+func renderInstallAndUpgrade(t *testing.T, installOpts *charts.Values, upgradeOpts []flag.Flag, templateOpts valuespkg.Options) (*bytes.Buffer, *bytes.Buffer, error) {
 	err := validateValues(context.Background(), nil, installOpts)
 	if err != nil {
-		return bytes.Buffer{}, bytes.Buffer{}, fmt.Errorf("failed to validate values: %w", err)
+		return nil, nil, fmt.Errorf("failed to validate values: %w", err)
 	}
 	installBuf := renderInstall(t, installOpts)
 	upgradeBuf, err := renderUpgrade(installBuf.String(), upgradeOpts, templateOpts)
diff --git a/test/integration/deep/opaqueports/assertions.go b/test/integration/deep/opaqueports/assertions.go
deleted file mode 100644
index 0f55c9e16..000000000
--- a/test/integration/deep/opaqueports/assertions.go
+++ /dev/null
@@ -1,75 +0,0 @@
-package opaqueports
-
-import (
-	"fmt"
-	"regexp"
-)
-
-var (
-	tcpMetricRE = regexp.MustCompile(
-		`tcp_open_total\{direction="inbound",peer="src",target_addr="[0-9\.]+:[0-9]+",target_ip="[0-9\.]+",target_port="[0-9]+",tls="true",client_id="default\.linkerd-opaque-ports[\-a-z]+-test\.serviceaccount\.identity\.linkerd\.cluster\.local",srv_kind="default",srv_name="all-unauthenticated".*} [0-9]+`,
-	)
-	tcpMetricOutUnmeshedAuthorityRE = regexp.MustCompile(
-		`tcp_open_total\{direction="outbound",peer="dst",authority="[a-zA-Z\-]+\.[a-zA-Z\-]+\.svc\.cluster\.local:[0-9]+",target_addr="[0-9\.]+:[0-9]+",target_ip="[0-9\.]+",target_port="[0-9]+",tls="no_identity",no_tls_reason="not_provided_by_service_discovery",.*\} [0-9]+`,
-	)
-	tcpMetricOutUnmeshedNoAuthorityRE = regexp.MustCompile(
-		`tcp_open_total\{direction="outbound",peer="dst",target_addr="[0-9\.]+:[0-9]+",target_ip="[0-9\.]+",target_port="[0-9]+",tls="no_identity",no_tls_reason="not_provided_by_service_discovery",.*\} [0-9]+`,
-	)
-	httpRequestTotalMetricRE = regexp.MustCompile(
-		`request_total\{direction="outbound",authority="[a-zA-Z\-]+\.[a-zA-Z\-]+\.svc\.cluster\.local:8080",target_addr="[0-9\.]+:8080",target_ip="[0-9\.]+",target_port="8080",tls="true",.*`,
-	)
-	httpRequestTotalUnmeshedRE = regexp.MustCompile(
-		`request_total\{direction="outbound",authority="[a-zA-Z\-]+\.[a-zA-Z\-]+\.svc\.cluster\.local:8080",target_addr="[0-9\.]+:8080",target_ip="[0-9\.]+",target_port="8080",tls="no_identity",.*`,
-	)
-	tcpMetricOutboundMeshedNoAuthorityRE = regexp.MustCompile(
-		`tcp_open_total\{direction="outbound",peer="dst",target_addr="[0-9\.]+:8080",target_ip="[0-9\.]+",target_port="8080",tls="true",server_id="default.linkerd-opaque-ports[\-a-z]+\-test.serviceaccount.identity.linkerd.cluster.local",dst_control_plane_ns="linkerd".*`,
-	)
-	tcpMetricOutboundMeshedAuthorityRE = regexp.MustCompile(
-		`tcp_open_total\{direction="outbound",peer="dst",authority="[a-zA-Z\-]+\.linkerd-opaque-ports[\-a-z]+\-test\.svc.cluster\.local:8080",target_addr="[0-9\.]+:8080",target_ip="[0-9\.]+",target_port="8080",tls="true".*`,
-	)
-)
-
-func hasNoOutbondHTTPRequest(metrics string) error {
-	if httpRequestTotalMetricRE.MatchString(metrics) {
-		return fmt.Errorf("expected not to find HTTP outbound requests when pod is opaque\n%s", metrics)
-	}
-	if httpRequestTotalUnmeshedRE.MatchString(metrics) {
-		return fmt.Errorf("expected not to find HTTP outbound requests when pod is opaque\n%s", metrics)
-	}
-	return nil
-}
-
-func hasInboundTCPTraffic(metrics string) error {
-	if !tcpMetricRE.MatchString(metrics) {
-		return fmt.Errorf("failed to find expected metric for inbound TLS TCP traffic\n%s", metrics)
-	}
-	return nil
-}
-
-func hasOutboundTCPWithAuthorityAndNoTLS(metrics string) error {
-	if !tcpMetricOutUnmeshedAuthorityRE.MatchString(metrics) {
-		return fmt.Errorf("failed to find expected metric for outbound non-TLS TCP traffic\n%s", metrics)
-	}
-	return nil
-}
-
-func hasOutboundTCPWithNoTLSAndNoAuthority(metrics string) error {
-	if !tcpMetricOutUnmeshedNoAuthorityRE.MatchString(metrics) {
-		return fmt.Errorf("failed to find expected metric for outbound non-TLS TCP traffic\n%s", metrics)
-	}
-	return nil
-}
-
-func hasOutboundTCPWithTLSAndAuthority(metrics string) error {
-	if !tcpMetricOutboundMeshedAuthorityRE.MatchString(metrics) {
-		return fmt.Errorf("failed to find expected metric for outbound TLS TCP traffic\n%s", metrics)
-	}
-	return nil
-}
-
-func hasOutboundTCPWithTLSAndNoAuthority(metrics string) error {
-	if !tcpMetricOutboundMeshedNoAuthorityRE.MatchString(metrics) {
-		return fmt.Errorf("failed to find expected metric for outbound TLS TCP traffic\n%s", metrics)
-	}
-	return nil
-}
diff --git a/test/integration/deep/opaqueports/opaque_ports_test.go b/test/integration/deep/opaqueports/opaque_ports_test.go
index 0bd981685..a519d8b2f 100644
--- a/test/integration/deep/opaqueports/opaque_ports_test.go
+++ b/test/integration/deep/opaqueports/opaque_ports_test.go
@@ -1,11 +1,10 @@
 package opaqueports
 
 import (
-	"bytes"
 	"context"
 	"fmt"
-	"html/template"
 	"os"
+	"regexp"
 	"testing"
 	"time"
 
@@ -15,30 +14,27 @@ import (
 
 var TestHelper *testutil.TestHelper
 
-var opaquePortsClientTemplate = template.Must(template.New("opaque_ports_client.yaml").ParseFiles("testdata/opaque_ports_client.yaml"))
-
 var (
 	opaquePodApp         = "opaque-pod"
 	opaquePodSC          = "slow-cooker-opaque-pod"
 	opaqueSvcApp         = "opaque-service"
 	opaqueSvcSC          = "slow-cooker-opaque-service"
-	opaqueUnmeshedSvcApp = "opaque-unmeshed"
 	opaqueUnmeshedSvcPod = "opaque-unmeshed-svc"
 	opaqueUnmeshedSvcSC  = "slow-cooker-opaque-unmeshed-svc"
+	tcpMetricRE          = regexp.MustCompile(
+		`tcp_open_total\{direction="inbound",peer="src",target_addr="[0-9\.]+:[0-9]+",target_ip="[0-9\.]+",target_port="[0-9]+",tls="true",client_id="default\.linkerd-opaque-ports-test\.serviceaccount\.identity\.linkerd\.cluster\.local",srv_kind="default",srv_name="all-unauthenticated"} [0-9]+`,
+	)
+	tcpMetricOutUnmeshedRE = regexp.MustCompile(
+		`tcp_open_total\{direction="outbound",peer="dst",authority="[a-zA-Z\-]+\.[a-zA-Z\-]+\.svc\.cluster\.local:[0-9]+",target_addr="[0-9\.]+:[0-9]+",target_ip="[0-9\.]+",target_port="[0-9]+",tls="no_identity",no_tls_reason="not_provided_by_service_discovery",.*\} [0-9]+`,
+	)
+	httpRequestTotalMetricRE = regexp.MustCompile(
+		`request_total\{direction="outbound",authority="[a-zA-Z\-]+\.[a-zA-Z\-]+\.svc\.cluster\.local:8080",target_addr="[0-9\.]+:8080",target_ip="[0-9\.]+",target_port="8080",tls="true",.*`,
+	)
+	httpRequestTotalUnmeshedRE = regexp.MustCompile(
+		`request_total\{direction="outbound",authority="svc-opaque-unmeshed\.linkerd-opaque-ports-test\.svc\.cluster\.local:8080",target_addr="[0-9\.]+:8080",target_ip="[0-9\.]+",target_port="8080",tls="no_identity",.*`,
+	)
 )
 
-type testCase struct {
-	name      string
-	appName   string
-	appChecks []check
-	scName    string
-	scChecks  []check
-}
-
-type check func(metrics string) error
-
-func checks(c ...check) []check { return c }
-
 func TestMain(m *testing.M) {
 	TestHelper = testutil.NewTestHelper()
 	// Block test execution until control plane is running
@@ -46,228 +42,141 @@ func TestMain(m *testing.M) {
 	os.Exit(m.Run())
 }
 
-// clientTemplateArgs is a struct that contains the arguments to be supplied
-// to the deployment template opaque_ports_client.yaml.
-type clientTemplateArgs struct {
-	ServiceCookerOpaqueServiceTargetHost     string
-	ServiceCookerOpaquePodTargetHost         string
-	ServiceCookerOpaqueUnmeshedSVCTargetHost string
-}
-
-func serviceName(n string) string {
-	return fmt.Sprintf("svc-%s", n)
-}
-
 //////////////////////
 /// TEST EXECUTION ///
 //////////////////////
 
-func TestOpaquePortsCalledByServiceTarget(t *testing.T) {
-	ctx := context.Background()
-	TestHelper.WithDataPlaneNamespace(ctx, "opaque-ports-called-by-service-name-test", map[string]string{}, t, func(t *testing.T, opaquePortsNs string) {
-		checks := func(c ...check) []check { return c }
-
-		if err := deployApplications(opaquePortsNs); err != nil {
-			testutil.AnnotatedFatal(t, "failed to deploy applications", err)
-		}
-		waitForAppDeploymentReady(t, opaquePortsNs)
-
-		tmplArgs := clientTemplateArgs{
-			ServiceCookerOpaqueServiceTargetHost:     serviceName(opaqueSvcApp),
-			ServiceCookerOpaquePodTargetHost:         serviceName(opaquePodApp),
-			ServiceCookerOpaqueUnmeshedSVCTargetHost: serviceName(opaqueUnmeshedSvcApp),
-		}
-		if err := deployClients(opaquePortsNs, tmplArgs); err != nil {
-			testutil.AnnotatedFatal(t, "failed to deploy client pods", err)
-		}
+func TestOpaquePorts(t *testing.T) {
 
-		runTests(ctx, t, opaquePortsNs, []testCase{
-			{
-				name:   "calling a meshed service when opaque annotation is on receiving pod",
-				scName: opaquePodSC,
-				scChecks: checks(
-					hasNoOutbondHTTPRequest,
-					hasOutboundTCPWithTLSAndNoAuthority,
-				),
-				appName:   opaquePodApp,
-				appChecks: checks(hasInboundTCPTraffic),
-			},
-			{
-				name:   "calling a meshed service when opaque annotation is on calling pod",
-				scName: opaqueSvcSC,
-				scChecks: checks(
-					hasNoOutbondHTTPRequest,
-					hasOutboundTCPWithTLSAndAuthority,
-				),
-				appName:   opaqueSvcApp,
-				appChecks: checks(hasInboundTCPTraffic),
-			},
-			{
-				name:   "calling an unmeshed service",
-				scName: opaqueUnmeshedSvcSC,
-				scChecks: checks(
-					hasNoOutbondHTTPRequest,
-					hasOutboundTCPWithAuthorityAndNoTLS,
-				),
-			},
-		})
-	})
-}
-
-func TestOpaquePortsCalledByPodTarget(t *testing.T) {
 	ctx := context.Background()
-	TestHelper.WithDataPlaneNamespace(ctx, "opaque-ports-called-by-pod-ip-test", map[string]string{}, t, func(t *testing.T, opaquePortsNs string) {
+	TestHelper.WithDataPlaneNamespace(ctx, "opaque-ports-test", map[string]string{}, t, func(t *testing.T, opaquePortsNs string) {
+		out, err := TestHelper.Kubectl("", "apply", "-f", "testdata/opaque_ports_application.yaml", "-n", opaquePortsNs)
+		if err != nil {
+			testutil.AnnotatedFatalf(t, "'kubectl apply' command failed",
+				"'kubectl apply' command failed\n%s", out)
+		}
 
-		if err := deployApplications(opaquePortsNs); err != nil {
-			testutil.AnnotatedFatal(t, "failed to deploy applications", err)
+		// Check that the server pods are started correctly before continuing
+		// with the rest of the test.
+		for _, deploy := range []string{opaquePodApp, opaqueSvcApp, opaqueUnmeshedSvcPod} {
+			if err := TestHelper.CheckPods(ctx, opaquePortsNs, deploy, 1); err != nil {
+				//nolint:errorlint
+				if rce, ok := err.(*testutil.RestartCountError); ok {
+					testutil.AnnotatedWarn(t, "CheckPods timed-out", rce)
+				} else {
+					testutil.AnnotatedError(t, "CheckPods timed-out", err)
+				}
+			}
 		}
-		waitForAppDeploymentReady(t, opaquePortsNs)
 
-		tmplArgs, err := templateArgsPodIP(ctx, opaquePortsNs)
+		// Deploy slow-cooker clients
+		out, err = TestHelper.Kubectl("", "apply", "-f", "testdata/opaque_ports_client.yaml", "-n", opaquePortsNs)
 		if err != nil {
-			testutil.AnnotatedFatal(t, "failed to fetch pod IPs", err)
+			testutil.AnnotatedFatalf(t, "'kubectl apply' command failed",
+				"'kubectl apply' command failed\n%s", out)
 		}
 
-		if err := deployClients(opaquePortsNs, tmplArgs); err != nil {
-			testutil.AnnotatedFatal(t, "failed to deploy client pods", err)
-		}
-		runTests(ctx, t, opaquePortsNs, []testCase{
-			{
-				name:   "calling a meshed service when opaque annotation is on receiving pod",
-				scName: opaquePodSC,
-				scChecks: checks(
-					hasNoOutbondHTTPRequest,
-					hasOutboundTCPWithTLSAndNoAuthority,
-				),
-				appName:   opaquePodApp,
-				appChecks: checks(hasInboundTCPTraffic),
-			},
-			{
-				name:   "calling a meshed service when opaque annotation is on calling pod",
-				scName: opaqueSvcSC,
-				scChecks: checks(
-					hasNoOutbondHTTPRequest,
-					// No authority here, because we are calling the pod directly.
-					hasOutboundTCPWithTLSAndNoAuthority,
-				),
-				appName:   opaqueSvcApp,
-				appChecks: checks(hasInboundTCPTraffic),
-			},
-			{
-				name:   "calling an unmeshed service",
-				scName: opaqueUnmeshedSvcSC,
-				scChecks: checks(
-					hasNoOutbondHTTPRequest,
-					// No authority here, because we are calling the pod directly.
-					hasOutboundTCPWithNoTLSAndNoAuthority,
-				),
-			},
-		})
-	})
-}
+		// Use a short time window for these tests to get rid of transient errors
+		// associated with slow cooker warm-up and initialization.
+		t.Run("expect absent HTTP outbound requests for opaque-pod slow cooker", func(t *testing.T) {
+			// Check the slow cooker metrics
+			err := TestHelper.RetryFor(30*time.Second, func() error {
+				pods, err := TestHelper.GetPods(ctx, opaquePortsNs, map[string]string{"app": opaquePodSC})
+				if err != nil || len(pods) == 0 {
+					return fmt.Errorf("error getting pods\n%w", err)
+				}
+				metrics, err := getPodMetrics(pods[0], opaquePortsNs)
+				if err != nil {
+					return fmt.Errorf("error getting metrics for pod\n%w", err)
+				}
+				if httpRequestTotalMetricRE.MatchString(metrics) {
+					return fmt.Errorf("expected not to find HTTP outbound requests when pod is opaque\n%s", metrics)
+				}
+				// Check the application metrics
+				pods, err = TestHelper.GetPods(ctx, opaquePortsNs, map[string]string{"app": opaquePodApp})
+				if err != nil {
+					return fmt.Errorf("error getting pods\n%w", err)
+				}
+				metrics, err = getPodMetrics(pods[0], opaquePortsNs)
+				if err != nil {
+					return fmt.Errorf("error getting metrics for pod\n%w", err)
+				}
+				if !tcpMetricRE.MatchString(metrics) {
+					return fmt.Errorf("failed to find expected TCP metric when pod is opaque\n%s", metrics)
+				}
 
-func waitForAppDeploymentReady(t *testing.T, opaquePortsNs string) {
-	TestHelper.WaitRollout(t, map[string]testutil.DeploySpec{
-		opaquePodApp: {
-			Namespace: opaquePortsNs,
-			Replicas:  1,
-		},
-		opaqueSvcApp: {
-			Namespace: opaquePortsNs,
-			Replicas:  1,
-		},
-		opaqueUnmeshedSvcPod: {
-			Namespace: opaquePortsNs,
-			Replicas:  1,
-		},
-	})
-}
+				return nil
+			})
 
-func templateArgsPodIP(ctx context.Context, ns string) (clientTemplateArgs, error) {
-	opaquePodSCPodIP, err := getPodIPByAppLabel(ctx, ns, opaquePodApp)
-	if err != nil {
-		return clientTemplateArgs{}, fmt.Errorf("failed to fetch pod IP for %q: %w", opaquePodApp, err)
-	}
-	opaqueSvcSCPodIP, err := getPodIPByAppLabel(ctx, ns, opaqueSvcApp)
-	if err != nil {
-		return clientTemplateArgs{}, fmt.Errorf("failed to fetch pod IP for %q: %w", opaqueSvcApp, err)
-	}
-	opaqueUnmeshedSvcPodIP, err := getPodIPByAppLabel(ctx, ns, opaqueUnmeshedSvcPod)
-	if err != nil {
-		return clientTemplateArgs{}, fmt.Errorf("failed to fetch pod IP for %q: %w", opaqueUnmeshedSvcPod, err)
-	}
-	return clientTemplateArgs{
-		ServiceCookerOpaquePodTargetHost:         opaquePodSCPodIP,
-		ServiceCookerOpaqueServiceTargetHost:     opaqueSvcSCPodIP,
-		ServiceCookerOpaqueUnmeshedSVCTargetHost: opaqueUnmeshedSvcPodIP,
-	}, nil
-}
+			if err != nil {
+				testutil.AnnotatedFatalf(t, "unexpected metric output", "unexpected metric output: %s", err)
+			}
+		})
 
-func runTests(ctx context.Context, t *testing.T, ns string, tcs []testCase) {
-	t.Helper()
-	for _, tc := range tcs {
-		t.Run(tc.name, func(t *testing.T) {
+		t.Run("expect inbound TCP connection metric with expected TLS identity for opaque service app", func(t *testing.T) {
+			// Check the slow cooker metrics
 			err := TestHelper.RetryFor(30*time.Second, func() error {
-				if err := checkPodMetrics(ctx, ns, tc.scName, tc.scChecks); err != nil {
-					return fmt.Errorf("failed to check metrics for client pod:%w", err)
+				pods, err := TestHelper.GetPods(ctx, opaquePortsNs, map[string]string{"app": opaqueSvcSC})
+				if err != nil || len(pods) == 0 {
+					return fmt.Errorf("error getting pods\n%w", err)
+				}
+				metrics, err := getPodMetrics(pods[0], opaquePortsNs)
+				if err != nil {
+					return fmt.Errorf("error getting metrics for pod\n%w", err)
 				}
-				if tc.appName == "" {
-					return nil
+				if httpRequestTotalMetricRE.MatchString(metrics) {
+					return fmt.Errorf("expected not to find HTTP outbound requests when service is opaque\n%s", metrics)
 				}
-				if err := checkPodMetrics(ctx, ns, tc.appName, tc.appChecks); err != nil {
-					return fmt.Errorf("failed to check metrics for app pod:%w", err)
+				// Check the application metrics
+				pods, err = TestHelper.GetPods(ctx, opaquePortsNs, map[string]string{"app": opaqueSvcApp})
+				if err != nil {
+					return fmt.Errorf("error getting pods\n%w", err)
 				}
+				metrics, err = getPodMetrics(pods[0], opaquePortsNs)
+				if err != nil {
+					return fmt.Errorf("error getting metrics for pod\n%w", err)
+				}
+				if !tcpMetricRE.MatchString(metrics) {
+					return fmt.Errorf("failed to find expected TCP metric when pod is opaque\n%s", metrics)
+				}
+
 				return nil
 			})
+
 			if err != nil {
-				testutil.AnnotatedFatalf(t, "unexpected metric for pod", "unexpected metric for pod: %s", err)
+				testutil.AnnotatedFatalf(t, "unexpected metric output", "unexpected metric output: %v", err)
 			}
 		})
-	}
-}
 
-func checkPodMetrics(ctx context.Context, opaquePortsNs string, podAppLabel string, checks []check) error {
-	pods, err := TestHelper.GetPods(ctx, opaquePortsNs, map[string]string{"app": podAppLabel})
-	if err != nil {
-		return fmt.Errorf("error getting pods for label 'app: %q': %w", podAppLabel, err)
-	}
-	if len(pods) == 0 {
-		return fmt.Errorf("no pods found for label 'app: %q'", podAppLabel)
-	}
-	metrics, err := getPodMetrics(pods[0], opaquePortsNs)
-	if err != nil {
-		return fmt.Errorf("error getting metrics for pod %q: %w", pods[0].Name, err)
-	}
-	for _, check := range checks {
-		if err := check(metrics); err != nil {
-			return fmt.Errorf("validation of client pod metrics failed: %w", err)
-		}
-	}
-	return nil
-}
+		t.Run("expect inbound TCP connection metric with no TLS identity for traffic between meshed and unmeshed opaque service", func(t *testing.T) {
+			// Slow cooker is meshed, should have valid outbound TCP metric, valid
+			// inbound TCP metric and no HTTP metric.
+			err := TestHelper.RetryFor(30*time.Second, func() error {
+				pods, err := TestHelper.GetPods(ctx, opaquePortsNs,
+					map[string]string{"app": opaqueUnmeshedSvcSC})
+				if err != nil {
+					return fmt.Errorf("error getting pods\n%w", err)
+				}
+				metrics, err := getPodMetrics(pods[0], opaquePortsNs)
+				if err != nil {
+					return fmt.Errorf("error getting metrics for pod\n%w", err)
+				}
 
-func deployApplications(ns string) error {
-	out, err := TestHelper.Kubectl("", "apply", "-f", "testdata/opaque_ports_application.yaml", "-n", ns)
-	if err != nil {
-		return fmt.Errorf("failed apply deployment file %q: %w", out, err)
-	}
-	return nil
-}
+				if httpRequestTotalUnmeshedRE.MatchString(metrics) {
+					return fmt.Errorf("expected not to find HTTP outbound requests when service is opaque\n%s", metrics)
+				}
+				if !tcpMetricOutUnmeshedRE.MatchString(metrics) {
+					return fmt.Errorf("failed to find expected TCP outbound metric when pod is opaque\n%s", metrics)
+				}
 
-func deployClients(ns string, templateArgs clientTemplateArgs) error {
-	return deployTemplate(ns, opaquePortsClientTemplate, templateArgs)
-}
+				return nil
+			})
 
-func deployTemplate(ns string, tmpl *template.Template, templateArgs interface{}) error {
-	bb := &bytes.Buffer{}
-	if err := tmpl.Execute(bb, templateArgs); err != nil {
-		return fmt.Errorf("failed to write deployment template: %w", err)
-	}
-	out, err := TestHelper.KubectlApply(bb.String(), ns)
-	if err != nil {
-		return fmt.Errorf("failed apply deployment file %q: %w", out, err)
-	}
-	return nil
+			if err != nil {
+				testutil.AnnotatedFatalf(t, "unexpected metric output", "unexpected metric output: %v", err)
+			}
+		})
+	})
 }
 
 func getPodMetrics(pod v1.Pod, ns string) (string, error) {
@@ -279,15 +188,3 @@ func getPodMetrics(pod v1.Pod, ns string) (string, error) {
 	}
 	return metrics, nil
 }
-
-func getPodIPByAppLabel(ctx context.Context, ns string, app string) (string, error) {
-	labels := map[string]string{"app": app}
-	pods, err := TestHelper.GetPods(ctx, ns, labels)
-	if err != nil {
-		return "", fmt.Errorf("failed to get pod by labels %v: %w", labels, err)
-	}
-	if len(pods) == 0 {
-		return "", fmt.Errorf("no pods found for labels %v", labels)
-	}
-	return pods[0].Status.PodIP, nil
-}
diff --git a/test/integration/deep/opaqueports/testdata/opaque_ports_client.yaml b/test/integration/deep/opaqueports/testdata/opaque_ports_client.yaml
index 11e6bba63..cee4405f8 100644
--- a/test/integration/deep/opaqueports/testdata/opaque_ports_client.yaml
+++ b/test/integration/deep/opaqueports/testdata/opaque_ports_client.yaml
@@ -16,7 +16,7 @@ spec:
         args:
         - -qps=1
         - -metric-addr=0.0.0.0:9999
-        - http://{{ .ServiceCookerOpaqueServiceTargetHost}}:8080
+        - http://svc-opaque-service:8080
         ports:
         - containerPort: 9999
       restartPolicy: OnFailure
@@ -39,7 +39,7 @@ spec:
         args:
         - -qps=1
         - -metric-addr=0.0.0.0:9999
-        - http://{{ .ServiceCookerOpaquePodTargetHost}}:8080
+        - http://svc-opaque-pod:8080
         ports:
         - containerPort: 9999
       restartPolicy: OnFailure
@@ -62,7 +62,7 @@ spec:
         args:
         - -qps=1
         - -metric-addr=0.0.0.0:9999
-        - http://{{ .ServiceCookerOpaqueUnmeshedSVCTargetHost}}:8080
+        - http://svc-opaque-unmeshed:8080
         ports:
         - containerPort: 9999
       restartPolicy: OnFailure