Skip to content

Commit 59ffbf4

Browse files
estespestesp
authored
Merge pull request containerd#10761 from cpuguy83/shim_remove_nethttp
More shim imports cleanup
2 parents f1c70e8 + b85909c commit 59ffbf4

File tree

7 files changed

+162
-54
lines changed

7 files changed

+162
-54
lines changed

‎Makefile‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -96,7 +96,11 @@ GO_BUILDTAGS += ${DEBUG_TAGS}
9696
ifneq ($(STATIC),)
9797
GO_BUILDTAGS += osusergo netgo static_build
9898
endif
99+
100+
SHIM_GO_BUILDTAGS := $(GO_BUILDTAGS) no_grpc
101+
99102
GO_TAGS=$(if $(GO_BUILDTAGS),-tags "$(strip $(GO_BUILDTAGS))",)
103+
SHIM_GO_TAGS=$(if $(SHIM_GO_BUILDTAGS),-tags "$(strip $(SHIM_GO_BUILDTAGS))",)
100104

101105
GO_LDFLAGS=-ldflags '-X $(PKG)/version.Version=$(VERSION) -X $(PKG)/version.Revision=$(REVISION) -X $(PKG)/version.Package=$(PACKAGE) $(EXTRA_LDFLAGS)
102106
ifneq ($(STATIC),)
@@ -150,7 +154,6 @@ GOTEST ?= $(GO) test
150154
OUTPUTDIR = $(join $(ROOTDIR), _output)
151155
CRIDIR=$(OUTPUTDIR)/cri
152156

153-
SHIM_GO_TAGS := --tags no_grpc
154157

155158
.PHONY: clean all AUTHORS build binaries test integration generate protos check-protos coverage ci check help install uninstall vendor release static-release mandir install-man install-doc genman install-cri-deps cri-release cri-cni-release cri-integration install-deps bin/cri-integration.test remove-replace clean-vendor
156159
.DEFAULT: default
@@ -267,7 +270,7 @@ bin/gen-manpages: cmd/gen-manpages FORCE
267270

268271
bin/containerd-shim-runc-v2: cmd/containerd-shim-runc-v2 FORCE # set !cgo and omit pie for a static shim build: https://github.com/golang/go/issues/17789#issuecomment-258542220
269272
@echo "$(WHALE) $@"
270-
CGO_ENABLED=${SHIM_CGO_ENABLED} $(GO) build ${GO_BUILD_FLAGS} -o $@ ${SHIM_GO_LDFLAGS} ${GO_TAGS} ${SHIM_GO_TAGS} ./cmd/containerd-shim-runc-v2
273+
CGO_ENABLED=${SHIM_CGO_ENABLED} $(GO) build ${GO_BUILD_FLAGS} -o $@ ${SHIM_GO_LDFLAGS} ${SHIM_GO_TAGS} ./cmd/containerd-shim-runc-v2
271274

272275
binaries: $(BINARIES) ## build binaries
273276
@echo "$(WHALE) $@"

‎cmd/containerd-shim-runc-v2/main_tracing.go‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,4 +18,7 @@
1818

1919
package main
2020

21-
import _ "github.com/containerd/containerd/v2/pkg/tracing/plugin"
21+
import (
22+
_ "github.com/containerd/containerd/v2/internal/pprof"
23+
_ "github.com/containerd/containerd/v2/pkg/tracing/plugin"
24+
)

‎integration/failpoint/cmd/containerd-shim-runc-fp-v1/plugin_linux.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,7 @@ func init() {
7474
}
7575

7676
var (
77-
_ = shim.TTRPCServerOptioner(&taskServiceWithFp{})
77+
_ = shim.TTRPCServerUnaryOptioner(&taskServiceWithFp{})
7878
)
7979

8080
type taskServiceWithFp struct {
@@ -87,7 +87,7 @@ func (s *taskServiceWithFp) RegisterTTRPC(server *ttrpc.Server) error {
8787
return nil
8888
}
8989

90-
func (s *taskServiceWithFp) UnaryInterceptor() ttrpc.UnaryServerInterceptor {
90+
func (s *taskServiceWithFp) UnaryServerInterceptor() ttrpc.UnaryServerInterceptor {
9191
return func(ctx context.Context, unmarshal ttrpc.Unmarshaler, info *ttrpc.UnaryServerInfo, method ttrpc.Method) (interface{}, error) {
9292
methodName := filepath.Base(info.FullMethod)
9393
if fp, ok := s.fps[methodName]; ok {

‎internal/pprof/plugin.go‎

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
/*
2+
Copyright The containerd Authors.
3+
4+
Licensed under the Apache License, Version 2.0 (the "License");
5+
you may not use this file except in compliance with the License.
6+
You may obtain a copy of the License at
7+
8+
http://www.apache.org/licenses/LICENSE-2.0
9+
10+
Unless required by applicable law or agreed to in writing, software
11+
distributed under the License is distributed on an "AS IS" BASIS,
12+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
See the License for the specific language governing permissions and
14+
limitations under the License.
15+
*/
16+
17+
package pprof
18+
19+
import (
20+
"expvar"
21+
"net/http"
22+
"net/http/pprof"
23+
"time"
24+
25+
"github.com/containerd/containerd/v2/plugins"
26+
"github.com/containerd/plugin"
27+
"github.com/containerd/plugin/registry"
28+
)
29+
30+
const pluginName = "pprof"
31+
32+
func init() {
33+
registry.Register(&plugin.Registration{
34+
ID: pluginName,
35+
Type: plugins.HTTPHandler,
36+
InitFn: func(ic *plugin.InitContext) (interface{}, error) {
37+
return newHandler(), nil
38+
},
39+
})
40+
}
41+
42+
func newHandler() *http.Server {
43+
m := http.NewServeMux()
44+
m.Handle("/debug/vars", expvar.Handler())
45+
m.Handle("/debug/pprof/", http.HandlerFunc(pprof.Index))
46+
m.Handle("/debug/pprof/cmdline", http.HandlerFunc(pprof.Cmdline))
47+
m.Handle("/debug/pprof/profile", http.HandlerFunc(pprof.Profile))
48+
m.Handle("/debug/pprof/symbol", http.HandlerFunc(pprof.Symbol))
49+
m.Handle("/debug/pprof/trace", http.HandlerFunc(pprof.Trace))
50+
51+
return &http.Server{
52+
Handler: m,
53+
ReadHeaderTimeout: 5 * time.Minute,
54+
}
55+
}

‎pkg/shim/shim.go‎

Lines changed: 47 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -20,13 +20,10 @@ import (
2020
"context"
2121
"encoding/json"
2222
"errors"
23-
"expvar"
2423
"flag"
2524
"fmt"
2625
"io"
2726
"net"
28-
"net/http"
29-
"net/http/pprof"
3027
"os"
3128
"path/filepath"
3229
"runtime"
@@ -43,7 +40,6 @@ import (
4340
"github.com/containerd/containerd/v2/plugins"
4441
"github.com/containerd/containerd/v2/version"
4542
"github.com/containerd/log"
46-
"github.com/containerd/otelttrpc"
4743
"github.com/containerd/plugin"
4844
"github.com/containerd/plugin/registry"
4945
"github.com/containerd/ttrpc"
@@ -112,10 +108,12 @@ type TTRPCService interface {
112108
RegisterTTRPC(*ttrpc.Server) error
113109
}
114110

115-
type TTRPCServerOptioner interface {
116-
TTRPCService
111+
type TTRPCServerUnaryOptioner interface {
112+
UnaryServerInterceptor() ttrpc.UnaryServerInterceptor
113+
}
117114

118-
UnaryInterceptor() ttrpc.UnaryServerInterceptor
115+
type TTRPCClientUnaryOptioner interface {
116+
UnaryClientInterceptor() ttrpc.UnaryClientInterceptor
119117
}
120118

121119
var (
@@ -249,13 +247,6 @@ func run(ctx context.Context, manager Manager, config Config) error {
249247
}
250248

251249
ttrpcAddress := os.Getenv(ttrpcAddressEnv)
252-
publisher, err := NewPublisher(ttrpcAddress, WithPublishTTRPCOpts(
253-
ttrpc.WithUnaryClientInterceptor(otelttrpc.UnaryClientInterceptor()),
254-
))
255-
if err != nil {
256-
return err
257-
}
258-
defer publisher.Close()
259250

260251
ctx = namespaces.WithNamespace(ctx, namespaceFlag)
261252
ctx = context.WithValue(ctx, OptsKey{}, Opts{BundlePath: bundlePath, Debug: debugFlag})
@@ -333,7 +324,15 @@ func run(ctx context.Context, manager Manager, config Config) error {
333324
Type: plugins.EventPlugin,
334325
ID: "publisher",
335326
InitFn: func(ic *plugin.InitContext) (interface{}, error) {
336-
return publisher, nil
327+
return NewPublisher(ttrpcAddress, func(cfg *publisherConfig) {
328+
p, _ := ic.GetByID(plugins.TTRPCPlugin, "otelttrpc")
329+
if p == nil {
330+
return
331+
}
332+
333+
opts := ttrpc.WithUnaryClientInterceptor(p.(TTRPCClientUnaryOptioner).UnaryClientInterceptor())
334+
WithPublishTTRPCOpts(opts)(cfg)
335+
})
337336
},
338337
})
339338

@@ -342,6 +341,8 @@ func run(ctx context.Context, manager Manager, config Config) error {
342341
ttrpcServices = []TTRPCService{}
343342

344343
ttrpcUnaryInterceptors = []ttrpc.UnaryServerInterceptor{}
344+
345+
pprofHandler server
345346
)
346347

347348
for _, p := range registry.Graph(func(*plugin.Registration) bool { return false }) {
@@ -389,20 +390,23 @@ func run(ctx context.Context, manager Manager, config Config) error {
389390
if src, ok := instance.(TTRPCService); ok {
390391
log.G(ctx).WithField("id", pID).Debug("registering ttrpc service")
391392
ttrpcServices = append(ttrpcServices, src)
393+
}
392394

395+
if src, ok := instance.(TTRPCServerUnaryOptioner); ok {
396+
ttrpcUnaryInterceptors = append(ttrpcUnaryInterceptors, src.UnaryServerInterceptor())
393397
}
394398

395-
if src, ok := instance.(TTRPCServerOptioner); ok {
396-
ttrpcUnaryInterceptors = append(ttrpcUnaryInterceptors, src.UnaryInterceptor())
399+
if result.Registration.ID == "pprof" {
400+
if src, ok := instance.(server); ok {
401+
pprofHandler = src
402+
}
397403
}
398404
}
399405

400406
if len(ttrpcServices) == 0 {
401407
return fmt.Errorf("required that ttrpc service")
402408
}
403409

404-
ttrpcUnaryInterceptors = append(ttrpcUnaryInterceptors, otelttrpc.UnaryServerInterceptor())
405-
406410
unaryInterceptor := chainUnaryServerInterceptors(ttrpcUnaryInterceptors...)
407411
server, err := newServer(ttrpc.WithUnaryServerInterceptor(unaryInterceptor))
408412
if err != nil {
@@ -415,7 +419,7 @@ func run(ctx context.Context, manager Manager, config Config) error {
415419
}
416420
}
417421

418-
if err := serve(ctx, server, signals, sd.Shutdown); err != nil {
422+
if err := serve(ctx, server, signals, sd.Shutdown, pprofHandler); err != nil {
419423
if !errors.Is(err, shutdown.ErrShutdown) {
420424
cleanupSockets(ctx)
421425
return err
@@ -436,7 +440,7 @@ func run(ctx context.Context, manager Manager, config Config) error {
436440

437441
// serve serves the ttrpc API over a unix socket in the current working directory
438442
// and blocks until the context is canceled
439-
func serve(ctx context.Context, server *ttrpc.Server, signals chan os.Signal, shutdown func()) error {
443+
func serve(ctx context.Context, server *ttrpc.Server, signals chan os.Signal, shutdown func(), pprof server) error {
440444
dump := make(chan os.Signal, 32)
441445
setupDumpStacks(dump)
442446

@@ -456,9 +460,9 @@ func serve(ctx context.Context, server *ttrpc.Server, signals chan os.Signal, sh
456460
}
457461
}()
458462

459-
if debugFlag {
460-
if err := serveDebug(ctx); err != nil {
461-
return err
463+
if debugFlag && pprof != nil {
464+
if err := setupPprof(ctx, pprof); err != nil {
465+
log.G(ctx).WithError(err).Warn("Could not setup pprof")
462466
}
463467
}
464468

@@ -477,31 +481,6 @@ func serve(ctx context.Context, server *ttrpc.Server, signals chan os.Signal, sh
477481
return reap(ctx, logger, signals)
478482
}
479483

480-
func serveDebug(ctx context.Context) error {
481-
l, err := serveListener(debugSocketFlag, 4)
482-
if err != nil {
483-
return err
484-
}
485-
go func() {
486-
defer l.Close()
487-
m := http.NewServeMux()
488-
m.Handle("/debug/vars", expvar.Handler())
489-
m.Handle("/debug/pprof/", http.HandlerFunc(pprof.Index))
490-
m.Handle("/debug/pprof/cmdline", http.HandlerFunc(pprof.Cmdline))
491-
m.Handle("/debug/pprof/profile", http.HandlerFunc(pprof.Profile))
492-
m.Handle("/debug/pprof/symbol", http.HandlerFunc(pprof.Symbol))
493-
m.Handle("/debug/pprof/trace", http.HandlerFunc(pprof.Trace))
494-
srv := &http.Server{
495-
Handler: m,
496-
ReadHeaderTimeout: 5 * time.Minute,
497-
}
498-
if err := srv.Serve(l); err != nil && !errors.Is(err, net.ErrClosed) {
499-
log.G(ctx).WithError(err).Fatal("containerd-shim: pprof endpoint failure")
500-
}
501-
}()
502-
return nil
503-
}
504-
505484
func dumpStacks(logger *log.Entry) {
506485
var (
507486
buf []byte
@@ -516,3 +495,22 @@ func dumpStacks(logger *log.Entry) {
516495
buf = buf[:stackSize]
517496
logger.Infof("=== BEGIN goroutine stack dump ===\n%s\n=== END goroutine stack dump ===", buf)
518497
}
498+
499+
type server interface {
500+
Serve(net.Listener) error
501+
}
502+
503+
func setupPprof(ctx context.Context, srv server) error {
504+
l, err := serveListener(debugSocketFlag, 4)
505+
if err != nil {
506+
return fmt.Errorf("could not setup pprof listener: %w", err)
507+
}
508+
509+
go func() {
510+
if err := srv.Serve(l); err != nil && !errors.Is(err, net.ErrClosed) {
511+
log.G(ctx).WithError(err).Fatal("containerd-shim: pprof endpoint failure")
512+
}
513+
}()
514+
515+
return nil
516+
}

‎pkg/tracing/plugin/ttrpc.go‎

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
/*
2+
Copyright The containerd Authors.
3+
4+
Licensed under the Apache License, Version 2.0 (the "License");
5+
you may not use this file except in compliance with the License.
6+
You may obtain a copy of the License at
7+
8+
http://www.apache.org/licenses/LICENSE-2.0
9+
10+
Unless required by applicable law or agreed to in writing, software
11+
distributed under the License is distributed on an "AS IS" BASIS,
12+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
See the License for the specific language governing permissions and
14+
limitations under the License.
15+
*/
16+
17+
package plugin
18+
19+
import (
20+
"github.com/containerd/containerd/v2/plugins"
21+
"github.com/containerd/otelttrpc"
22+
"github.com/containerd/plugin"
23+
"github.com/containerd/plugin/registry"
24+
"github.com/containerd/ttrpc"
25+
)
26+
27+
func init() {
28+
const pluginName = "otelttrpc"
29+
30+
registry.Register(&plugin.Registration{
31+
ID: pluginName,
32+
Type: plugins.TTRPCPlugin,
33+
InitFn: func(ic *plugin.InitContext) (interface{}, error) {
34+
return otelttrpcopts{}, nil
35+
},
36+
})
37+
}
38+
39+
type otelttrpcopts struct{}
40+
41+
func (otelttrpcopts) UnaryServerInterceptor() ttrpc.UnaryServerInterceptor {
42+
return otelttrpc.UnaryServerInterceptor()
43+
}
44+
45+
func (otelttrpcopts) UnaryClientInterceptor() ttrpc.UnaryClientInterceptor {
46+
return otelttrpc.UnaryClientInterceptor()
47+
}

‎plugins/types.go‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,8 @@ const (
7373
CRIServicePlugin plugin.Type = "io.containerd.cri.v1"
7474
// ShimPlugin implements a shim service
7575
ShimPlugin plugin.Type = "io.containerd.shim.v1"
76+
// HTTPHandler implements an http handler
77+
HTTPHandler plugin.Type = "io.containerd.http.v1"
7678
)
7779

7880
const (

0 commit comments

Comments
 (0)