From 4bc006c8a4911c2053f47889f7ef7069c7fb7418 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?= Date: Tue, 2 Mar 2021 14:33:25 +0100 Subject: [PATCH 1/2] runtime: Short the shim-monitor path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Instead of having something like "/containerd-shim/$namespace/$sandboxID/shim-monitor.sock", let's change the approach to: * create the file in a more neutral location "/run/vc", instead of "/containerd-shim"; * drop the namespace, as the sandboxID should be unique; * remove ".sock" from the socket name. This will result on a name that looks like: "/run/vc/$sandboxID/shim-monitor" Fixes: #497 Signed-off-by: Fabiano Fidêncio --- src/runtime/cli/kata-exec.go | 9 ++++++--- src/runtime/containerd-shim-v2/shim_management.go | 7 +------ 2 files changed, 7 insertions(+), 9 deletions(-) diff --git a/src/runtime/cli/kata-exec.go b/src/runtime/cli/kata-exec.go index d556f782ac..dba2e8f299 100644 --- a/src/runtime/cli/kata-exec.go +++ b/src/runtime/cli/kata-exec.go @@ -26,6 +26,7 @@ import ( clientUtils "github.com/kata-containers/kata-containers/src/runtime/virtcontainers/pkg/agent/protocols/client" "github.com/pkg/errors" "github.com/urfave/cli" + "go.opentelemetry.io/otel/label" ) const ( @@ -82,7 +83,9 @@ var kataExecCLICommand = cli.Command{ return err } - conn, err := getConn(namespace, sandboxID, port) + span.SetAttributes(label.Key("sandbox").String(sandboxID)) + conn, err := getConn(sandboxID, port) + if err != nil { return err } @@ -165,8 +168,8 @@ func (s *iostream) Read(data []byte) (n int, err error) { return s.conn.Read(data) } -func getConn(namespace, sandboxID string, port uint64) (net.Conn, error) { - socketAddr := filepath.Join(string(filepath.Separator), "containerd-shim", namespace, sandboxID, "shim-monitor.sock") +func getConn(sandboxID string, port uint64) (net.Conn, error) { + socketAddr := filepath.Join(string(filepath.Separator), "run", "vc", sandboxID, "shim-monitor") client, err := kataMonitor.BuildUnixSocketClient(socketAddr, defaultTimeout) if err != nil { return nil, err diff --git a/src/runtime/containerd-shim-v2/shim_management.go b/src/runtime/containerd-shim-v2/shim_management.go index d7b4bb1cac..7be8af41d7 100644 --- a/src/runtime/containerd-shim-v2/shim_management.go +++ b/src/runtime/containerd-shim-v2/shim_management.go @@ -16,7 +16,6 @@ import ( "strconv" "strings" - "github.com/containerd/containerd/namespaces" cdshim "github.com/containerd/containerd/runtime/v2/shim" vc "github.com/kata-containers/kata-containers/src/runtime/virtcontainers" vcAnnotations "github.com/kata-containers/kata-containers/src/runtime/virtcontainers/pkg/annotations" @@ -189,9 +188,5 @@ func (s *service) mountPprofHandle(m *http.ServeMux, ociSpec *specs.Spec) { } func socketAddress(ctx context.Context, id string) (string, error) { - ns, err := namespaces.NamespaceRequired(ctx) - if err != nil { - return "", err - } - return filepath.Join(string(filepath.Separator), "containerd-shim", ns, id, "shim-monitor.sock"), nil + return filepath.Join(string(filepath.Separator), "run", "vc", id, "shim-monitor"), nil } From 3caed6f88d37e542133a39a28ec502566d07c722 Mon Sep 17 00:00:00 2001 From: Eric Ernst Date: Thu, 6 May 2021 17:09:47 -0700 Subject: [PATCH 2/2] runtime: shim: dedup client, socket addr code (1) Add an accessor function, SocketAddress, to the shim-v2 code for determining the shim's abstract domain socket address, given the sandbox ID. (2) In kata monitor, create a function, BuildShimClient, for obtaining the appropriate http.Client for communicating with the shim's monitoring endpoint. (3) Update the kata CLI and kata-monitor code to make use of these. (4) Migrate some kata monitor methods to be functions, in order to ease future reuse. (5) drop unused namespace from functions where it is no longer needed. Signed-off-by: Eric Ernst --- src/runtime/cli/kata-exec.go | 19 ++----------------- .../containerd-shim-v2/shim_management.go | 12 +++++------- src/runtime/pkg/kata-monitor/metrics.go | 6 +++--- src/runtime/pkg/kata-monitor/monitor.go | 7 +------ src/runtime/pkg/kata-monitor/shim_client.go | 19 +++++++++---------- 5 files changed, 20 insertions(+), 43 deletions(-) diff --git a/src/runtime/cli/kata-exec.go b/src/runtime/cli/kata-exec.go index dba2e8f299..24c6e2f5b6 100644 --- a/src/runtime/cli/kata-exec.go +++ b/src/runtime/cli/kata-exec.go @@ -14,7 +14,6 @@ import ( "net/http" "net/url" "os" - "path/filepath" "strings" "sync" @@ -26,7 +25,6 @@ import ( clientUtils "github.com/kata-containers/kata-containers/src/runtime/virtcontainers/pkg/agent/protocols/client" "github.com/pkg/errors" "github.com/urfave/cli" - "go.opentelemetry.io/otel/label" ) const ( @@ -38,10 +36,8 @@ const ( subCommandName = "exec" // command-line parameters name - paramRuntimeNamespace = "runtime-namespace" paramDebugConsolePort = "kata-debug-port" defaultKernelParamDebugConsoleVPortValue = 1026 - defaultRuntimeNamespace = "k8s.io" ) var ( @@ -57,21 +53,12 @@ var kataExecCLICommand = cli.Command{ Name: subCommandName, Usage: "Enter into guest by debug console", Flags: []cli.Flag{ - cli.StringFlag{ - Name: paramRuntimeNamespace, - Usage: "Namespace that containerd or CRI-O are using for containers. (Default: k8s.io, only works for containerd)", - }, cli.Uint64Flag{ Name: paramDebugConsolePort, Usage: "Port that debug console is listening on. (Default: 1026)", }, }, Action: func(context *cli.Context) error { - namespace := context.String(paramRuntimeNamespace) - if namespace == "" { - namespace = defaultRuntimeNamespace - } - port := context.Uint64(paramDebugConsolePort) if port == 0 { port = defaultKernelParamDebugConsoleVPortValue @@ -83,7 +70,6 @@ var kataExecCLICommand = cli.Command{ return err } - span.SetAttributes(label.Key("sandbox").String(sandboxID)) conn, err := getConn(sandboxID, port) if err != nil { @@ -169,8 +155,7 @@ func (s *iostream) Read(data []byte) (n int, err error) { } func getConn(sandboxID string, port uint64) (net.Conn, error) { - socketAddr := filepath.Join(string(filepath.Separator), "run", "vc", sandboxID, "shim-monitor") - client, err := kataMonitor.BuildUnixSocketClient(socketAddr, defaultTimeout) + client, err := kataMonitor.BuildShimClient(sandboxID, defaultTimeout) if err != nil { return nil, err } @@ -181,7 +166,7 @@ func getConn(sandboxID string, port uint64) (net.Conn, error) { } if resp.StatusCode != http.StatusOK { - return nil, fmt.Errorf("Failed to get %s: %d", socketAddr, resp.StatusCode) + return nil, fmt.Errorf("Failure from %s shim-monitor: %d", sandboxID, resp.StatusCode) } defer resp.Body.Close() diff --git a/src/runtime/containerd-shim-v2/shim_management.go b/src/runtime/containerd-shim-v2/shim_management.go index 7be8af41d7..ab7ebf2041 100644 --- a/src/runtime/containerd-shim-v2/shim_management.go +++ b/src/runtime/containerd-shim-v2/shim_management.go @@ -128,11 +128,7 @@ func decodeAgentMetrics(body string) []*dto.MetricFamily { func (s *service) startManagementServer(ctx context.Context, ociSpec *specs.Spec) { // metrics socket will under sandbox's bundle path - metricsAddress, err := socketAddress(ctx, s.id) - if err != nil { - shimMgtLog.WithError(err).Error("failed to create socket address") - return - } + metricsAddress := SocketAddress(s.id) listener, err := cdshim.NewSocket(metricsAddress) if err != nil { @@ -187,6 +183,8 @@ func (s *service) mountPprofHandle(m *http.ServeMux, ociSpec *specs.Spec) { m.Handle("/debug/pprof/trace", http.HandlerFunc(pprof.Trace)) } -func socketAddress(ctx context.Context, id string) (string, error) { - return filepath.Join(string(filepath.Separator), "run", "vc", id, "shim-monitor"), nil +// SocketAddress returns the address of the abstract domain socket for communicating with the +// shim management endpoint +func SocketAddress(id string) string { + return filepath.Join(string(filepath.Separator), "run", "vc", id, "shim-monitor") } diff --git a/src/runtime/pkg/kata-monitor/metrics.go b/src/runtime/pkg/kata-monitor/metrics.go index b4fcd5830a..aeb9f72c5d 100644 --- a/src/runtime/pkg/kata-monitor/metrics.go +++ b/src/runtime/pkg/kata-monitor/metrics.go @@ -176,7 +176,7 @@ func (km *KataMonitor) aggregateSandboxMetrics(encoder expfmt.Encoder) error { for sandboxID, namespace := range sandboxes { wg.Add(1) go func(sandboxID, namespace string, results chan<- []*dto.MetricFamily) { - sandboxMetrics, err := km.getSandboxMetrics(sandboxID, namespace) + sandboxMetrics, err := getSandboxMetrics(sandboxID) if err != nil { monitorLog.WithError(err).WithField("sandbox_id", sandboxID).Errorf("failed to get metrics for sandbox") } @@ -234,8 +234,8 @@ func (km *KataMonitor) aggregateSandboxMetrics(encoder expfmt.Encoder) error { } // getSandboxMetrics will get sandbox's metrics from shim -func (km *KataMonitor) getSandboxMetrics(sandboxID, namespace string) ([]*dto.MetricFamily, error) { - body, err := km.doGet(sandboxID, namespace, defaultTimeout, "metrics") +func getSandboxMetrics(sandboxID string) ([]*dto.MetricFamily, error) { + body, err := doGet(sandboxID, defaultTimeout, "metrics") if err != nil { return nil, err } diff --git a/src/runtime/pkg/kata-monitor/monitor.go b/src/runtime/pkg/kata-monitor/monitor.go index 64266fdb7d..1c663a5fdd 100644 --- a/src/runtime/pkg/kata-monitor/monitor.go +++ b/src/runtime/pkg/kata-monitor/monitor.go @@ -87,13 +87,8 @@ func (km *KataMonitor) GetAgentURL(w http.ResponseWriter, r *http.Request) { commonServeError(w, http.StatusBadRequest, err) return } - namespace, err := km.getSandboxNamespace(sandboxID) - if err != nil { - commonServeError(w, http.StatusBadRequest, err) - return - } - data, err := km.doGet(sandboxID, namespace, defaultTimeout, "agent-url") + data, err := doGet(sandboxID, defaultTimeout, "agent-url") if err != nil { commonServeError(w, http.StatusBadRequest, err) return diff --git a/src/runtime/pkg/kata-monitor/shim_client.go b/src/runtime/pkg/kata-monitor/shim_client.go index b2c462e8fa..ef4dc8a330 100644 --- a/src/runtime/pkg/kata-monitor/shim_client.go +++ b/src/runtime/pkg/kata-monitor/shim_client.go @@ -11,6 +11,8 @@ import ( "net" "net/http" "time" + + shim "github.com/kata-containers/kata-containers/src/runtime/containerd-shim-v2" ) const ( @@ -33,16 +35,13 @@ func getSandboxIDFromReq(r *http.Request) (string, error) { return "", fmt.Errorf("sandbox not found in %+v", r.URL.Query()) } -func (km *KataMonitor) buildShimClient(sandboxID, namespace string, timeout time.Duration) (*http.Client, error) { - socketAddr, err := km.getMonitorAddress(sandboxID, namespace) - if err != nil { - return nil, err - } - return BuildUnixSocketClient(socketAddr, timeout) +// BuildShimClient builds and returns an http client for communicating with the provided sandbox +func BuildShimClient(sandboxID string, timeout time.Duration) (*http.Client, error) { + return buildUnixSocketClient(shim.SocketAddress(sandboxID), timeout) } -// BuildUnixSocketClient build http client for Unix socket -func BuildUnixSocketClient(socketAddr string, timeout time.Duration) (*http.Client, error) { +// buildUnixSocketClient build http client for Unix socket +func buildUnixSocketClient(socketAddr string, timeout time.Duration) (*http.Client, error) { transport := &http.Transport{ DisableKeepAlives: true, Dial: func(proto, addr string) (conn net.Conn, err error) { @@ -61,8 +60,8 @@ func BuildUnixSocketClient(socketAddr string, timeout time.Duration) (*http.Clie return client, nil } -func (km *KataMonitor) doGet(sandboxID, namespace string, timeoutInSeconds time.Duration, urlPath string) ([]byte, error) { - client, err := km.buildShimClient(sandboxID, namespace, timeoutInSeconds) +func doGet(sandboxID string, timeoutInSeconds time.Duration, urlPath string) ([]byte, error) { + client, err := BuildShimClient(sandboxID, timeoutInSeconds) if err != nil { return nil, err }