Repository: docker/compose-cli
Pull request #681 (2020-09-28 07:03:21)

Api metrics context: send context type, not contet name, and never fail
Signed-off-by: Guillaume Tardif <guillaume.tardif@docker.com>

**What I did**
* never fail in metrics interceptor
* send context type, not context name

**Related issue**

<!-- optional tests
You can add a / mention to run tests executed by default only on main branch : 
* `test-aci` to run ACI E2E tests
* `test-ecs` to run ECS E2E tests
* `test-windows` to run tests & E2E tests on windows
-->

**(not mandatory) A picture of a cute animal, if possible in relation with what you did**



Patch:
diff --git a/INSTALL.md b/INSTALL.md
index 542b8abb..7a561121 100644
--- a/INSTALL.md
+++ b/INSTALL.md
@@ -19,7 +19,7 @@ with a fresh install of Ubuntu 20.04.
 You can install the Compose CLI using the install script:
 
 ```console
-curl -L https://raw.githubusercontent.com/docker/compose-cli/main/scripts/install/install_linux.sh | sh
+curl -L https://raw.githubusercontent.com/docker/compose-cli/main/scripts/install_linux.sh | sh
 ```
 
 ## Manual install
diff --git a/api/client/client.go b/api/client/client.go
index a35c0b74..9dfb2f62 100644
--- a/api/client/client.go
+++ b/api/client/client.go
@@ -44,10 +44,16 @@ func New(ctx context.Context) (*Client, error) {
 		return nil, err
 	}
 
-	return &Client{
-		backendType: cc.Type(),
+	client := NewClient(cc.Type(), service)
+	return &client, nil
+}
+
+// NewClient returns new client
+func NewClient(backendType string, service backend.Service) Client {
+	return Client{
+		backendType: backendType,
 		bs:          service,
-	}, nil
+	}
 }
 
 // GetCloudService returns a backend CloudService (typically login, create context)
@@ -61,6 +67,11 @@ type Client struct {
 	bs          backend.Service
 }
 
+// ContextType the context type associated with backend
+func (c *Client) ContextType() string {
+	return c.backendType
+}
+
 // ContainerService returns the backend service for the current context
 func (c *Client) ContainerService() containers.Service {
 	if cs := c.bs.ContainerService(); cs != nil {
diff --git a/docs/install/install.sh b/docs/install/install.sh
new file mode 100755
index 00000000..00b767bf
--- /dev/null
+++ b/docs/install/install.sh
@@ -0,0 +1,197 @@
+#!/bin/sh
+
+#   Copyright 2020 Docker Compose CLI authors
+
+#   Licensed under the Apache License, Version 2.0 (the "License");
+#   you may not use this file except in compliance with the License.
+#   You may obtain a copy of the License at
+
+#       http://www.apache.org/licenses/LICENSE-2.0
+
+#   Unless required by applicable law or agreed to in writing, software
+#   distributed under the License is distributed on an "AS IS" BASIS,
+#   WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+#   See the License for the specific language governing permissions and
+#   limitations under the License.
+
+
+# Script to install the Docker ACI integration CLI on Ubuntu (Beta).
+
+set -eu
+
+CLI_VERSION=${CLI_VERSION:-0.1.4}
+DOWNLOAD_URL="${DOWNLOAD_URL:-https://github.com/docker/aci-integration-beta/releases/download/v${CLI_VERSION}/docker-linux-amd64}"
+LINK_NAME="${LINK_NAME:-com.docker.cli}"
+DRY_RUN="${DRY_RUN:-}"
+
+desktop_install_url="https://www.docker.com/products/docker-desktop"
+engine_install_url="https://docs.docker.com/get-docker/"
+
+link_path="/usr/local/bin/${LINK_NAME}"
+existing_cli_path="/usr/bin/docker"
+
+manual_install() {
+	echo "Please follow the manual install instructions"
+}
+
+is_new_cli() {
+	azure_version_str="$($1 version 2>/dev/null | grep 'Azure' || true)"
+	if [ -n "$azure_version_str" ]; then
+		echo 1
+	else
+		echo 0
+	fi
+}
+
+echo "Running checks..."
+
+# Check OS
+if [ "$(command -v uname)" ]; then
+	case "$(uname -s)" in
+		"Linux")
+			# Check for Ubuntu/Debian based distro
+			if ! [ -f "/etc/lsb-release" ]; then
+				echo "Warning: This script has been tested on Ubuntu and may not work on other distributions"
+			fi
+			# Pass
+			;;
+		"Darwin")
+			echo "Error: Script not needed on macOS, please install Docker Desktop Edge: $desktop_install_url"
+			exit 1
+			;;
+		"*")
+			echo "Error: Unsupported OS, please follow manual instructions"
+			exit 1
+			;;
+	esac
+else
+	# Assume Windows
+	echo "Error: Script not needed on Windows, please install Docker Desktop Edge: $desktop_install_url"
+	exit 1
+fi
+
+user="$(id -un 2>/dev/null || true)"
+sh_c='sh -c'
+sudo_sh_c='sh -c'
+if [ "$user" != 'root' ]; then
+    if [ "$(command -v sudo)" ]; then
+        sudo_sh_c='sudo -E sh -c'
+    elif [ "$(command -v su)" ]; then
+        sudo_sh_c='su -c'
+    else
+        echo "Error: This installer needs the ability to run commands as root."
+        exit 1
+    fi
+fi
+
+if [ -n "$DRY_RUN" ]; then
+	sh_c="echo $sh_c"
+	sudo_sh_c="echo $sudo_sh_c"
+fi
+
+# Check if Docker Engine is installed
+if ! [ "$(command -v docker)" ]; then
+	echo "Error: Docker Engine not found"
+	echo "You need to install Docker first: $engine_install_url"
+	exit 1
+fi
+
+# Check if the ACI CLI is already installed
+if [ $(is_new_cli "docker") -eq 1 ]; then
+	echo "You already have the Docker ACI Integration CLI installed"
+	exit 1
+fi
+
+# Check if this script has already been run
+if [ -f "${link_path}" ]; then
+	echo "Error: This script appears to have been run as ${link_path} exists"
+	echo "Please uninstall and rerun this script or follow the manual instructions"
+	exit 1
+fi
+
+# Check current Docker CLI is installed to /usr/bin/
+if ! [ -f "${existing_cli_path}" ]; then
+	echo "Error: This script only works if the Docker CLI is installed to /usr/bin/"
+	manual_install
+	exit 1
+fi
+
+# Check that PATH contains /usr/bin and /usr/local/bin and that the latter is
+# higher priority
+path_directories=$(echo "${PATH}" | tr ":" "\n")
+usr_bin_pos=-1
+usr_local_bin_pos=-1
+count=0
+for d in ${path_directories}; do
+	if [ "${d}" = '/usr/bin' ]; then
+		usr_bin_pos=$count
+	fi
+	if [ "${d}" = '/usr/local/bin' ]; then
+		usr_local_bin_pos=$count
+	fi
+	count=$((count + 1))
+done
+if [ $usr_bin_pos -eq -1 ]; then
+	echo "Error: /usr/bin not found in PATH"
+	manual_install
+	exit 1
+elif [ $usr_local_bin_pos -eq -1 ]; then
+	echo "Error: /usr/local/bin not found in PATH"
+	manual_install
+	exit 1
+elif ! [ $usr_local_bin_pos -lt $usr_bin_pos ]; then
+	echo "Error: /usr/local/bin is not ordered higher than /usr/bin in your PATH"
+	manual_install
+	exit 1
+fi
+
+download_cmd='curl -fsSLo'
+# Check that system has curl installed
+if ! [ "$(command -v curl)" ]; then
+	echo "Error: curl not found"
+	echo "Please install curl"
+	exit 1
+fi
+
+echo "Checks passed!"
+echo "Downloading CLI..."
+
+# Download CLI to temporary directory
+download_dir=$($sh_c 'mktemp -d')
+$sh_c "${download_cmd} ${download_dir}/docker-aci ${DOWNLOAD_URL}"
+
+echo "Downloaded CLI!"
+echo "Installing CLI..."
+
+# Link existing Docker CLI
+$sudo_sh_c "ln -s ${existing_cli_path} ${link_path}"
+
+# Install downloaded CLI
+$sudo_sh_c "install -m 775 ${download_dir}/docker-aci /usr/local/bin/docker"
+
+# Exit on dry run
+if [ -n "$DRY_RUN" ]; then
+	exit 0
+fi
+
+# Clear cache
+cleared_cache=1
+if [ "$(command hash)" ]; then
+	$sh_c "hash -r"
+elif [ "$(command rehash)" ]; then
+	$sh_c "rehash"
+else
+	cleared_cache=
+	echo "Warning: Unable to clear command cache"
+fi
+
+if [ -n "$cleared_cache" ]; then
+	# Check ACI CLI is working
+	if [ $(is_new_cli "docker") -eq 0 ]; then
+		echo "Error: Docker ACI Integration CLI installation error"
+		exit 1
+	fi
+	echo "Done!"
+else
+	echo "Please log out and in again to use the Docker ACI integration CLI"
+fi
diff --git a/scripts/install/install_linux.sh b/scripts/install/install_linux.sh
index ace08caa..d5927c19 100644
--- a/scripts/install/install_linux.sh
+++ b/scripts/install/install_linux.sh
@@ -180,7 +180,7 @@ echo "Installing CLI..."
 $sudo_sh_c "ln -s ${existing_cli_path} ${link_path}"
 
 # Install downloaded CLI
-$sudo_sh_c "install -m 775 ${download_dir}/docker /usr/local/bin/docker"
+$sudo_sh_c "install -m 775 ${download_dir}/docker-linux-amd64 /usr/local/bin/docker"
 
 # Clear cache
 cleared_cache=1
diff --git a/server/interceptor.go b/server/interceptor.go
index 41072b34..17165a10 100644
--- a/server/interceptor.go
+++ b/server/interceptor.go
@@ -111,10 +111,7 @@ func configureContext(ctx context.Context, currentContext string, method string)
 			return nil, err
 		}
 
-		ctx, err = proxy.WithClient(ctx, c)
-		if err != nil {
-			return nil, err
-		}
+		ctx = proxy.WithClient(ctx, c)
 	}
 
 	s, err := store.New(configDir)
diff --git a/server/metrics.go b/server/metrics.go
index 9c5c64f4..83ee4884 100644
--- a/server/metrics.go
+++ b/server/metrics.go
@@ -22,6 +22,7 @@ import (
 	"google.golang.org/grpc"
 
 	"github.com/docker/compose-cli/metrics"
+	"github.com/docker/compose-cli/server/proxy"
 )
 
 var (
@@ -41,14 +42,12 @@ var (
 	}
 )
 
-func metricsServerInterceptor(clictx context.Context, client metrics.Client) grpc.UnaryServerInterceptor {
+func metricsServerInterceptor(client metrics.Client) grpc.UnaryServerInterceptor {
 	return func(ctx context.Context, req interface{}, info *grpc.UnaryServerInfo, handler grpc.UnaryHandler) (interface{}, error) {
-		currentContext, err := getIncomingContext(ctx)
-		if err != nil {
-			currentContext, err = getConfigContext(clictx)
-			if err != nil {
-				return nil, err
-			}
+		backendClient := proxy.Client(ctx)
+		contextType := ""
+		if backendClient != nil {
+			contextType = backendClient.ContextType()
 		}
 
 		data, err := handler(ctx, req)
@@ -61,7 +60,7 @@ func metricsServerInterceptor(clictx context.Context, client metrics.Client) grp
 		if command != "" {
 			client.Send(metrics.Command{
 				Command: command,
-				Context: currentContext,
+				Context: contextType,
 				Source:  metrics.APISource,
 				Status:  status,
 			})
diff --git a/server/proxy/proxy.go b/server/proxy/proxy.go
index 7a9f0f4e..30733ba9 100644
--- a/server/proxy/proxy.go
+++ b/server/proxy/proxy.go
@@ -31,8 +31,8 @@ import (
 type clientKey struct{}
 
 // WithClient adds the client to the context
-func WithClient(ctx context.Context, c *client.Client) (context.Context, error) {
-	return context.WithValue(ctx, clientKey{}, c), nil
+func WithClient(ctx context.Context, c *client.Client) context.Context {
+	return context.WithValue(ctx, clientKey{}, c)
 }
 
 // Client returns the client from the context
diff --git a/server/server.go b/server/server.go
index 0842afe8..e5adfa53 100644
--- a/server/server.go
+++ b/server/server.go
@@ -33,7 +33,7 @@ func New(ctx context.Context) *grpc.Server {
 	s := grpc.NewServer(
 		grpc.ChainUnaryInterceptor(
 			unaryServerInterceptor(ctx),
-			metricsServerInterceptor(ctx, metrics.NewClient()),
+			metricsServerInterceptor(metrics.NewClient()),
 		),
 		grpc.StreamInterceptor(streamServerInterceptor(ctx)),
 	)


Test patch:
diff --git a/server/metrics_test.go b/server/metrics_test.go
index ad061e15..9f9e886f 100644
--- a/server/metrics_test.go
+++ b/server/metrics_test.go
@@ -26,6 +26,11 @@ import (
 	"google.golang.org/grpc/metadata"
 	"gotest.tools/v3/assert"
 
+	"github.com/docker/compose-cli/api/client"
+	"github.com/docker/compose-cli/api/compose"
+	"github.com/docker/compose-cli/api/containers"
+	"github.com/docker/compose-cli/api/secrets"
+	"github.com/docker/compose-cli/api/volumes"
 	"github.com/docker/compose-cli/errdefs"
 	"github.com/docker/compose-cli/metrics"
 	containersv1 "github.com/docker/compose-cli/protos/containers/v1"
@@ -55,18 +60,21 @@ func TestAllMethodsHaveCorrespondingCliCommand(t *testing.T) {
 func TestTrackSuccess(t *testing.T) {
 	var mockMetrics = &mockMetricsClient{}
 	mockMetrics.On("Send", metrics.Command{Command: "ps", Context: "aci", Status: "success", Source: "api"}).Return()
-	interceptor := metricsServerInterceptor(context.TODO(), mockMetrics)
+	newClient := client.NewClient("aci", noopService{})
+	interceptor := metricsServerInterceptor(mockMetrics)
 
-	_, err := interceptor(incomingContext("aci"), nil, containerMethodRoute("List"), mockHandler(nil))
+	ctx := proxy.WithClient(incomingContext("acicontext"), &newClient)
+	_, err := interceptor(ctx, nil, containerMethodRoute("List"), mockHandler(nil))
 	assert.NilError(t, err)
 }
 
 func TestTrackSFailures(t *testing.T) {
 	var mockMetrics = &mockMetricsClient{}
-	mockMetrics.On("Send", metrics.Command{Command: "ps", Context: "default", Status: "failure", Source: "api"}).Return()
-	interceptor := metricsServerInterceptor(context.TODO(), mockMetrics)
+	newClient := client.NewClient("moby", noopService{})
+	interceptor := metricsServerInterceptor(mockMetrics)
 
-	_, err := interceptor(incomingContext("default"), nil, containerMethodRoute("Create"), mockHandler(errdefs.ErrLoginRequired))
+	ctx := proxy.WithClient(incomingContext("default"), &newClient)
+	_, err := interceptor(ctx, nil, containerMethodRoute("Create"), mockHandler(errdefs.ErrLoginRequired))
 	assert.Assert(t, err == errdefs.ErrLoginRequired)
 }
 
@@ -100,6 +108,13 @@ func setupServer() *grpc.Server {
 	return s
 }
 
+type noopService struct{}
+
+func (noopService) ContainerService() containers.Service { return nil }
+func (noopService) ComposeService() compose.Service      { return nil }
+func (noopService) SecretsService() secrets.Service      { return nil }
+func (noopService) VolumeService() volumes.Service       { return nil }
+
 type mockMetricsClient struct {
 	mock.Mock
 }